[Bug lto/60567] New: lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[Bug lto/60567] New: lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[burnus at gcc dot gnu.org]

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

            Bug ID: 60567
           Summary: lto1 ICE in add_symbol_to_partition, at
                    lto/lto-partition.c:233
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org

Possibly related: PR56775, PR56963 and PR57715 - or not.

The following code ICEs when doing with GCC 4.9:

$ gcc -c -flto test13.ii
$ gcc -r -nostdlib test13.o -flto
lto1: internal compiler error: in add_symbol_to_partition, at
lto/lto-partition.c:233
0x5b4dc4 add_symbol_to_partition
        ../../gcc/lto/lto-partition.c:231
0x5b4e69 add_references_to_partition
        ../../gcc/lto/lto-partition.c:94
0x5b4f9a add_symbol_to_partition_1
        ../../gcc/lto/lto-partition.c:168
0x5b649e lto_balanced_map()
        ../../gcc/lto/lto-partition.c:727
0x5b08a0 do_whole_program_analysis
        ../../gcc/lto/lto.c:3276
0x5b08a0 lto_main()
        ../../gcc/lto/lto.c:3416


Testcase:

#pragma implementation
class EIdent {   };
#pragma interface
class CellInsertedListener {
  virtual void cellInserted( const EIdent& newCell ) = 0;
};
class LayerListUpdateListener {
  virtual void layerRemoved( const EIdent& layerId ) = 0;
};
class VwBboxCacheMaster :
   public CellInsertedListener,
   public LayerListUpdateListener {
  virtual void layerRemoved (const EIdent&) {  }
};

[Bug lto/60567] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[burnus at gcc dot gnu.org]

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

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> Works for me - any more specifics?  Host/target?

gcc version 4.9.0 20140319, r208679.
Target: x86_64-unknown-linux-gnu
CentOS 6.5, glibc-2.12-1.132, binutils-2.20.51.0.2-5.36

Seems to be a problem of Binutils. If I use the current Binutils from
binutils-gdb git (= 2.24.51.20140319), it works.


The question is whether one can do something about it instead of ICEing. And
whether the other PRs (PR56775, PR56963 and PR57715) might have the same
problem.

[Bug lto/60567] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |UNCONFIRMED
     Ever confirmed|1                           |0

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #2)
> Seems to be a problem of Binutils. If I use the current Binutils from
> binutils-gdb git (= 2.24.51.20140319), it works.

But only for the test case of comment 0. It still fails for the big program and
for the following test case. [I wonder whether you can reproduce this one.]


Using either CentOS 6's Binutils or the git-trunk of Binutils
(2.24.51.20140319), the following fails for me:


$ g++ -Wfatal-errors -std=c++11 -r -nostdlib -flto test15.ii
lto1: internal compiler error: in add_symbol_to_partition, at
lto/lto-partition.c:233
0x5b4d44 add_symbol_to_partition
        ../../gcc/lto/lto-partition.c:231


//----------------------------------------------
#pragma implementation
#pragma interface
class MutableError { };
class MutableSelector { };
template <class CONVERTER> class MutableValueHolder {
  virtual MutableError
  isValidNativeValue(const typename CONVERTER::Value_t) const {}
};
class MutableNumericTaggedType : virtual public MutableSelector {
};
class MutableNumericType : public virtual MutableNumericTaggedType { };
class MutableIntegerConverter {
   public:
     typedef int Value_t;
};
class MutableIntegerValue
  : public MutableNumericType,
    protected MutableValueHolder<MutableIntegerConverter> {
  virtual MutableError
  isValidNativeValue(const MutableIntegerConverter::Value_t value) const {    }
};

[Bug lto/60567] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[burnus at gcc dot gnu.org]

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #3)
> But only for the test case of comment 0. It still fails for the big program
> and for the following test case. [I wonder whether you can reproduce this
> one.]

I tried it myself on openSUSE Factory and there both test cases succeed; it
also succeeds on Jakub's Fedora 20.

However, I just did a bootstrap on gcc20.fsffrance.org - and there both test
cases fail identically to CentOS6. (gcc20 = Debian 6.0.4/Squeeze; binutils
2.20; libc6 2.11.3-3).

[Bug lto/60567] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233

[burnus at gcc dot gnu.org]

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

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Another data point. Running on the "-flto"-compiled object file
  $ build/gcc/lto1  -fwpa test.o
fails with the ICE. However, using
  $ build/gcc/lto1  -flto test.o
is successful both on the CentOS6 system and on gcc20. On the openSUSE system,
either works flawlessly.

[Bug lto/60567] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 - when no -fresolution= is available

[burnus at gcc dot gnu.org]

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

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
The gcc/configure check has the following comment:
    # Require GNU ld or gold 2.21+ for plugin support by default.
    # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.

In the driver the difference comes via HAVE_LTO_PLUGIN - and only when set, the
.res files are produced. On the CentOS6 machine, Binutils is too old, hence
also -fuse-linker-plugin is not supported.

Regarding this PR: I think either LTO should be disabled with an error message
or lto1 be modified to not ICE when the .res data is not available.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |4.8.3
           Keywords|                            |lto
   Last reconfirmed|2014-03-19 00:00:00         |2014-03-21
                 CC|                            |hubicka at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|lto1 ICE in                 |[4.9 Regression] lto1 ICE
                   |add_symbol_to_partition, at |in add_symbol_to_partition,
                   |lto/lto-partition.c:233 -   |at lto/lto-partition.c:233
                   |when no -fresolution= is    |with -fno-use-linker-plugin
                   |available                   |
   Target Milestone|---                         |4.9.0

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
.res data is produced by the linker-plugin thus this bug boils down to an ICE
that can be reproduced with -fno-use-linker-plugin.

The non-linker-plugin path doesn't get much attention these days.  4.8 works
for me.

Confirmed.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |miles at gnu dot org

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
*** Bug 56775 has been marked as a duplicate of this bug. ***

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[hubicka at gcc dot gnu.org]

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

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 #10 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I will take a look.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
P1 for now.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I've bisected it (with hacked ld wrapper script that reported a 2010-ish GNU ld
in --version) down to r207489.
As Honza's 2014-02-04 change looked just like an optimization, I wonder whether
we couldn't perhaps just conditionalize those changes (the ipa.c and/or the
lto-partition.c change) on whether linker plugin is used or not (at least
temporarily for 4.9).  Honza, ping, all the other 3 pending P1s have patches
floating around or in the works, it would be nice to get rc1 out this week.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
--- gcc/symtab.c.jj    2014-03-21 22:23:43.000000000 +0100
+++ gcc/symtab.c    2014-04-09 17:37:40.709523997 +0200
@@ -1312,9 +1312,13 @@ symtab_get_symbol_partitioning_class (sy
   /* Linker discardable symbols are duplicated to every use unless they are
      keyed.
      Keyed symbols or those.  */
-  if (DECL_ONE_ONLY (node->decl)
+  if ((HAVE_LTO_PLUGIN
+       && (flag_use_linker_plugin
+       || (HAVE_LTO_PLUGIN == 2
+           && !global_options_set.x_flag_use_linker_plugin))
+       ? (DECL_ONE_ONLY (node->decl) && !node->forced_by_abi)
+       : DECL_COMDAT (node->decl))
       && !node->force_output
-      && !node->forced_by_abi
       && !symtab_used_from_object_file_p (node))
     return SYMBOL_DUPLICATE;


seems to make the ICE go away with -fno-use-linker-plugin or when the linker
doesn't support the plugin at all, didn't have to change ipa.c similarly.
But why is this happening and why this helps needs analyzing.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[hubicka at gcc dot gnu.org]

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

--- Comment #14 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
OK, the problem is that one comdat group has two functions:
_ZNK19MutableIntegerValue18isValidNativeValueEi/0 (isValidNativeValue)
@0x7ffff6adfe18
  Type: function definition analyzed
  Visibility: forced_by_abi externally_visible public weak
comdat_group:_ZNK19MutableIntegerValue18isValidNativeValueEi one_only
section_name:.text._ZNK19MutableIntegerValue18isValidNativeValueEi virtual
  Same comdat group as: _ZThn8_NK19MutableIntegerValue18isValidNativeValueEi/2
  Address is taken.
  Aux: @0x1  References: 
  Referring: *.LTHUNK0/1 (alias)_ZTV19MutableIntegerValue/3 (addr)
  Read from file: t.o
  Availability: available
  First run: 0
  Function flags:
  Called by: 
  Calls: 

and 

_ZThn8_NK19MutableIntegerValue18isValidNativeValueEi/2
(_ZThn8_NK19MutableIntegerValue18isValidNativeValueEi) @0x7ffff6c64148
  Type: function definition analyzed
  Visibility: externally_visible public weak
comdat_group:_ZNK19MutableIntegerValue18isValidNativeValueEi one_only
section_name:.text._ZNK19MutableIntegerValue18isValidNativeValueEi virtual
artificial
  Same comdat group as: _ZNK19MutableIntegerValue18isValidNativeValueEi/0
  Address is taken.
  Aux: @0x1  References: 
  Referring: _ZTV19MutableIntegerValue/3 (addr)
  Read from file: t.o
  Availability: overwritable
  First run: 0
  Function flags:
  Thunk fixed offset -8 virtual value 0 has virtual offset 0)
  Called by: 
  Calls: *.LTHUNK0/1 (1.00 per call) 

Thunk doesn't have forced_by_abi. This makes the partitinoning code to deal
with the comdat in two ways - duplicating into every partion that use it as
well as keying it to one partition.

This looks to me as C++ FE bug: When function is forced (keyed), its thunk
should also be forced IMO. It doesn't seem to make sense to keep function but
optimize out the thunk as we would do now (even w/o LTO)

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[hubicka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at redhat dot com

--- Comment #15 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This patch fixes the ICE by copying forced_by_abi as part of cgraph fixup in
ipa visibility.  I would like Jason to comment on this. I think fix at C++ FE
side would be more appropriate if the thunk is indeed keyed.

If not, I will update partitinoning predicate to always iterate the whole group
and see if any of symbols is keyed.

Index: ipa.c
===================================================================
--- ipa.c       (revision 209170)
+++ ipa.c       (working copy)
@@ -1032,6 +1032,7 @@ function_and_variable_visibility (bool w
                                   == DECL_COMDAT_GROUP (decl_node->decl));
              gcc_checking_assert (node->same_comdat_group);
            }
+         decl_node->forced_by_abi = node->forced_by_abi;
          if (DECL_EXTERNAL (decl_node->decl))
            DECL_EXTERNAL (node->decl) = 1;
        }

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jason at gcc dot gnu.org]

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

--- Comment #16 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #14)
> It doesn't seem to make sense to keep function
> but optimize out the thunk as we would do now (even w/o LTO)

Right.  When we emit a function, we need to also emit any associated thunks.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #15)
> This patch fixes the ICE by copying forced_by_abi as part of cgraph fixup in
> ipa visibility.  I would like Jason to comment on this. I think fix at C++
> FE side would be more appropriate if the thunk is indeed keyed.
> 
> If not, I will update partitinoning predicate to always iterate the whole
> group and see if any of symbols is keyed.
> 
> Index: ipa.c
> ===================================================================
> --- ipa.c       (revision 209170)
> +++ ipa.c       (working copy)
> @@ -1032,6 +1032,7 @@ function_and_variable_visibility (bool w
>                                    == DECL_COMDAT_GROUP (decl_node->decl));
>               gcc_checking_assert (node->same_comdat_group);
>             }
> +         decl_node->forced_by_abi = node->forced_by_abi;
>           if (DECL_EXTERNAL (decl_node->decl))
>             DECL_EXTERNAL (node->decl) = 1;
>         }

Shouldn't that be the other way around though?
  node->forced_by_abi = decl_node->forced_by_abi;
>From what I understand, decl_node is the target of the thunk, and node is
thunk, on this testcase decl_node->forced_by_abi is true originally, while
node->forced_by_abi is false.  So, if the thunks are supposed to inherit the
flag from the thunk target, we should change node->forced_by_abi, similarly how
we e.g. change DECL_EXTERNAL (node->decl).

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
By the C++ FE change, do you mean something like:
--- gcc/cp/method.c.jj    2014-03-27 08:06:11.000000000 +0100
+++ gcc/cp/method.c    2014-04-10 10:59:36.226288999 +0200
@@ -387,6 +387,7 @@ use_thunk (tree thunk_fndecl, bool emit_
   thunk_node = cgraph_add_thunk (funcn, thunk_fndecl, function,
                  this_adjusting, fixed_offset, virtual_value,
                  virtual_offset, alias);
+  thunk_node->forced_by_abi = funcn->forced_by_abi;
   if (DECL_ONE_ONLY (function))
     symtab_add_to_same_comdat_group (thunk_node,
                      funcn);
(which fixes this testcase too, but otherwise untested so far), or say change
cgraph_add_thunk so that it has node->forced_by_abi = decl_node->forced_by_abi;
(also untested).

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 32581
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32581&action=edit
gcc49-pr60567.patch

The ipa.c version (changing node->forced_by_abi instead of
decl_node->forced_by_abi), also bootstrapped/regtested on i686-linux and
bootstrapped on x86_64-linux with regtest still ongoing there.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #19)
> Created attachment 32581 [details]
> gcc49-pr60567.patch
> 
> The ipa.c version (changing node->forced_by_abi instead of
> decl_node->forced_by_abi), also bootstrapped/regtested on i686-linux and
> bootstrapped on x86_64-linux with regtest still ongoing there.

Regtested fine even on x86_64-linux.

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Apr 10 18:57:48 2014
New Revision: 209280

URL: http://gcc.gnu.org/viewcvs?rev=209280&root=gcc&view=rev
Log:
    PR lto/60567
    * ipa.c (function_and_variable_visibility): Copy forced_by_abi flag from
    decl_node to node.

    * g++.dg/lto/pr60567_0.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/lto/pr60567_0.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/ipa.c
    trunk/gcc/testsuite/ChangeLog

[Bug lto/60567] [4.9 Regression] lto1 ICE in add_symbol_to_partition, at lto/lto-partition.c:233 with -fno-use-linker-plugin

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.