Why does modifying a field that is referenced by another variable lead to unexpected behavior?

Question

I wrote this code that looked very simple to me at a first glance. It modifies a variable that is referenced by a reference variable and then returns the value of the reference. A simplified version that reproduces the odd behavior looks like this:

#include <iostream>
using std::cout;

struct A {
    int a;
    int& b;

    A(int x) : a(x), b(a) {}
    A(const A& other) : a(other.a), b(a) {}
    A() : a(0), b(a) {}
};

int foo(A a) {
    a.a *= a.b;
    return a.b;
}


int main() {
    A a(3);

    cout << foo(a) << '\n';
    return 0;
}

However, when it is compiled with optimization enabled (g++ 7.5), it produces output different to non-optimized code (i.e. 9 without optimizations - as expected, and 3 with optimizations enabled).

I am aware of the volatile keyword, which prevents the compiler from reordering and other optimizations in the presence of certain side-effects (e.g. async execution and hardware-specific stuff), and it helps in this case as well.

However, I do not understand why I need to declare reference b as volatile in this particular case? Where is the source of error in this code?

Allyce

volatile is very rarely the solution. Usually it's a bug. – Munson

Doesn’t it reduce to a problem like the following (without classes): int a = 3; int &b = a; a = a * b; - If so, does that fall under the sequence point rule? See en.cppreference.com/w/cpp/language/eval_order - “[It is] Undefined Behavior .. if a side effect on a scalar object is unsequenced relative to another side effect on the same scalar object..” – Photokinesis

@AlanBirtles the code in the post reproduces the described problem, and is already short. However, I added missing includes to make it compilable and added emphasis on expected output in the post. – Allyce

Managed to reproduce on godbolt. Remove -O3 to get 9 output. @AlanBirtles – Apace

possibly a bug introduced in gcc 6.4 godbolt.org/z/nn9G3h? – Birkett

perhaps the compiler thinks "a.a inside foo isn't used anywhere else, so I can 'optimise' that evaluation away", leaving a.b unchanged. It would be interesting to examine the assembly code to confirm if this is the case – Adventurer

What the compiler sees is that a.b is read twice. When optimizations are enabled (-O1 is sufficient) this read is optimized down to a single read, so the modified value is not picked in the return statement. I experimented with various -fno-strict-aliasing, -fstrict-aliasing flags (and the related warnings) but did not find anything that caused the compiler to re-read a.b. (gcc 10.1, x64, via Compiler Explorer) – Folderol

This behavior looks like a textbook Undefined Behavior. However the more I look at it, the more it seems to me that it's a gcc bug. Every object is initialized. Afaict there is no dangling reference. If we make everything constexpr clang correctly gives an error "reference to subobject of 'a' is not a constant expression". However gcc gives a wrong error message: "'A(3)' is not a constant expression because it refers to an incompletely initialized variable" which could mean just an bad-worded error text, or it could be that gcc thinks that somehow an subobject is uninitialized here. – Cortney

Any output other than 9 is a compiler bug – Hacker

Cases like this where optimization of extremely simple code is screwed up, always leaves me wondering how much critical code is out there that's been mis-compiled and nobody noticed yet – Hacker

You should file a bug report on GCC bugzilla – Apace

Answer 1

I could not find a source of UB in regard of the standard. This looks to me like a bug of the optimizer that would fail to notice that a.b and a.a both refer to the same object:

• First of all, foo() works on a copy. I changed foo() to pass by reference, and the expected result was consistently obtained. I suspected an issue in the initialization of the reference. But the provided copy constructor deals correctly with a.b.

• Then I suspected some UB related to side effects of undeterminately sequenced operations in the same expression. But the side effect on the lhs of *= is sequenced after the rhs, so that there is no UB here either.

• Adding some logging after the *= statement made it unexpectedly work as expected. This appeared very strange: it looks like the usual problems encountered when strict aliasing constraint is not respected, i.e. when the compiler doesn't realize that a pointed object was modified and otpimizes the code as if the value was unchanged. In such case, it's not unusual that additional code would cause the right value to be reloaded and find a different result.

• There is however no aliasing issue here, since the original member and the reference to it both are both based on the same type.

When you have eliminated the impossible, whatever remains, however improbable, must be the truth.
- Sir Arthur Conan Doyle

After having eliminated bugs and UB in the OP code, the only remaining possibility is a bug in the optimizer. It seems that the optimizer fails to note that a.a and a.b are the same object, and that it simply reuses the latest known value of a.b which is already in a register.

Ashantiashbaugh

Can you please explain why this answer different from the one I gave? – Agha

@KorelK Your answer cites a compiler bug, but it also invokes a number of misconceptions about how compilers operate (and the role of "reasonable code" vs a standard) and what the expected result of the code should be, which ultimately distract the reader and misinform them. For example, it invokes the notion of updates to memory and the register "racing" each other, which is very likely to be misinterpreted and/or misconstrued. The compiler bug is much likely a simpler aliasing/reuse bug based on invalid assumptions made during optimization. – Carlcarla

@Carlcarla I got the same results in C, and in C++ without optimizations. I mentioned there that it is a bug, but I gave the explanation what it might cost to fix it. – Agha

@KorelK I think the reasoning is different: I proceed by eliminating the possible UB cases according to the C++ language specifications, and come to the conclusion of a bug, that I can link to a case where such behavior would be normal but doesn't apply. In your answer you start to look at the generated code, assume this a s a bug, without verifying if there's any UB or implementation dependent rule could explain/allow this code generation. – Ashantiashbaugh

@KorelK I won't discount the results of your experiments, but I do disagree with the conclusion that you draw from them. There are plenty of reasons why a function call to sleep causes the bug go to away, many of which are far more believable that the compiler somehow emitted assembly that causes a race between stores to registers and mmemory. Furthermore, whether devs "usually" won't update a variable, and return some reference to it is irrelevant. If the C++ spec allows it, then the compiler must support it in order to be standards-compliant. – Carlcarla

@Ashantiashbaugh I agree with your explanation, but I do think this answer deserve at least one comment that says it, rather then kill it with DV lol – Agha

I deleted my answer 'cause I am not here to make anyone mistake, but I still believe the the trade-offs that I mentioned there are the reasons the bug exists in the C compiler, and came also to the C++ one. – Agha

@KorelK In my own experience, I must say that I'm always astonished how accurate the optimiser is in its analysis, and how prudent it is when optimisation conditions are not completely guaranteed. I never witnessed any "tradeoff" that would not be allowed according to the language specifications: no compiler vendor would dare to take this risks, because C++ is used in life critical medical devices, nuclear power plants, telecommunication networks, defence systems, and even Space X. – Ashantiashbaugh

@Ashantiashbaugh I didn't think of it this way, you're right. – Agha

@Ashantiashbaugh after talking again with a compilers writer, I understood that I was right about the things I wrote, and compilers optimizations have to take simplifying assumptions, or else most of the optimizations are illegal. – Agha

@KorelK I don't know what you mean by "simplifying assumptions", but the only simplifying assumptions that optimizations can make are "conservative" ones, ie an optimization can be applied in fewer cases than permitted. Your deleted answer isn't consistent with basic principles of programming language definition & implementation. Also it is not clear. – Eleanor

@Eleanor Your basic assumption that the compiler is a perfect creature, that can optimize your code using registers while consistently update the RAM, and keep references 100% of the time updated, even when it doesn't fully see the connection between the reference and the source, is wrong. I had this assumption too before, until a compilers writer opened my eyes to the reality. I understand that here you are looking for the rules and not for their reasons, and in the OP question might be a missed rule, but it doesn't mean that these rules have no reasons for existence. – Agha

@KorelK The goal & coding of compiler writers is exactly to make a compiled program act the way the manual says, although there are bugs in compilers, as with any human endeavour. MarkVeltzer's & your comments to the contrary are wrong. (I can't make sense of your last sentence.) We'll have to disagree. google.com/… – Eleanor