[Bug c++/63412] New: aliasing issue exposed by inlining

[doug.gilmore at imgtec dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63412

            Bug ID: 63412
           Summary: aliasing issue exposed by inlining
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: doug.gilmore at imgtec dot com

Created attachment 33616
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33616&action=edit
test program

The attached test program fails with 4.7 up to ToT at -O2 on both x86
(I built x86_64 with the -m32 multi-lib variant) and MIPS.

$ g++ -Wall -g -m32 -std=gnu++11 -O2 -fno-exceptions bad_i5.c -static -o la
-save-temps && ./la
Aborted (core dumped)
$ g++ -Wall -g -m32 -std=gnu++11 -O0 -fno-exceptions bad_i5.c -static -o la
-save-temps && ./la
$ 
Note that simplifying one of the expressions makes the program work:
$ g++ -Wall -g -DNO_VOL -m32 -std=gnu++11 -O2 -fno-exceptions bad_i5.c -static
-o la -save-temps && ./la
$ 

The generated code has the store below the implicit
load in the compare:

    cmpl    %ebx, 4(%esp,%edx,4)
    movl    %eax, 4(%esp)
    jne    .L5

which is incorrect.  It should be:

    movl    %eax, 4(%esp)
    cmpl    %ebx, 4(%esp,%edx,4)
    jne    .L5

We have an internal debate on what the issue is.

Some are of the opinion that casting is breaking alias rules and
thus the behavior of the program is undefined.

Thus something along the lines the following changes are needed.

$ diff bad_i5{,_mod}.c
48c48
<     return reference_->AsMirrorPtr();
---
>     return static_cast<T*>(reference_->AsMirrorPtr());
50c50
<   ObjectReference<T>* reference_;
---
>   ObjectReference<Object>* reference_;
52,53c52,53
<     : reference_(reinterpret_cast<ObjectReference<T>*>(reference))
<     { }
---
>     : reference_((reference))
>   { }
$ g++ -g -m32 -std=gnu++11 -O2 -fno-exceptions bad_i5_mod.c -static -o la
-save-temps && ./la
$

If there is a strict aliasing issue, shouldn't -Wall be warning about
it?

My take is that the casting is not a concern here since the returns
(and entries) from the inlined routines effectively sequences the
problematic store to be above the problematic load, and thus should
be considered a bug in GCC.