[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |lto, wrong-code
      Known to work|                            |4.9.2
   Target Milestone|---                         |5.0
            Summary|multiple definition of      |[5 Regression] multiple
                   |typeinfo in 5.0 (4.9.2      |definition of typeinfo in
                   |works)                      |5.0 (4.9.2 works)

[Bug lto/64860] multiple definition of typeinfo in 5.0 (4.9.2 works)

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Honza?

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-02-01
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Mine

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Hmm, interesting, I did not even think we support this ;)
I am not sure how much it makes sense to do incremntal linking like this with
LTO because this limits LTO to incremental link only (it would perhaps make
sense to produce LTO file after merigng in this case).

What changed since GCC 4.9 is that visibility pass is now more aggressive on
bringing comdat locals.  In resolution file we have:
$ more *.res
1
file2.o 6
212 f6705eca5fa850e3 PREVAILING_DEF _ZN11CDialogBase11GetDocumentEv
217 f6705eca5fa850e3 PREVAILING_DEF dialog2
204 f6705eca5fa850e3 PREVAILING_DEF _ZTV11CDialogBase
232 f6705eca5fa850e3 PREVAILING_DEF _ZTS11CDialogBase
248 f6705eca5fa850e3 PREVAILING_DEF _ZTI11CDialogBase
259 f6705eca5fa850e3 UNDEF _ZTVN10__cxxabiv117__class_type_infoE

which makes us to believe that all the symbols are previaling ones for current
DSO.  The trick it that incrementally this is not quite true. I suppose only
way to make this work reliably is to modify:
static bool                                                                     
resolution_to_local_definition_p (enum ld_plugin_symbol_resolution resolution)  
{                                                                               
  return (resolution == LDPR_PREVAILING_DEF                                     
          || resolution == LDPR_PREVAILING_DEF_IRONLY_EXP                       
          || resolution == LDPR_PREVAILING_DEF_IRONLY);                         
}                                                                               
to exclude LDPR_PREVAILING_DEF and similarly in ipa.c.  This way we lose some
optimization in non-incremental linking where we know that we are the only LTO
unit in proram.

So perhaps doing this conditionally on -r? We will probably makes it funnier to
produce testcases. Or we don't really care as it normally affects only cases
where non-LTO and LTO objects are mixed.

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yes, -r should be honored by the appropriate resolution info (not _IR_ONLY). 
Thus you have to treat that conservatively.

Otherwise you can create a testcase that fails with the final link and the
other strong definition in a shared library.

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Well, the problem is that -r is the only case where we get LDPR_PREVAILING_DEF
or IRONLY and the resulting symbol may be removed from the unit later.
We would need a new LDPR_PREVAILING_DEF_FOR_NOW or something like that to
represent the incremental nature of linking here, or we need a global flag in
GCC saying if incremental linking is supposed tohappen or not.

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
(just to explain bit more - the main difference between static and dynamic
linking here is that dynamic linking never remove any definition and thus you
can dissolve comdat groups)

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

--- Comment #8 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Created attachment 35100
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35100&action=edit
partial patch

Hi,
this is a patch that adds necessary checks into resolution code. Basically if
static linking is anticipated, we can not derive much of useful info from
PREVAILING_DEF/PREVAILING_DEF_IRONLY_EXP and must anticipate that static
linking may change these and drop comdat groups.

THe catch is that someone needs to pass -r to lto1 that is not currently
happening (I verified that if I hack opts.c to set incremental_linking to true
the testcase works).

With -r option, I can imagine that lto-wrapper can do it, because we get:
COLLECT_GCC_OPTIONS='-c' '-fmath-errno' '-fsigned-zeros' '-ftrapping-math'
'-fno-trapv' '-fno-openmp' '-fno-openacc' '-mtune=generic' '-march=x86-64'
'-O2' '-B' './' '-O2' '-r' '-v' '-mtune=generic' '-march=x86-64'
'-fltrans-output-list=/tmp/cc3o3UIr.ltrans.out' '-fwpa'
'-fresolution=/tmp/cchYXuvF.res

I donot know hwere -r gets dropped though.  But the testcase uses -Wl,-r that
may need LTO plugin extension?

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Lowering to P2 on Honza's request.

[Bug lto/64860] [5 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[hubicka at gcc dot gnu.org]

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

--- Comment #10 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I filled in binutils PR for extension of plugin API
https://sourceware.org/bugzilla/show_bug.cgi?id=18178

[Bug lto/64860] [5/6 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug lto/64860] [5/6 Regression] multiple definition of typeinfo in 5.0 (4.9.2 works)

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.