[Bug c++/51910] New: -frepo linking failure

[Bug c++/51910] New: -frepo linking failure

[nephatrine at gmail dot com]

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

             Bug #: 51910
           Summary: -frepo linking failure
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: nephatrine@gmail.com


Created attachment 26388
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26388
Minimal Testcase

I'm having an issue with -frepo apparently not instantiating typeinfo/vtables
at least on x86_64 (have not tested on other archs). It is creating .rpo files,
but it cannot find the functions at the relinking stage.

I've attached a minimal testcase. The code compiles cleanly on GCC 4.6.2
without any errors, but 4.7 chokes on it. Here is the output of 'make' on my
system for the testcase.

g++ -frepo -o main.o -c main.cpp
g++ -frepo -o testcase main.o
collect: recompiling main.cpp
collect: relinking
collect2: error: '_ZN3FooIiED0Ev' was assigned to 'main.rpo', but was not
defined during recompilation, or vice versa
main.o:main.cpp:function Foo<int>::~Foo(): error: undefined reference to
'Foo<int>::~Foo()'
main.o:main.cpp:function Bar::~Bar(): error: undefined reference to
'Foo<int>::~Foo()'
main.o:main.cpp:function vtable for Foo<int>: error: undefined reference to
'Foo<int>::~Foo()'
main.o:main.cpp:function vtable for Foo<int>: error: undefined reference to
'Foo<int>::haggis()'
main.o:main.cpp:function typeinfo for Foo<int>: error: undefined reference to
'typeinfo name for Foo<int>'
collect2: error: ld returned 1 exit status
make: *** [testcase] Error 1

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|4.6.2                       |
            Summary|-frepo linking failure      |[4.7 Regression] -frepo
                   |                            |linking failure
      Known to fail|4.7.0                       |

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-20 02:58:40 UTC ---
Is there a reason why you are using -frepo ?  It is not as useful as it was
before elf had comdat.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[nephatrine at gmail dot com]

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

--- Comment #2 from Daniel Wolf <nephatrine at gmail dot com> 2012-01-20 03:44:05 UTC ---
(In reply to comment #1)
> Is there a reason why you are using -frepo ?  It is not as useful as it was
> before elf had comdat.

Removing -frepo seems to bloat my executable. The GCC documentation
(http://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html) seemed to
portray -frepo as being useful given that the alternatives I'd face are either
unnecessarily larger executables or reworking a large header-only template
library written in the "Borland model." I suppose explicit instantiation with
no implicit instantiation is also an option, but seems needlessly tedious and
error-prone to maintain particularly when you have many developers working on a
project.

>From your comment, I take it there is some way to enable something like COMDAT
folding to remove the duplicated function instantiations? If so, what flag(s)
would I need to add to the project to enable this?

Either way, I'd think the issue should probably be looked at. If -frepo is
considered deprecated or just too complicated or unimportant to fix, it should
probably just be removed altogether.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-01-20 03:47:05 UTC ---
> From your comment, I take it there is some way to enable something like COMDAT
folding to remove the duplicated function instantiations?

That is the point of COMDAT so that duplicated function instantiations are not
done.  

> Removing -frepo seems to bloat my executable.
Have you tried using -Wl,--gc-sections ?

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[nephatrine at gmail dot com]

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

--- Comment #4 from Daniel Wolf <nephatrine at gmail dot com> 2012-01-20 04:46:33 UTC ---
(In reply to comment #3)
> That is the point of COMDAT so that duplicated function instantiations are not
> done.  

They definitely are or the executable wouldn't be larger without -frepo, right?

> > Removing -frepo seems to bloat my executable.
> Have you tried using -Wl,--gc-sections ?

Enabling that flag does make the code smaller, but the combination of -frepo
and --gc-sections is smaller still, with the savings exactly equal to the sum
of the savings of using each flag individually. The flags seem completely
orthogonal in their function.

This all is getting pretty tangential to the reported bug, though. Is there a
better place to continue this particular discussion? For now I can just leave
out the flag and get the project to compile, albeit larger, but it's not an
ideal long-term solution. Of course, whether or not an alternative exists
doesn't invalidate the bug at hand. What's the point making -frepo available if
it does not work?

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[nephatrine at gmail dot com]

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

--- Comment #5 from Daniel Wolf <nephatrine at gmail dot com> 2012-01-22 22:10:17 UTC ---
If someone has any ideas about what the issue might be or can point me in the
right direction, I'll try to suss this out myself and post a patch if possible.
I'm just not very familiar with GCC's internals.

The .rpo files created by the working versions (4.6.1 & 4.6.2 - also confirmed
also by Ubuntu Linaro GCC maintainer) and trunk are identical aside from having
a different -frandom-seed value.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.0

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-01-23
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |sandra at codesourcery dot
                   |                            |com
     Ever Confirmed|0                           |1

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> 2012-01-23 21:54:29 UTC ---
Smaller testcase:

template<typename T>
struct Foo
{
  virtual ~Foo() { }
};

int main( int, char*[] )
{
  Foo<int> test;
}

rm -f wa.rpo && g++ -c -frepo wa.C && g++ wa.o

The problem is that the linker is demangling symbols, so collect2 can't tell
the difference between the different destructor variants and keeps trying to
adjust the first one when it's not the problem.

You can work around this by linking with -Wl,--no-demangle

This bug was caused by

2011-07-24  Sandra Loosemore  <sandra@codesourcery.com>

        * configure.ac (demangler_in_ld): Default to yes.
        * configure: Regenerated.
        * collect2.c (main): When HAVE_LD_DEMANGLE is defined, don't
        mess with COLLECT_NO_DEMANGLE, and just pass --demangle and
        --no-demangle options straight through to ld.  When
        HAVE_LD_DEMANGLE is not defined, set COLLECT_NO_DEMANGLE in a
        way that has the intended effect on Windows.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #7 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-23 23:14:18 UTC ---
In addition to specifying an explicit command-line option, I think that if you
configure GCC with --with-demangler-in-ld=no it'll restore the previous
behavior, at least on systems where the previous behavior actually worked.  See

http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01368.html

for the discussion of the various bugs I found here.

If proper operation of -frepo depends on particular linker demangling options
that ought to be documented, at least.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-27 10:10:08 UTC ---
Created attachment 26477
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26477
gcc47-pr51910.patch

Untested fix.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-27 10:17:25 UTC ---
Alternatively, we could temporarily (for the duration of do_tlink) #ifdef
HAVE_LD_DEMANGLE conditionally add putenv ("COLLECT_NO_DEMANGLE=1"); if getenv
("COLLECT_NO_DEMANGLE") is NULL and at the end of function remove it from the
environment again.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-27 11:05:04 UTC ---
Created attachment 26479
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26479
gcc47-pr51910.patch

The alternative patch.  I like the first patch more, because it will work even
with -Wl,--demangle.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #11 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-27 15:31:14 UTC ---
I like the first patch too.  Since -frepo seems to depend on telling the linker
not to demangle, better to just say so.

I'm not familiar with the overall code flow here.  Does -frepo end up doing the
final link with the demangling setting requested by the user, or does this
change mean it always implies --no-demangle?  E.g., if I specify both -frepo
and -Wl,-Map expecting to get a demangled map file, will I get one?

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-27 15:50:54 UTC ---
Unfortunately the patch doesn't work, I thought that there are separate ld
invocations for rpo stuff only and then a final one without, but that is not
the case.  So, I'm withdrawing both patches.  Either the changes will need to
be backed out and always force demangling to be done in collect2 as opposed to
ld,
or something else.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #13 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-27 20:14:22 UTC ---
Sigh.  I think it would be OK to make -frepo imply --no-demangle, and document
that this is the case.  If my previous patch is reverted, that'll still leave
-frepo broken on Windows hosts (because the logic in collect2 to disable
demangling in ld was broken on Windows) in addition to re-breaking mapfile
demangling on both Windows and Posix in the "normal" case where you aren't
using -frepo.

WDYT?

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-27 20:24:14 UTC ---
As can be seen from Jason's example, -frepo option is unfortunately not
mandatory on the link line when some objects have been compiled with -frepo.
And collect2 seems to work by running the linker, if it succeeded, fine,
otherwise look for *.rpo files around, try to parse the error output from the
linker, compile something again, link again.

So it would need to be done always.  Or perhaps tlink.c could somehow always
handle all the symbols that demangle the same together if the linker error
output contains demangled symbols.  Jason, do you think it would be possible?

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #15 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-27 23:22:45 UTC ---
I've just dug around in the code a bit and I think we can fix this.  I don't
have a build tree to use for this set up at the moment, but roughly:  the loop
to attempt relinking after processing repo files is in do_tlink.  Move the
tlink_execute call at the bottom of the loop to the top and add --no-demangle. 
Add another tlink_execute call without --no-demangle after the end of the loop
(but still in the "if (read_repo_files ..." condition).

That means you'll do two extra link steps when processing repo files, but
incurs no extra overhead in the normal case.

I'll play with that over the weekend unless somebody points out that it's a
dumb idea that won't work.  :-P

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #16 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-29 04:50:12 UTC ---
Created attachment 26498
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26498
new patch

The attached patch seems to DTRT; I tested it also with explicit -Wl,--demangle
and -Wl,--no-demangle on the command line, and -Wl,-Map=wa.map. 
Regression-testing now, and trying to figure out how to wrap up the test case
for dejagnu.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #17 from Sandra Loosemore <sandra at codesourcery dot com> 2012-01-30 00:12:53 UTC ---
Cleaned up version of patch, with Jason's test case.

http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01591.html

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jason at gcc dot gnu.org]

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

--- Comment #18 from Jason Merrill <jason at gcc dot gnu.org> 2012-02-11 08:50:27 UTC ---
Author: jason
Date: Sat Feb 11 08:50:23 2012
New Revision: 184127

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184127
Log:
    PR c++/51910
    * tlink.c (demangled_hash_entry): Change mangled to a VEC.
    (demangle_new_symbols): Fill it.
    (scan_linker_output): Walk it.
    (start_tweaking): Split out from scan_linker_output.
    (maybe_tweak): Update sym->chosen.
    * Makefile.in (COLLECT2_OBJS): Add vec.o and gcc-none.o

Added:
    trunk/gcc/testsuite/g++.dg/template/repo10.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tlink.c

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-11 09:16:32 UTC ---
Fixed, thanks.

[Bug c++/51910] [4.7 Regression] -frepo linking failure

[sandra at codesourcery dot com]

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

--- Comment #20 from Sandra Loosemore <sandra at codesourcery dot com> 2012-02-17 18:51:48 UTC ---
Apropos of the complaint that -frepo produces smaller executables than relying
just on the linker discarding duplicate COMDAT groups....  I finally got around
to packing up and submitting this linker patch that's been in my pile for a
while.

http://sourceware.org/ml/binutils/2012-02/msg00146.html