[Bug c++/67056] New: Wrong code generated

[Bug c++/67056] New: Wrong code generated

[balder@yahoo-inc.com]

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

            Bug ID: 67056
           Summary: Wrong code generated
           Product: gcc
           Version: 5.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: balder@yahoo-inc.com
  Target Milestone: ---

Yes, I know a very bad summary. I need some help to narrow it down.

When upgrading compile from 4.9.3 to 5.2 1 of our unit tests started failing
with segmentation fault. It compiles fine, but generates bad code. There are no
warning neither with 4.9.3 nor with 5.2. We are also running the unit tests
with latest valgrind. With 5.2 valgrind complains about accessing deleted
memory.

I am not sure how I best can provide you with what you need in order to
investigate this as it fails at runtime and the unit test uses other shared
libraries. Any suggestions ?

I have made the following observations.

1 - We are building with -O3. The issue is also present at -O2. However if I
add -fno-tree-vrp it improves. Then there is no segmentation fault, but
valgrind complains about a memory leak. It fails to run a destructor.

2 - I can fix the issue by reordering members in one class from
    ConfigSnapshot snap;
    std::atomic<bool> configured;
    std::atomic<bool> throwException;
to
    std::atomic<bool> configured;
    std::atomic<bool> throwException;
    ConfigSnapshot snap;
Then valgrind is happy to. No need for tinkering with the compile flags.

3 - I move the implementation of the destructor and the constructor out of the
class definition and add __attribute__((noinline)) to them.
Then everything works perfectly too.

What can I provide that can help you investigate this ?

This is the compile command for the unit test. The code that I mention in 2 and
3 are both contained in the file compiled here.
g++ -g -O3 -Wuninitialized -Werror -Wall -W -Wchar-subscripts -Wcomment
-Wformat -Wparentheses -Wreturn-type -Wswitch -Wtrigraphs -Wunused -Wshadow
-Wpointer-arith -Wcast-qual -Wcast-align -Wwrite-strings -DGCC_X86_64 -fPIC
-D_GLIBCXX_USE_CXX11_ABI=0 -DBOOST_DISABLE_ASSERTS -march=westmere -mtune=intel
-Wnon-virtual-dtor -std=c++14 -fvisibility-inlines-hidden            
-DNOT_BOOST_SPIRIT_THREADSAFE=1 -DNOT_PHOENIX_THREADSAFE=1
-DBOOST_NO_AUTO_PTR=1        -I../..
-I"/home/balder/build/lz4-PREINST/lz4-install/include" 
-I/home/balder/build/staging_vespalib-HEAD/staging_vespalib-install/include
-I"/home/balder/build/ytracelib-PREINST/ytracelib-install/include" 
-I/home/balder/build/fastlib-HEAD/fastlib-install/include
-I/home/balder/build/fnet-HEAD/fnet-install/include
-I/home/balder/build/vespalib-HEAD/vespalib-install/include -isystem
/home/balder/build/llvm-PREINST/llvm-install/include -isystem
/home/balder/build/boost-PREINST/boost-install/include/
-I/home/balder/build/vespalog-HEAD/vespalog-install/include
-I/home/balder/build/fastos-HEAD/fastos-install/include  
-DV_TAG_DATE='"20150729.142010"'  -DV_TAG_YINST='"5.81.0.20150729.142010"' 
-DV_TAG_COMPONENT='"5.81.0"'  -DV_TAG_ARCH='"x86_64"'  -DV_TAG_SYSTEM='"Linux"'
 -DV_TAG_SYSTEM_REV='"rhel-6.6"' 
-DV_TAG_BUILDER='"balder@honda.trondheim.corp"'  -DV_TAG='"CURRENT"'  -c
configretriever.cpp -o configretriever.o

g++ -v
[balder@honda configretriever]$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/home/y/bin64/../libexec/gcc/x86_64-redhat-linux/5.2.0/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ./configure --prefix=/home/y --enable-languages=c,c++
--host=x86_64-redhat-linux --build=x86_64-redhat-linux
--target=x86_64-redhat-linux --disable-bootstrap
LDFLAGS='-Wl,-rpath,/home/y/lib64 -L/home/y/lib64' --with-mpc=/home/y
--with-gmp=/home/y --with-mpfr=/home/y
Thread model: posix
gcc version 5.2.0 (GCC)

[Bug c++/67056] Wrong code generated

[ktkachov at gcc dot gnu.org]

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

--- Comment #1 from ktkachov at gcc dot gnu.org ---
There are some tips and instructions at:
https://gcc.gnu.org/bugs/

A reduced, preprocessed testcase would be ideal.

[Bug c++/67056] Wrong code generated

[trippels at gcc dot gnu.org]

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

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |trippels at gcc dot gnu.org

--- Comment #2 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Also try to build your project with -fsanitize=undefined and see
if any issues pop up while running the unit tests.

[Bug c++/67056] Wrong code generated

[balder@yahoo-inc.com]

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

--- Comment #3 from Henning Baldersheim <balder@yahoo-inc.com> ---
valgrind does not complain about anything undefined, but will try the sanitize
option too.

Will also try to isolate it as much as possible.

[Bug c++/67056] Wrong code generated

[balder@yahoo-inc.com]

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

--- Comment #4 from Henning Baldersheim <balder@yahoo-inc.com> ---
Using the -fsanitize-undefined caused this error.
configretriever.cpp: info:  running test suite 'configretriever.cpp'
/home/y/include/c++/5.2.0/bits/unique_ptr.h:76:2: runtime error: execution
reached a __builtin_unreachable() call
make: *** [test] Error 1

While it works fine when not inlining the constructor.

[Bug c++/67056] Wrong code generated

[vegard@yahoo-inc.com]

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

Vegard Sjonfjell <vegard@yahoo-inc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vegard@yahoo-inc.com

--- Comment #5 from Vegard Sjonfjell <vegard@yahoo-inc.com> ---
Created attachment 36122
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36122&action=edit
Minimal failing example

We've tried to strip the code down to a minimal failing example. Compile with
-O3 -fPIC -std=c++11 (or std=c++14).

[Bug c++/67056] Wrong code generated

[vegard@yahoo-inc.com]

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

--- Comment #6 from Vegard Sjonfjell <vegard@yahoo-inc.com> ---
Created attachment 36123
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36123&action=edit
Preprocessed file

Also adding the preprocessed file.

[Bug c++/67056] Wrong code generated

[trippels at gcc dot gnu.org]

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

--- Comment #7 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Created attachment 36126
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36126&action=edit
Somewhat reduced testcase

[Bug ipa/67056] [5/6 regression] Wrong code generated

[trippels at gcc dot gnu.org]

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

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-08-04
                 CC|                            |hubicka at gcc dot gnu.org
          Component|c++                         |ipa
            Summary|Wrong code generated        |[5/6 regression] Wrong code
                   |                            |generated
     Ever confirmed|0                           |1

--- Comment #8 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
markus@x4 tmp % g++  -fsanitize=undefined -std=c++14 -O2 -fPIC mem.ii
markus@x4 tmp % ./a.out
mem.ii:72:5: runtime error: execution reached a __builtin_unreachable() call
markus@x4 tmp % g++ -fno-devirtualize -fsanitize=undefined  -std=c++14 -O2
-fPIC mem.ii
markus@x4 tmp % ./a.out
markus@x4 tmp %

[Bug ipa/67056] [5/6 regression] Wrong code generated

[trippels at gcc dot gnu.org]

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

--- Comment #9 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Started with r215902.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[balder@yahoo-inc.com]

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

--- Comment #10 from Henning Baldersheim <balder@yahoo-inc.com> ---
Wrong code generation on valid code does sound like a P2 to me.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[trippels at gcc dot gnu.org]

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

--- Comment #11 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
(In reply to Henning Baldersheim from comment #10)
> Wrong code generation on valid code does sound like a P2 to me.

The release manager sets the importance.

Another question is if the transition from "requests valid" to
"wrong code" is a regression at all.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[trippels at gcc dot gnu.org]

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

--- Comment #12 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
(In reply to Markus Trippelsdorf from comment #11)
> Another question is if the transition from "requests valid" to
> "wrong code" is a regression at all.

Apart from the typo (s/requests/rejects) please ignore the comment
above, it was meant for a different PR.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org

--- Comment #13 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Will take a look.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

--- Comment #14 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
OK, the unreachable is introduced here:
 - Creating a specialized node of bool staticBoolFunc(CompositeClass*)/414 for
all known contexts.
     the new node is <built-in>/977.                                            
     known ctx 0 is     Outer type (dynamic):struct EmptyClass offset -64       
No devirtualization target in <built-in>/977                                    
ipa-prop: Discovered a virtual call to a known target (<built-in>/977 -> void
__builtin_unreachable()/976), for stmt OBJ_TYPE_REF(_15;ptr_2(D)->1)
(ptr_2(D));
/aux/hubicka/trunk-install/include/c++/6.0.0/bits/unique_ptr.h:76:2: note:
converting indirect call in <built-in> to direct call to void
__builtin_unreachable()
No devirtualization target in <built-in>/977                                    
ipa-prop: Discovered a virtual call to a known target (<built-in>/977 -> void
__builtin_unreachable()/976), for stmt OBJ_TYPE_REF(_27;ptr_2(D)->1)
(ptr_2(D));
/aux/hubicka/trunk-install/include/c++/6.0.0/bits/unique_ptr.h:76:2: note:
converting indirect call in <built-in> to direct call to void
__builtin_unreachable()

So ipa-CP thinks that staticBoolFunc is called on EmptyClass instead of
CompositeClass:

Jump functions:                                                                 
  Jump functions of caller  long unsigned int __builtin_object_size(const
void*, int)/967:
  Jump functions of caller  void operator delete(void*, long unsigned int)/964: 
  Jump functions of caller  void* operator new(std::size_t)/963:                
  Jump functions of caller  int main(int, char**)/415:                          
    callsite  int main(int, char**)/415 -> void operator delete(void*, long
unsigned int)/964 :
    callsite  int main(int, char**)/415 -> bool
staticBoolFunc(CompositeClass*)/414 :
       param 0: UNKNOWN                                                         
         Context:     Outer type (dynamic):struct EmptyClass offset -64         
         Unknown alignment                                                      
    callsite  int main(int, char**)/415 -> EmptyClass::EmptyClass()/404 :       
       param 0: UNKNOWN                                                         
         Context:     Outer type (dynamic): (or a derived type) (maybe in
construction) offset 64 Speculative outer type:struct CompositeClass (or a
derived type) at offset 64
         Unknown alignment                                                      

This is indeed wrong. Jump function analysis seems to confuse constructors:

Modification phase of node int main(int, char**)/402
int main(int, char**) (int D.39529, char * * D.39530)
{
  void * _3;
  struct EmptyClass * _7;

  <bb 2>:
  _3 = operator new (16);
  MEM[(struct  &)_3] ={v} {CLOBBER};
  MEM[(struct CompositeClass *)_3]._vptr.CompositeClass = &MEM[(void
*)&_ZTV14CompositeClass + 16B];
  _7 = &MEM[(struct CompositeClass *)_3].object;
  EmptyClass::EmptyClass (_7);

  <bb 3>:
  staticBoolFunc (_3);
  return 0;

<L1>:
  operator delete (_3, 16);
  resx 1

EmptyClass ctor is called, but it should not type the object.

Determining dynamic type for call: staticBoolFunc (_3);
  Starting walk at: staticBoolFunc (_3);
  instance pointer: _3  Outer instance pointer: _3 offset: 0 (bits) vtbl
reference: 
  Checking constructor call: EmptyClass::EmptyClass (_7);
  Recording type: struct EmptyClass at offset -64
  Determined dynamic type.

This is quite a nonsense, because EmptyClass is not even. So there are two
bugs.
First is that we determine useless outer type. This should be just missed
optimization. But we also manage to consider to miss the case in placement_new
checking where we are completely off the structure....

[Bug ipa/67056] [5/6 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

--- Comment #15 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Created attachment 36520
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36520&action=edit
Fix I am teching.

[Bug ipa/67056] [5/6 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |doko at gcc dot gnu.org

--- Comment #16 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
*** Bug 66738 has been marked as a duplicate of this bug. ***

[Bug ipa/67056] [5/6 regression] Wrong code generated

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |5.3

[Bug ipa/67056] [5/6 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

--- Comment #17 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Author: hubicka
Date: Wed Oct 21 21:14:06 2015
New Revision: 229148

URL: https://gcc.gnu.org/viewcvs?rev=229148&root=gcc&view=rev
Log:
        PR ipa/67056
        * ipa-polymorphic-call.c (possible_placement_new): If cur_offset
        is negative we don't know the type.
        (check_stmt_for_type_change): Skip constructors of non-polymorphic
        types as those won't help devirutalization.
        * g++.dg/ipa/pr67056.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/pr67056.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-polymorphic-call.c
    trunk/gcc/testsuite/ChangeLog

[Bug ipa/67056] [5 regression] Wrong code generated

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |UNCONFIRMED
            Summary|[5/6 regression] Wrong code |[5 regression] Wrong code
                   |generated                   |generated
     Ever confirmed|1                           |0

--- Comment #18 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Fixed on trunk so far; fix is backportable.