[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
   Target Milestone|---                         |4.9.0

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[reichelt at gcc dot gnu.org]

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

Volker Reichelt <reichelt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin.liska at gmail dot com

--- Comment #2 from Volker Reichelt <reichelt at gcc dot gnu.org> ---
*** Bug 59266 has been marked as a duplicate of this bug. ***

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Mine; another manifestation of the virtual inheritance representation problem.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[octoploid at yandex dot com]

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

--- Comment #5 from Markus Trippelsdorf <octoploid at yandex dot com> ---
(In reply to Alexander Ivchenko from comment #4)
> This broke Android image build with trunk compiler (in addition to pr58585)..

Yes. Many C++ projects that use multiple inheritance don't compile ATM:
Chromium, KDE, etc.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[nheghathivhistha at gmail dot com]

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

David Kredba <nheghathivhistha at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nheghathivhistha at gmail dot com

--- Comment #6 from David Kredba <nheghathivhistha at gmail dot com> ---
Affects qt-webkit-4.8.5 too.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

--- Comment #7 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This testcase manifest the other virtual inehritance problem.
Here it is algorithm of get_binfo_at_offset that first walks fields of a
structure and if it finds corresponding field at given offset, it walks bases
of structure to dive recursively into.

I am speaking of testcase in #1

Here get_binfo_at_offset is called on binfo of class D looking for class A at
offset 0.  The binfo of class D itself is one nested in E.  The inheritance
tree
looks as follows:

   A   B
 v/v\ /   (v marks virtual inheritance)
 C   D
  \ /
   E

ipa-devirt does not expect get_binfo_at_offset to fail since it knows D derives
A.

Now get_binfo_at_offset looks into memory layout of type D.  
 <record_type 0x7ffff76b17e0 D addressable tree_2 needs-constructing type_5 BLK
    size <integer_cst 0x7ffff753e140 type <integer_type 0x7ffff753c150
bitsizetype> constant 128>
    unit size <integer_cst 0x7ffff753e160 type <integer_type 0x7ffff753c0a8
sizetype> constant 16>
    align 64 symtab 0 alias set -1 canonical type 0x7ffff76b17e0
    fields <field_decl 0x7ffff769eed8 D.2255
        type <record_type 0x7ffff76a87e0 B addressable tree_2
needs-constructing type_5 BLK
            size <integer_cst 0x7ffff753e0c0 constant 64>
            unit size <integer_cst 0x7ffff753e0e0 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff76a87e0 fields
<field_decl 0x7ffff769e4c0 _vptr.B> context <translation_unit_decl
0x7ffff7546170 D.1>
            full-name "struct B"
            needs-constructor X() X(constX&) this=(X&) n_parents=0
use_template=0 interface-unknown
            pointer_to_this <pointer_type 0x7ffff76a8a80> chain <type_decl
0x7ffff7693ac8 B>>
        ignored decl_6 BLK file /home/jan/t.C line 13 col 8 size <integer_cst
0x7ffff753e0c0 64> unit size <integer_cst 0x7ffff753e0e0 8>
        align 64 offset_align 128
        offset <integer_cst 0x7ffff753e100 constant 0>
        bit offset <integer_cst 0x7ffff753e180 constant 0> context <record_type
0x7ffff76b17e0 D>
        chain <type_decl 0x7ffff76b3170 D type <record_type 0x7ffff76b1888 D>
            nonlocal decl_4 VOID file /home/jan/t.C line 14 col 1
            align 1 context <record_type 0x7ffff76b17e0 D> result <record_type
0x7ffff76b17e0 D>
            chain <field_decl 0x7ffff76b4098 D.2257>>> context
<translation_unit_decl 0x7ffff7546170 D.1>
    full-name "struct D"
    needs-constructor X() X(constX&) this=(X&) n_parents=2 use_template=0
interface-unknown
    pointer_to_this <pointer_type 0x7ffff76b1a80> chain <type_decl
0x7ffff76b30b8 D>>

and here it finds B that is a wrong turn and gives up after looking into B.
Now if it looked further
(gdb) p debug_tree (fld->common.chain->common.chain)
 <field_decl 0x7ffff76b4098 D.2257
    type <record_type 0x7ffff76a8000 A addressable tree_2 needs-constructing
type_5 BLK
        size <integer_cst 0x7ffff753e0c0 constant 64>
        unit size <integer_cst 0x7ffff753e0e0 constant 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff76a8000
        fields <field_decl 0x7ffff769e1c8 _vptr.A type <pointer_type
0x7ffff768ec78>
            unsigned virtual DI file /home/jan/t.C line 1 col 8 size
<integer_cst 0x7ffff753e0c0 64> unit size <integer_cst 0x7ffff753e0e0 8>
            align 64 offset_align 128
            offset <integer_cst 0x7ffff753e100 constant 0>
            bit offset <integer_cst 0x7ffff753e180 constant 0> context
<record_type 0x7ffff76a8000 A> chain <type_decl 0x7ffff7693958 A>> context
<translation_unit_decl 0x7ffff7546170 D.1>
        full-name "struct A"
        needs-constructor X() X(constX&) this=(X&) n_parents=0 use_template=0
interface-unknown
        pointer_to_this <pointer_type 0x7ffff76a82a0> chain <type_decl
0x7ffff76938a0 A>>
    ignored decl_6 BLK file /home/jan/t.C line 13 col 8 size <integer_cst
0x7ffff753e0c0 64> unit size <integer_cst 0x7ffff753e0e0 8>
    align 64 offset_align 128 offset <integer_cst 0x7ffff753e100 0> bit offset
<integer_cst 0x7ffff753e0c0 64> context <record_type 0x7ffff76b17e0 D>>
$20 = void

here it would find A, but this time at offset 64.
So perhaps we are looking for A at wrong offset? Or are we taking wrong vtable?

type_binfo is D since it is last on the walk down to A with virtual table
attached. Maybe it is because all references to A in D go via C's vtable and we
are safe to give up.  Or do we need to compensate for offsets here like this?
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c        (revision 205941)
+++ ipa-devirt.c        (working copy)
@@ -646,8 +646,11 @@

   if (types_same_for_odr (type, outer_type))
     {
-      tree inner_binfo = get_binfo_at_offset (type_binfo,
-                                             offset, otr_type);
+      tree inner_binfo = get_binfo_at_offset
+                         (type_binfo, offset
+                          + (tree_to_uhwi (BINFO_OFFSET (type_binfo))
+                             - tree_to_uhwi (BINFO_OFFSET (binfo)))
+                          * BITS_PER_UNIT, otr_type);
       /* For types in anonymous namespace first check if the respective vtable
         is alive. If not, we know the type can't be called.  */
       if (!flag_ltrans && anonymous)

(in single inheritance case this is not needed, since every binfo with non-zero
offset must have vtable attached).

Other option would be to simply return NULL when inner_binfo is NULL with a
comment that this is virtual multiple inheritance case where vtable was already
taken care on at different branch of recursion.

Jason, what do you think?

Markus, I do not have my usual firefox testing setup until christmas. Would be
nice to know if the proposed patch works on anything else than this testcase.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[trippels at gcc dot gnu.org]

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

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

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

--- Comment #8 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Honza,

I've tested your patch from comment 7 and it doesn't work.
However your suggestion "to simply return NULL when inner_binfo is NULL"
does seem to work fine. I've successfully build Chromium with it.

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index f5b5926504c7..c0fce4cde069 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -648,6 +648,8 @@ record_target_from_binfo (vec <cgraph_node *> &nodes,
     {
       tree inner_binfo = get_binfo_at_offset (type_binfo,
                                              offset, otr_type);
+      if (!inner_binfo)
+       return;
       /* For types in anonymous namespace first check if the respective vtable
         is alive. If not, we know the type can't be called.  */
       if (!flag_ltrans && anonymous)
@@ -661,7 +663,6 @@ record_target_from_binfo (vec <cgraph_node *> &nodes,
          if (!vnode || !vnode->definition)
            return;
        }
-      gcc_assert (inner_binfo);
       if (!pointer_set_insert (matched_vtables, BINFO_VTABLE (inner_binfo)))
        {
          tree target = gimple_get_virt_method_for_binfo (otr_token,
inner_binfo);

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at ucw dot cz]

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

--- Comment #9 from Jan Hubicka <hubicka at ucw dot cz> ---
> Honza,
> 
> I've tested your patch from comment 7 and it doesn't work.
> However your suggestion "to simply return NULL when inner_binfo is NULL"
> does seem to work fine. I've successfully build Chromium with it.

Good to know. I am however not sure if it is safe - this scenario happens in
"diamond"
shaped virtual inheritance and here we know we visited the base in question on
different
path. But I wonder if we can encounter different types of thunks.

I put more complete attempt to
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58252

Honza

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[aivchenk at gmail dot com]

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

--- Comment #10 from Alexander Ivchenko <aivchenk at gmail dot com> ---
Patch from comment #7 didn't cure Android build as well..

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[trippels at gcc dot gnu.org]

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

--- Comment #11 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
(In reply to Alexander Ivchenko from comment #10)
> Patch from comment #7 didn't cure Android build as well..

Can you try the patch from PR58252 comment 7? 
I've build Chromium (browser) successfully with it.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[aivchenk at gmail dot com]

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

--- Comment #12 from Alexander Ivchenko <aivchenk at gmail dot com> ---
(In reply to Markus Trippelsdorf from comment #11)
> (In reply to Alexander Ivchenko from comment #10)
> > Patch from comment #7 didn't cure Android build as well..
> 
> Can you try the patch from PR58252 comment 7? 
> I've build Chromium (browser) successfully with it.

Yep, the patch from PR58252 comment 7 has cured my build of Android

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[nheghathivhistha at gmail dot com]

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

--- Comment #13 from David Kredba <nheghathivhistha at gmail dot com> ---
Qtwebkit-4.8.5 still ICEs after that patch applied:
lto1: internal compiler error: in record_target_from_binfo, at ipa-devirt.c:673

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[trippels at gcc dot gnu.org]

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

--- Comment #14 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
(In reply to David Kredba from comment #13)
> Qtwebkit-4.8.5 still ICEs after that patch applied:
> lto1: internal compiler error: in record_target_from_binfo, at
> ipa-devirt.c:673

Can you attach a testcase, please?

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[nheghathivhistha at gmail dot com]

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

--- Comment #15 from David Kredba <nheghathivhistha at gmail dot com> ---
I am sorry, I can't. When I did -nostdlib link on all object files, it linked.
Only full link command with all libraries causes ICE for me and I do not now
how to reduce it/identify.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

--- Comment #16 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Should be fixed by proposed fix for
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58585.
Please help me to test it.

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[nheghathivhistha at gmail dot com]

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

--- Comment #17 from David Kredba <nheghathivhistha at gmail dot com> ---
Qtwebkit-4.8.5 compilation not ICEs gcc after your patch applied (it not links
the final library but that is another problem).

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[nheghathivhistha at gmail dot com]

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

--- Comment #18 from David Kredba <nheghathivhistha at gmail dot com> ---
It linked!

(After appended "__attribute__((used))" to line "void* JIT_STUB
cti_vm_throw(STUB_ARGS_DECLARATION);" in
qt-everywhere-opensource-src-4.8.5/src/3rdparty/webkit/Source/JavaScriptCore/jit/JITStubs.h.
Same trick as for qtscript for -flto.)

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

--- Comment #19 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Author: hubicka
Date: Fri Jan 10 09:33:24 2014
New Revision: 206516

URL: http://gcc.gnu.org/viewcvs?rev=206516&root=gcc&view=rev
Log:

    PR ipa/58252
    PR ipa/59226
    * ipa-devirt.c record_target_from_binfo): Take as argument
    stack of binfos and lookup matching one for virtual inheritance.
    (possible_polymorphic_call_targets_1): Update.

    * g++.dg/ipa/devirt-20.C: New testcase.    
    * g++.dg/torture/pr58252.C: Likewise.
    * g++.dg/torture/pr59226.C: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/devirt-20.C
    trunk/gcc/testsuite/g++.dg/torture/pr58252.C
    trunk/gcc/testsuite/g++.dg/torture/pr59226.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-devirt.c
    trunk/gcc/testsuite/ChangeLog

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #20 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Fixed, finally!

[Bug ipa/59226] [4.9 Regression] ICE: in record_target_from_binfo, at ipa-devirt.c:661

[hubicka at gcc dot gnu.org]

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

Bug 59226 depends on bug 58585, which changed state.

Bug 58585 Summary: [4.9 Regression] ICE in ipa with virtual inheritance
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58585

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED