Sebastian Krysmanski
Sebastian Krysmanski

Reputation: 8394

Strange C++ reference invalidation on return

Have a look at the following code. The goal here is to return a reference through two functions (from ReferenceProvider::getReference() to getRef() to main()):

#include <tchar.h>
#include <assert.h>
#include <string>

class BaseClass {
public:
  virtual void write() const {
    printf("In base class\n");
  }
};
typedef BaseClass* BaseClassPointer;

class ChildClass : public BaseClass {
public:
  virtual void write() const {
    printf("In child class\n");
  }
};
typedef ChildClass* ChildClassPointer;

//////////////////////////////////////////////////////////////////////////

ChildClass* g_somePointer = new ChildClass();

class ReferenceProvider {
public:
  const BaseClassPointer& getReference() {
    const BaseClassPointer& val = g_somePointer;
    return val;
  }
};

ReferenceProvider g_provider;

const BaseClassPointer& getRef() {
  std::string test;

  const BaseClassPointer& val = g_provider.getReference();
  return val;
}

int _tmain(int argc, _TCHAR* argv[]) {
  BaseClass* child = getRef();

  assert(child == g_somePointer);
  child->write();

  return 0;
}

Now, when debugging this code (in Visual C++), breaking at return val; in getRef() will give you a screen like this:

Breaking a return

Notice how the values of g_somePointer and val are the same. Now, step over the return statement and you'll get a screen like this:

Breaking after return

Notice how val has become invalid (0xcccccccc). This is probably because the stack of getRef() has been cleared and val is no longer available.

The problem now is that child in _tmain() will get this invalid value (0xcccccccc) rendering child unusable. So my first (and main) question is: How to do this correctly?

(Please note that this is just an boiled down example from some other code I've been working on. It needs to be structured like with, including using references to pointers.)

What's making this whole thing very strange (and hard to debug) is that the function getRef() works under some conditions:

  1. If you change the type of g_somePointer to BaseClass* (from ChildClass*)
  2. If you remove the local variable in getRef() (i.e. the line std::string test;)

In both cases the reference variable val (in getRef()) will not become invalid and the function will return the correct pointer address. Can anybody explain this to me?

Upvotes: 3

Views: 595

Answers (6)

Xeo
Xeo

Reputation: 131789

To explain what's going on:

const BaseClassPointer& val = g_somePointer;

This line is the problem. Let's do away with the typedef:

BaseClass* const& val = g_somePointer;

Here, the type of g_somePointer is ChildClass*. In order to assign it to a BaseClass*, a conversion is needed. From that conversion, a temporary pointer is introduced. That pointer is bound to a reference-to-const, which extends the temporaries lifetime until the reference dies, which is exactly the case after your return val; statement. At that point, the temporary base-class pointer doesn't exist anymore and you have undefined behaviour.

To avoid all that mess, just return a BaseClass*.

Upvotes: 3

Mike Seymour
Mike Seymour

Reputation: 254431

The problem is here:

const BaseClassPointer& val = g_somePointer;

Since g_somePointer has a different type (ChildClass* is convertible to BaseClass*, but is not the same type), val cannot refer directly to g_somePointer. Instead, a temporary copy is made, converted to the correct type, and val refers to that.

The temporary only lasts as long as val, going out of scope at the end of the function, so the function returns an invalid reference.

If you change the type of g_somePointer to BaseClass* (from ChildClass*)

In that case, no pointer conversion is required, and so val can refer directly to g_somePointer. The code is then correct, but fragile.

If you remove the local variable in getRef() (i.e. the line std::string test;)

With the string variable, there is a destructor call at the end of the function, which overwrites the defunct stack frame that contains the temporary pointer. Without it, nothing overwrites the memory, so the code appears to work - which is unfortunate, as it makes the error much harder to notice.

Upvotes: 6

user1090249
user1090249

Reputation:

If you want the "val" to "survive", getReference() method should return a reference to a static object. Is that "static" going to work in your current architecture is another question.

Upvotes: 1

MGZero
MGZero

Reputation: 5963

You don't want to do something like that. Returning a reference to local memory is the same as returning the address of local memory, which is undefined behavior. All sorts of things can go wrong (or by chance, things can go right).

Upvotes: 1

Coder02
Coder02

Reputation: 260

You're returning a const reference to the local val instead of the returned getRef().

Also, how do you transform the pointer to a reference?

const BaseClassPointer& val = g_somePointer;
return val;

won't work if g_somePointer is a pointer - did you use *g_somePointer or similar?

Upvotes: 0

Dietmar K&#252;hl
Dietmar K&#252;hl

Reputation: 153820

You can never return a reference to a local object: it will always go out of scope when the function is exited. Sometimes it may appear as if it works but this is just because the data is normally not change when the stack pointer is adjusted.

Upvotes: 3

Related Questions