Bug 35067 - [4.3/4.4/4.5 Regression] multiple definition of virtual thunk
[4.3/4.4/4.5 Regression] multiple definition of virtual thunk
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: c++
4.2.3
: P4 normal
: 4.3.5
Assigned To: Jason Merrill
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-03 22:32 UTC by Vincent Riviere
Modified: 2009-11-04 23:41 UTC (History)
2 users (show)

See Also:
Host:
Target: i386-netbsd
Build:
Known to work: 4.1.2
Known to fail: 4.2.3 4.3.0
Last reconfirmed: 2009-11-03 21:21:25


Attachments
Output of f1.cpp (8.54 KB, text/plain)
2008-02-03 23:02 UTC, Vincent Riviere
Details
Output of f2.cpp (8.12 KB, text/plain)
2008-02-03 23:03 UTC, Vincent Riviere
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Riviere 2008-02-03 22:32:09 UTC
C++ programs using virtual inheritance cannot be linked due to multiple definitions of thunk symbols.

$ cat c.h
class B
{
public:
        virtual ~B() { }
};

class C : virtual public B
{

};

$ cat f1.cpp 
#include "c.h"

C c1;

main() { }

$ cat f2.cpp 
#include "c.h"

C c2;

$ g++ f1.cpp f2.cpp -nostartfiles
/tmp/ccU01avI.o:/tmp/ccU01avI.o:(.text+0x6c): multiple definition of `virtual thunk to C::~C()'
/tmp/ccwZV0B9.o:/tmp/ccwZV0B9.o:(.text+0x90): first defined here
/tmp/ccU01avI.o:/tmp/ccU01avI.o:(.text+0x122): multiple definition of `virtual thunk to C::~C()'
/tmp/ccwZV0B9.o:/tmp/ccwZV0B9.o:(.text+0x146): first defined here

Note: I use -nostartfiles because crt0.o is not available in my test environment.

This is a regression.
The same testcase has no problem with GCC 4.1.2
Comment 1 Andrew Pinski 2008-02-03 22:35:27 UTC
The virtual thunk should be link once in this case.  Can you attach the assembly files?  Also how did you configure GCC?
Comment 2 Vincent Riviere 2008-02-03 22:57:05 UTC
This problem seems to happen on targets where ASM_WEAKEN_LABEL() is defined, but MAKE_DECL_ONE_ONLY() is not defined. In that case, the `virtual
thunk to C::~C()' symbol is defined using .globl instead of using .weak (like it was in 4.1.2).

I made some investigation. The problem may be related to these source files:
gcc/cp/methods.c in function use_thunk()
  if (DECL_ONE_ONLY (function))
    make_decl_one_only (thunk_fndecl);
This code was different in 4.1.2

gcc/varasm.c in function make_decl_one_only()
  if (SUPPORTS_ONE_ONLY)
    {
#ifdef MAKE_DECL_ONE_ONLY
      MAKE_DECL_ONE_ONLY (decl);
#endif
      DECL_ONE_ONLY (decl) = 1;
    }

When MAKE_DECL_ONE_ONLY is not defined, SUPPORTS_ONE_ONLY is set to 0. So DECL_ONE_ONLY (decl) = 1 is never called.

Thus if the destructor is flagged DECL_ONE_ONLY, the virtual thunk is not flagged DECL_ONE_ONLY as it should, then it is defined with .globl instead of .weak
Comment 3 Vincent Riviere 2008-02-03 23:02:02 UTC
Created attachment 15090 [details]
Output of f1.cpp
Comment 4 Vincent Riviere 2008-02-03 23:03:58 UTC
Created attachment 15091 [details]
Output of f2.cpp

I configured gcc with:
configure --target=i386-netbsd --disable-multilib --disable-nls --enable-languages="c,c++"
Comment 5 Richard Biener 2008-02-04 10:14:05 UTC
Can you please check a recent gcc 4.3 snapshot?
Comment 6 Vincent Riviere 2008-02-04 13:33:20 UTC
Same problem with version 4.3-20080201
Comment 7 Vincent Riviere 2008-02-04 13:38:53 UTC
(In reply to comment #6)
Sorry, I messed up with the versions...
I'm going to check again the version 4.3 right now.
Comment 8 Vincent Riviere 2008-02-04 14:02:48 UTC
I confirm this problem is still present in GCC 4.3-20080201.
Comment 9 Jakub Jelinek 2008-02-05 20:31:09 UTC
This doesn't affect any primary/secondary platforms.
Comment 10 Joseph S. Myers 2008-05-19 20:24:42 UTC
4.2.4 is being released, changing milestones to 4.2.5.
Comment 11 Joseph S. Myers 2009-03-31 20:18:31 UTC
Closing 4.2 branch.
Comment 12 Richard Biener 2009-08-04 12:28:45 UTC
GCC 4.3.4 is being released, adjusting target milestone.
Comment 13 Jason Merrill 2009-11-04 19:56:12 UTC
Subject: Bug 35067

Author: jason
Date: Wed Nov  4 19:55:56 2009
New Revision: 153912

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153912
Log:
	PR c++/35067
	* method.c (use_thunk): Check DECL_WEAK as well as
	DECL_ONE_ONLY.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/abi/thunk5.C
Modified:
    branches/gcc-4_4-branch/gcc/cp/ChangeLog
    branches/gcc-4_4-branch/gcc/cp/method.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 14 Jason Merrill 2009-11-04 20:08:56 UTC
Subject: Bug 35067

Author: jason
Date: Wed Nov  4 20:08:44 2009
New Revision: 153913

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153913
Log:
	PR c++/35067
	* method.c (use_thunk): Check DECL_WEAK as well as
	DECL_ONE_ONLY.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/abi/thunk5.C
Modified:
    branches/gcc-4_3-branch/gcc/cp/ChangeLog
    branches/gcc-4_3-branch/gcc/cp/method.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 15 Jason Merrill 2009-11-04 20:10:24 UTC
Fixed for 4.3.5, 4.4.3, 4.5.0.  Vincent, can you verify that it works with the current sources?
Comment 16 Vincent Riviere 2009-11-04 23:41:32 UTC
Hello, I've just tested with GCC trunk, it seems to be fixed !
Thanks, Jason.