2

I have two classes, Foo and Bar. Bar maintains a reference to Foo, and has methods that call methods in Foo to change its state. The code is shown below.

class Foo
{
private:
    double m_value;

public:
    void setValue(double value) { 
        this->m_value = value;
    };

    double getValue() {
        return this->m_value;
    };
};

class Bar
{
private:
    Foo& m_foo;

public:
    Bar() : m_foo(Foo()) {
    };

    void setFooValue(double value) {
        m_foo.setValue(value);
    };

    double getFooValue() {
        return m_foo.getValue();
    };
};    

The problem arises when I try to access the value of foo after setting it, as below:

Bar bar;

bar.setFooValue(10000.0);

double value = bar.getFooValue();

std::cout << "Foo value is: " << value << std::endl;

Which outputs Foo value is -9.25596e+061. It appears the memory has become corrupt - why? I understand that not storing m_foo as a reference (i.e. using Foo m_foo;) will fix the issue, however I don't understand why this is either.

More puzzling still is that the code above works as desired when running in release mode.

I am using Visual Studio 2010 to compile.

Many thanks in advance!

mickeyt
  • 386
  • 1
  • 3
  • 16
  • That it *appears to work* in release mode and not in debug mode should be a fairly strong indicator you have undefined behavior, and as the answers below will testify, that indicator is well-warranted. – WhozCraig Apr 12 '13 at 10:29

3 Answers3

2

Few compilers does report error for constructor Bar(). I have got the below error while compiling itself,

In constructor 'Bar::Bar()': invalid initialization of non-const reference of type 'Foo&' from a temporary of type 'Foo' compilation terminated due to -Wfatal-errors

Bar() : m_foo(Foo()) { };

In the constructor Bar, Foo() returns an object that is created in the stack and its life time ends when the constructor returns. Hence its life time is temporary and this will lead to accessing undefined memory or dangling references.

Solution 1: Use the Foo object as is without any reference

class Bar
{
private:
   Foo m_foo;

public:
   Bar(){
};

void setFooValue(double value) {
    m_foo.setValue(value);
};

double getFooValue() {
    return m_foo.getValue();
};
};    

Solution 2: Pass Foo object as parameter to the Bar constructor and this will remain valid till the scope of main.

class Bar
{
private:
    Foo &m_foo;

public:
    Bar(Foo &x) : m_foo(x) {
};

void setFooValue(double value) {
    m_foo.setValue(value);
};

double getFooValue() {
    return m_foo.getValue();
};
};    

int main()
{
Foo x;
Bar bar(x);

bar.setFooValue(10000.0);

double value = bar.getFooValue();

std::cout << "Foo value is: " << value << std::endl;
}
Rahul Sundar
  • 480
  • 8
  • 26
  • 1
    This is not a solution to the problem. The `Foo x` by-val parameter of the `Bar` constructor is just as temporary as the original broken code from the OP, and has just as much undefined behavior. – WhozCraig Apr 12 '13 at 10:25
  • Till its in scope it is valid and goes undefined only when out of scope. – Rahul Sundar Apr 12 '13 at 10:29
  • And when do you think that **by-val** *parameter* is going to go out of scope? (look carefully at the constructor for `Bar` in Solution 2). – WhozCraig Apr 12 '13 at 10:30
  • Ah, got your point. Modified the Bar() constructor to receive by reference – Rahul Sundar Apr 12 '13 at 10:46
1
Bar() : m_foo(Foo())

You are trying to initialize lvalue-reference from temporary. You shouldn't do such things. After last bracket - object will be destroyed and you will have dangling reference. Really, there is non-standard extension in MSVC, that allows bind temporary object to reference. For example gcc give error for such cases test here You can check this answer rvalue to lvalue conversion Visual Studio too.

Community
  • 1
  • 1
ForEveR
  • 55,233
  • 2
  • 119
  • 133
1

When you create Bar

Bar() : m_foo(Foo()) {
};

You provide reference to newly created foo instance that will go out of scope and will be deleted right after being assigned to m_foo. And this is actually VS c compiler flaw, since g++ would give you and error for such code, that you can see here. http://liveworkspace.org/code/3kW04t$218

alexrider
  • 4,449
  • 1
  • 17
  • 27