[Bug optimization/10823] [3.3/3.4 regression] wrong code after first cse pass

["bangerth at ices dot utexas dot edu" <gcc-bugzilla at gcc dot gnu dot org>]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10823



------- Additional Comments From bangerth at ices dot utexas dot edu  2003-07-18 00:01 -------
Subject: Re:  [3.3/3.4 regression] wrong code after first cse pass


>     bangerth> Your code is faulty: in this function
>
>     bangerth>   LV get() const { return op(*static_cast<const ITP&>(*this),
> *second); }
>
>     bangerth> the first operand to op.operator() is constructed by calling
>
>     bangerth>   Uop::result_type Uev::operator* () const { return op(*p); }
>
>     bangerth> with result_type being of type RV. Thus, it's
>     bangerth> returned object is a temporary. However,
>     bangerth> op.Bop::operator() takes a reference and passes it
>     bangerth> on. This doesn't work.
>
> I think, your analysis is not entirely correct.  The first operand of
> op.operator() is indeed a temporary, but it is guaranteed to stay
> alive up to the next sequence point, that is, end of the return
> statement.  The Bop::operator() passes the reference to this temporary
> RV to the constructor of LV, which makes a copy of it [...]

Hm, that seems right. I think I thought this called the implemented 
constructor of RV, but it of course just calls the copy constructor.


> And, after all, even if this code had been an example of wrong usage
> of references (of that kind I'm seeing regularly by our students), it
> had to lead to a program crash when accessing an invalid reference.  The
> (miscompiled) code of the LV constructor reads instead the wrong field in
> RV (s2._size instead of s2._start) when copying from the temporary into the
> new LV object.

No, that's not quite right: when the compiler re-uses the stack space of a 
temporary, it might place other data there. When you later access is, 
expecting that the old object were there, you might just get "wrong" data. In 
general, violations of lifetimes of temporaries just invoke undefined 
behavior -- which might be wrong data or crashes, depending on circumstances.


> I hope, you revisit this case and eventually revert the RESOLUTION, so
> that the two days I've spent reading the assembler output and RTL
> dumps were not in vain :-).

I'm not yet convinced of legality of the code, but I'm willing to hear. What 
we would need is a _simpler_ testcase than your original one. I remember that 
it was _very_ hard to trace the whole thing due to the use of operator() and 
that all names somehow sounded similar. If you can come up with a somewhat 
clearer testcase, I'm sure we'll revisit the issue. Feel free to take my 
reduced testcase as a basis if you think it is valid.

I think the thing is just that the code is too complicated to reduce for 
someone who is not familiar what it is actually doing. We just need you help 
in this -- try to make it as small and self-explanatory as you can.


> Another evidence of this problem being a gcc bug is the fact that when
> I simplify the case a little bit further, namely throw away the
> inheritance Seq : Gen, the problem disappears and the code becomes correct!

No, that's the same thing as above: if you invoke undefined behavior, you may 
get anything, including the behavior you expected.


> By the way, what is this mysterious strict aliasing exactly?
> Is it described formally anywhere?  Is it merely the list in [3.10] (15) of
> ANSI C++ or something more elaborated?

In short, yes. In practice, it's often hard to find out, but usually people 
are doing casts in the wrong way.

W.

-------------------------------------------------------------------------
Wolfgang Bangerth              email:            bangerth@ices.utexas.edu
                               www: http://www.ices.utexas.edu/~bangerth/