Bug 59176

Summary: [4.9 Regression] ICE edge points to wrong declaration / verify_cgraph_node failed
Product: gcc Reporter: Tobias Burnus <burnus>
Component: ipaAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: dcb314, hubicka, jamborm
Priority: P1 Keywords: ice-on-valid-code
Version: 4.9.0   
Target Milestone: 4.9.0   
URL: http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01075.html
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2014-01-17 00:00:00
Attachments: Test case: Run as "g++ -O3 -c test24.ii"
Full output of "g++ -c -O3 test24.ii"
gzipped C++ source code

Description Tobias Burnus 2013-11-18 17:51:27 UTC
Created attachment 31237 [details]
Test case: Run as "g++ -O3 -c test24.ii"

The attached file fails on x86-64-gnu-linux with GCC 4.9.0 20131118 (r204944).

$ g++ -O2 -c test24.ii
$ g++ -O3 -c test24.ii 
...
test24.ii:19:27: error: edge points to wrong declaration:
...
test24.ii:19:27: internal compiler error: verify_cgraph_node failed
0x884276 verify_cgraph_node(cgraph_node*)
        ../../gcc/cgraph.c:2891
0x87f28e verify_symtab_node(symtab_node*)
        ../../gcc/symtab.c:853
Comment 1 Tobias Burnus 2013-11-18 17:52:16 UTC
Created attachment 31238 [details]
Full output of "g++ -c -O3 test24.ii"
Comment 2 Tobias Burnus 2013-11-18 23:51:16 UTC
While the full code very recently compiled for me with GCC 4.9, the reduced test case already ICEs with GCC 4.9.0 20130601; it works with GCC 4.8.3 r203692.

Bisecting points at r199577 (GCC 4.9.0 20130601).

Honza, can you have a look?
Comment 3 Tobias Burnus 2013-11-29 19:31:45 UTC
Now that PR59208 was fixed, I tried the test case of this PR again (with today's r205539).

Result: Still the same, an ICE with -O3 in verify_cgraph_node. (-O2 is fine.)
Comment 4 Markus Trippelsdorf 2014-01-17 09:45:28 UTC
A bit more reduced:

markus@x4 tmp % cat test.ii
template <class> class A {
protected:
  void m_fn2();
  ~A() { m_fn2(); }
  virtual void m_fn1();
};

class D : A<int> {};
template <class Key> void A<Key>::m_fn2() {
  m_fn1();
  m_fn1();
  m_fn1();
}

#pragma interface
class B {
  D m_cellsAlreadyProcessed;
  D m_cellsNotToProcess;

public:
  virtual ~B() {}
  void m_fn1();
};

class C {
  unsigned long m_fn1();
  B m_fn2();
  unsigned long m_fn3();
};
unsigned long C::m_fn1() {
CellHierarchy:
  m_fn2().m_fn1();
}

unsigned long C::m_fn3() {
CellHierarchy:
  m_fn2().m_fn1();
}


markus@x4 tmp % /var/tmp/gcc_test/usr/local/bin/g++ -O3 -c test.ii
test.ii:38:1: error: edge points to wrong declaration:
 }
 ^
 <function_decl 0x7fbc9b7e4c00 __base_dtor .constprop
    type <method_type 0x7fbc9b7ca690
        type <void_type 0x7fbc9b671000 void type_6 VOID
            align 8 symtab 0 alias set -1 canonical type 0x7fbc9b671000
            pointer_to_this <pointer_type 0x7fbc9b6710a8>>
        QI
        size <integer_cst 0x7fbc9b660280 constant 8>
        unit size <integer_cst 0x7fbc9b6602a0 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7fbc9b7ca690 method basetype <record_type 0x7fbc9b7c8bd0 B>
        arg-types <tree_list 0x7fbc9b7f30a0 value <pointer_type 0x7fbc9b7c8e70>
            chain <tree_list 0x7fbc9b65cb40 value <void_type 0x7fbc9b671000 void>>>>
    static autoinline decl_5 QI file test.ii line 21 col 11 align 16 context <record_type 0x7fbc9b7c8bd0 B> abstract_origin <function_decl 0x7fbc9b7b3200 B>
    full-name "B::~B()"
    pending-inline-info 0x7fbc9b7cc780>
 Instead of: <function_decl 0x7fbc9b7cb200 __comp_dtor 
    type <method_type 0x7fbc9b7c8dc8
        type <void_type 0x7fbc9b671000 void type_6 VOID
            align 8 symtab 0 alias set -1 canonical type 0x7fbc9b671000
            pointer_to_this <pointer_type 0x7fbc9b6710a8>>
        QI
        size <integer_cst 0x7fbc9b660280 constant 8>
        unit size <integer_cst 0x7fbc9b6602a0 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7fbc9b7c8dc8 method basetype <record_type 0x7fbc9b7c8bd0 B>
        arg-types <tree_list 0x7fbc9b7c0898 value <pointer_type 0x7fbc9b7c8e70>
            chain <tree_list 0x7fbc9b65cb40 value <void_type 0x7fbc9b671000 void>>>
        pointer_to_this <pointer_type 0x7fbc9b7ca498>>
    addressable used public static external weak autoinline decl_5 QI file test.ii line 21 col 11 align 16 context <record_type 0x7fbc9b7c8bd0 B> initial <error_mark 0x7fbc9b655f48> abstract_origin <function_decl 0x7fbc9b7b3200 B>
    full-name "B::~B()"
    pending-inline-info 0x7fbc9b7cc900 chain <function_decl 0x7fbc9b7cb100 __deleting_dtor >>
_ZN1C5m_fn3Ev/8 (long unsigned int C::m_fn3()) @0x7fbc9b7d2000
  Type: function definition analyzed
  Visibility: forced_by_abi externally_visible public
  References: __gxx_personality_v0/22 (addr)
  Referring: 
  Availability: available
  First run: 0
  Function flags: body
  Called by: 
  Calls: _ZN1BD2Ev.constprop.0/35 _ZN1BD2Ev.constprop.0/39 (inlined) (1.00 per call) (can throw external) _ZN1B5m_fn1Ev/21 (1.00 per call) _ZN1C5m_fn2Ev/20 (1.00 per call) (can throw external) 
test.ii:38:1: internal compiler error: verify_cgraph_node failed
Comment 5 Marek Polacek 2014-02-05 18:35:38 UTC
Started with r199577.  Might be a dup.
Comment 6 David Binderman 2014-02-16 17:55:13 UTC
I have another test case, available on request.
Comment 7 David Binderman 2014-02-26 15:25:59 UTC
(In reply to David Binderman from comment #6)
> I have another test case, available on request.

Still going wrong. 

From the gcc test suite, g++.old-deja/g++.jason/thunk1.C
is fine with -O2 and then goes wrong with -O3.
Comment 8 Jan Hubicka 2014-02-28 01:03:22 UTC
This seems to be bug in ipa-cp, it redirect edge from thunk to clone of the function thunk is associated with.
Old value = (cgraph_node *) 0x7ffff6c5fb88
New value = (cgraph_node *) 0x7ffff6c5f668
cgraph_set_edge_callee (e=0x7ffff6c76d68, n=0x7ffff6c5f668) at ../../gcc/cgraph.c:1081

(gdb) p dump_cgraph_node (stderr, (cgraph_node *)0x7ffff6c5fb88)
_ZN1BD1Ev/5 (B::~B()) @0x7ffff6c5fb88
  Type: function definition analyzed alias cpp_implicit_alias
  Visibility: external public weak comdat_group:_ZN1BD5Ev one_only
  Address is taken.
  Aux: @0x1d026e0  References: _ZN1BD2Ev/4 (alias)
  Referring: _ZTV1B/13 (addr)
  Availability: available
  First run: 0
  Function flags:
  Called by: _ZN1BD0Ev/6 (1.00 per call) (can throw external) 
  Calls: 
$24 = void
(gdb) p dump_cgraph_node (stderr, (cgraph_node *)0x7ffff6c5f668)
_ZN1BD2Ev.constprop.0/35 (<built-in>) @0x7ffff6c5f668
  Type: function definition analyzed
  Visibility: external public weak comdat_group:_ZN1BD5Ev one_only
  References: 
  Referring: 
  Availability: local
  First run: 0
  Function flags: local
  Called by: _ZN1C5m_fn3Ev/8 _ZN1C5m_fn3Ev/8 (1.00 per call) (can throw external) _ZN1C5m_fn1Ev/7 _ZN1C5m_fn1Ev/7 (1.00 per call) (can throw external) 
  Calls: 

I believe it is bug somewhere in logic skipping thunks.  Martin, this is probably yours.
Comment 9 Martin Jambor 2014-02-28 16:25:42 UTC
(In reply to Jan Hubicka from comment #8)
> 
> I believe it is bug somewhere in logic skipping thunks.  Martin, this is
> probably yours.

I on the other hand think this is either a verification false positive
or some sort of cgraph node removal bug.

What happens in IPA-CP is this: m_fn3/8 originally calls B::~B()/5 which
is an alias (not a thunk as far as I can see but I do not see how that
matters even if I was wrong) to B::~B()/4.  IPA-CP does not gather
constants for aliases or thunks separately (though it is careful not to
propagate first parameters through thunks), so IPA-CP is only interested
in the /4 real thing.  Furthermore, it decides to clone B::~B()/4 and
redirect the call from m_fn3/8 to the new clone.

In fact, the modified edge passes verify_edge_corresponds_to_fndecl
just fine several times because of

!clone_of_p (cgraph_function_or_thunk_node (node, NULL), e->callee))

part of the checking correctly detects this case.

But then, after being successfully verified a number of times, the above
fails, because the cgraph_function_or_thunk_node (node, NULL) part stops
returning the /4 node but returns the argument itself which is /5.  When
I now dump the node I cannot see it being an alias and that is because
symtab_remove_unreachable_nodes had zapped it.

I think that either symtab_remove_unreachable_nodes should remove nodes
it cripples from the decl->symtab hash (why does it not call
cgraph_remove_node?)  or the verifier needs to understand that these
nodes are no good and ignore them.
Comment 10 Jan Hubicka 2014-02-28 17:36:45 UTC
hmm, I see, same body alias...

Well, remove_unreachable_nodes does two things, it removes nodes that are completely unreachable and also removes definitions of nodes where definition is not needed, but they are still referenced and thus needs to stay.  I suppose it is what happens here.
I will check how to silence the verifier.
Comment 11 David Binderman 2014-03-15 12:39:48 UTC
(In reply to Jan Hubicka from comment #10)
> I will check how to silence the verifier.

Over two weeks has elapsed. 

Is there anything I can help with to expedite this bug fix ?

4.9.0 is due soon.
Comment 12 Martin Jambor 2014-03-20 16:11:37 UTC
(In reply to Jan Hubicka from comment #10)
> I will check how to silence the verifier.

I've just proposed a way of doing that on the mailing list:
http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01075.html
Comment 13 Martin Jambor 2014-03-21 13:00:06 UTC
Author: jamborm
Date: Fri Mar 21 12:59:35 2014
New Revision: 208748

URL: http://gcc.gnu.org/viewcvs?rev=208748&root=gcc&view=rev
Log:
2014-03-21  Martin Jambor  <mjambor@suse.cz>

	PR ipa/59176
	* cgraph.h (symtab_node): New flag body_removed.
	* ipa.c (symtab_remove_unreachable_nodes): Set body_removed flag
	when removing bodies.
	* symtab.c (dump_symtab_base): Dump body_removed flag.
	* cgraph.c (verify_edge_corresponds_to_fndecl): Skip nodes which
	had their bodies removed.

testsuite/
        * g++.dg/torture/pr59176.C: New test.


Added:
    trunk/gcc/testsuite/g++.dg/ipa/pr59176.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cgraph.c
    trunk/gcc/cgraph.h
    trunk/gcc/ipa.c
    trunk/gcc/symtab.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 Martin Jambor 2014-03-21 13:01:22 UTC
Fixed.
Comment 15 David Binderman 2014-03-24 10:17:03 UTC
Created attachment 32434 [details]
gzipped C++ source code
Comment 16 David Binderman 2014-03-24 10:18:58 UTC
I am not sure this is fixed. Please see attached
source code which still fails.
Comment 17 Martin Jambor 2014-03-24 17:46:10 UTC
(In reply to David Binderman from comment #16)
> I am not sure this is fixed. Please see attached
> source code which still fails.

Although the ICE message is the same, this is most certainly a
different bug as it happens at a different time during the compilation
process.  I will look it at it some more tomorrow.
Comment 18 Markus Trippelsdorf 2014-03-24 19:16:12 UTC
markus@x4 tmp % cat test.ii
class ASN1Object
{
public:
  virtual ~ASN1Object ();
};
class A
{
  virtual unsigned m_fn1 () const;
};
class B
{
public:
  ASN1Object Element;
  virtual unsigned m_fn1 (bool) const;
};
template <class BASE> class C : public BASE
{
};

class D : ASN1Object, public B
{
};
class G : public D
{
  unsigned m_fn1 (bool) const {}
};
class F : A
{
public:
  F (A);
  unsigned m_fn1 () const
  {
    int a;
    a = m_fn2 ().m_fn1 (0);
    return a;
  }
  const B &m_fn2 () const { return m_groupParameters; }
  C<G> m_groupParameters;
};
template <class D> void BenchMarkKeyAgreement (int *, int *, int)
{
  A f;
  D d (f);
}

void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }


markus@x4 tmp % g++ -c -O3 test.ii
test.ii:46:60: error: edge points to wrong declaration:
 void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }
                                                            ^
 <function_decl 0x7fd4c010fa00 m_fn1.constprop
    type <function_type 0x7fd4c00d3348
        type <integer_type 0x7fd4bff4a738 unsigned int public unsigned SI
            size <integer_cst 0x7fd4bff4c440 constant 32>
            unit size <integer_cst 0x7fd4bff4c460 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7fd4bff4a738 precision 32 min <integer_cst 0x7fd4bff4c480 0> max <integer_cst 0x7fd4bff4c420 4294967295>
            pointer_to_this <pointer_type 0x7fd4c0060150>>
        QI
        size <integer_cst 0x7fd4bff4c280 constant 8>
        unit size <integer_cst 0x7fd4bff4c2a0 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7fd4c00d3348
        arg-types <tree_list 0x7fd4bff48b40 value <void_type 0x7fd4bff5d000 void>>>
    readonly addressable used nothrow private static autoinline decl_5 QI defer-output file test.ii line 25 col 12 align 16 context <record_type 0x7fd4c00b7e70 G>>
 Instead of: <function_decl 0x7fd4c00b8c00 _ZThn8_NK1G5m_fn1Eb
    type <method_type 0x7fd4c00ba150
        type <integer_type 0x7fd4bff4a738 unsigned int public unsigned SI
            size <integer_cst 0x7fd4bff4c440 constant 32>
            unit size <integer_cst 0x7fd4bff4c460 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7fd4bff4a738 precision 32 min <integer_cst 0x7fd4bff4c480 0> max <integer_cst 0x7fd4bff4c420 4294967295>
            pointer_to_this <pointer_type 0x7fd4c0060150>>
        QI
        size <integer_cst 0x7fd4bff4c280 constant 8>
        unit size <integer_cst 0x7fd4bff4c2a0 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7fd4c00ba150 method basetype <record_type 0x7fd4c00b7e70 G>
        arg-types <tree_list 0x7fd4c00abca8 value <pointer_type 0x7fd4c00ba1f8>
            chain <tree_list 0x7fd4c00ab3c0 value <boolean_type 0x7fd4bff4abd0 bool>
                chain <tree_list 0x7fd4bff48b40 value <void_type 0x7fd4bff5d000 void>>>>
        pointer_to_this <pointer_type 0x7fd4c00baa80>>
    readonly addressable asm_written used public weak virtual decl_5 QI file test.ii line 25 col 12 align 8 context <record_type 0x7fd4c00b7e70 G>
    arguments <parm_decl 0x7fd4c00bb380 this
        type <pointer_type 0x7fd4c00ba2a0 type <record_type 0x7fd4c00ba0a8 G>
            readonly unsigned DI
            size <integer_cst 0x7fd4bff4c0c0 constant 64>
            unit size <integer_cst 0x7fd4bff4c0e0 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7fd4c00ba2a0>
        readonly unsigned DI file test.ii line 25 col 25 size <integer_cst 0x7fd4bff4c0c0 64> unit size <integer_cst 0x7fd4bff4c0e0 8>
        align 64 context <function_decl 0x7fd4c00b8c00 _ZThn8_NK1G5m_fn1Eb> arg-type <pointer_type 0x7fd4c00ba2a0>
        chain <parm_decl 0x7fd4c00bb400 D.2326 type <boolean_type 0x7fd4bff4abd0 bool>
            unsigned QI file test.ii line 25 col 19 size <integer_cst 0x7fd4bff4c280 8> unit size <integer_cst 0x7fd4bff4c2a0 1>
            align 8 context <function_decl 0x7fd4c00b8c00 _ZThn8_NK1G5m_fn1Eb>
            arg-type <integer_type 0x7fd4bff4a690 int>>>
    full-name "virtual unsigned int G::_ZThn8_NK1G5m_fn1Eb(bool) const"
   >
_ZNK1F5m_fn1Ev/3 (virtual unsigned int F::m_fn1() const) @0x7fd4c00c4290
  Type: function definition analyzed
  Visibility: externally_visible public weak comdat comdat_group:_ZNK1F5m_fn1Ev one_only virtual
  Address is taken.
  References: 
  Referring: _ZTV1F/31 (addr)
  Availability: available
  First run: 0
  Function flags: body
  Called by: 
  Calls: _ZNK1G5m_fn1Eb.constprop.1/73 (1.00 per call) (can throw external) 
test.ii:46:60: internal compiler error: verify_cgraph_node failed
Comment 19 Tobias Burnus 2014-03-24 20:46:39 UTC
(In reply to Martin Jambor from comment #17)
> Although the ICE message is the same, this is most certainly a
> different bug as it happens at a different time during the compilation
> process.  I will look it at it some more tomorrow.

I have filled PR60640 for this - with a reduced test case.
Comment 20 Martin Jambor 2014-03-25 10:57:39 UTC
Author: jamborm
Date: Tue Mar 25 10:57:07 2014
New Revision: 208809

URL: http://gcc.gnu.org/viewcvs?rev=208809&root=gcc&view=rev
Log:
2014-03-25  Martin Jambor  <mjambor@suse.cz>

	PR ipa/59176
	* lto-cgraph.c (lto_output_node): Stream body_removed flag.
	(lto_output_varpool_node): Likewise.
	(input_overwrite_node): Likewise.
	(input_varpool_node): Likewise.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lto-cgraph.c