8

In my C++ program I have a function that returns a map containing elements that each can have a pointer to another element in the map. I set these pointers before returning the map at the end of the function. Example code:

#include <iostream>
#include <map>
#include <string>

class TestObject{
public:
    TestObject(std::string message) : message(message), other(nullptr){}

    TestObject* other;
    std::string message;
};

std::map<std::string, TestObject> mapReturningFunction(){
    std::map<std::string, TestObject> returnMap;

    TestObject firstObject("I'm the first message!");

    returnMap.insert(std::make_pair("first", firstObject));

    TestObject secondObject("I'm the second message!");
    returnMap.insert(std::make_pair("second", secondObject));

    TestObject* secondObjectPointer = &(returnMap.at("second"));
    returnMap.at("first").other = secondObjectPointer;

    return returnMap;
}

int main(){
    std::map<std::string, TestObject> returnedMap = mapReturningFunction();

    std::cout << returnedMap.at("first").other->message << std::endl; // Gives a valid message every time

    std::cin.get();

    return 0;
}

At the call-site of the function, the pointer other is still valid, even though I suspected it would become invalid because the map inside the function of which the 'pointed-to object' is an element of goes out of scope.

Is this basically the same as mentioned in Can a local variable's memory be accessed outside its scope? ? I'm basically 'lucky' the pointer is still pointing to valid data? Or is something different going on?

I really do think it is a 'lucky' hit every time, but some confirmation would be very nice.

Community
  • 1
  • 1
Soulrot
  • 135
  • 6
  • 1
    possible duplicate of [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – erenon Aug 06 '14 at 18:12
  • if NRVO is not performed, then the returned map will contain pointer to freed memory. – yngccc Aug 06 '14 at 18:13

5 Answers5

3

Yes, you are "lucky" (not that much, your program will crash or do bad things in a way or another sooner or later).

Explanation:

returnMap is allocated on the stack: it will be destroyed when mapReturningFunction returns.

You take the address of an object inside the map and assign it as an other pointer.

When your function returns, returnMap is copied to the returned value, so the (copied) pointer now really points to garbage.

Optimizer often avoid this last copy, its called "copy elision" (or "Return Value Optimization") and might be the reason of your "normal" behavior. But it doesn't matter, your program has undefined behavior.

quantdev
  • 23,517
  • 5
  • 55
  • 88
1

It's not luck. You are returning a pointer to a location on the stack. The location is valid and it happens to have the last value that was placed there, until something else changes it.

Here's an example, and I'm borrowing the idea from the other question you link too, which is exactly the same:

#include <stdio.h>

int* foo()
{
    int a = 5;
    return &a;
}

void nukestack()
{
    int a = 7;
    printf("putting 7 on the stack\n");
}

void main()
{
    int* p = foo();
    printf("%d\n", *p);
    nukestack();
    printf("%d\n", *p);
}

The print from the program will be this:

5
putting 7 on the stack
7

The reason is this. We first call foo(), which allocates space on the stack for variable a. We write 5 to this location, then return from the function, releasing that stack space, but leaving the memory untouched. We then call nukestack(), which allocates space on the stack for its own variable a. Because the functions are so similar, and the variables are the same size in both functions, their memory locations happen to overlap.

At this point the new a variable will still have the old value. But we now overwrite the 5 with 7. We return from the function and our old pointer p still points to this same location, which now has a 7 there.

This is undefined behavior and you're technically breaking the rules if you rely on it. With most compilers you will also get a warning when you return a pointer to a local variable, and warnings should really never be ignored.

Flawe
  • 49
  • 2
  • "You are returning a pointer to a location on the stack." No. He returns a map from the stack, not a pointer. The source map contains pointers to elements on the heap. When the source map is destroyed, those elements are destroyed too, rendering access to the memory they occupied illegal. The copied map therefore contains pointers to illegal memory. And `void main` is not C++ and should be refused by every modern compiler I know. – Christian Hackl Aug 06 '14 at 18:22
  • Yes, I worded that wrong. In my example the pointer is returned. In the original question the pointer to the map is stored, which is still points to the same memory location on the stack. Even if the map gets destroyed, memory is often not overwritten in the destructor and contains the same values until overwritten. – Flawe Aug 06 '14 at 18:28
1

Yes, it's only "lucky". You are getting undefined behaviour by keeping a pointer to the element of a map which has been destroyed. This is not exactly the same as keeping a pointer to a local map itself, because the map's elements are allocated dynamically, but it comes down to the same result here.

It's not that easy to actually enforce a crash or other "wrong" behaviour here. This is presumably due to return-value optimisation, i.e. the copy of the map is elided, so the source is not overwritten.

But the following works for me in VC++ 2013:

In your mapReturnFunction, change the return statement to

return true ? returnMap : returnMap;

As strange as this may be, this trick will disable return-value optimisation, resulting in an actual copy being made. Compiled with /EHsc /Za (although none of these two flags should really make a difference here), the result on my machine is that nothing is printed.

The fact that the observable behaviour changes so drastically through a statement which should make no difference gives you a strong hint that something is terribly wrong.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
1

You are not lucky. Copy elision or move constructors (C++11) will prevent destruction of the original data. While the first is an optional compiler optimization, the second will (if the first is not applicable) create a moved version of the original without invalidating the references.

Be aware, as soon you make a copy (without move) and an original gets destroyed, the pointers to other elements are invalid!

1

Pre C++11, you are "lucky". The code will likely work due copy elision, specifically NRVO (Named Return Value Optimization) means that returnMap and returnedMap are in fact the same object. However, you are not allowed to trust that, so the code invokes undefined behaviour.

Post C++11, you are "lucky", but less so. The map has a move constructor, and return returnMap; implicitly moves the local returnMap as prvalue, which is then usable for move constructor of the returnedMap. The compiler can still elide this, but you are allowed to rely on local value being moved. However, the standard does not give quarantees on what exactly happens when a container is moved, so you still invoke undefined behaviour, but that likely changes once LWG open issue 2321 is resolved.

eidolon
  • 3,223
  • 2
  • 16
  • 5