Bug 3187 - gcc lays down two copies of constructors
gcc lays down two copies of constructors
Status: NEW
Product: gcc
Classification: Unclassified
Component: c++
3.0
: P3 enhancement
: ---
Assigned To: Not yet assigned to anyone
: ABI, missed-optimization
: 13660 13725 (view as bug list)
Depends on:
Blocks: 16996 41090
  Show dependency treegraph
 
Reported: 2001-06-14 11:46 UTC by peter
Modified: 2014-04-01 17:26 UTC (History)
21 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-04-16 22:06:29


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description peter 2001-06-14 11:46:02 UTC
Two (sometimes three) identical copies of constructors and destructors are laid down. The linker doesn't fail this, but the binaries produced are 20% bigger (on our real-world example) than necessary.

Release:
gcc version 3.0 20010614 (prerelease)

Environment:
PC Linux host, ARM Linux cross-compiler

Configured with: ../gcc/configure --prefix=/home/peter --host=i686-linux --build=i686-linux --target=arm-cvs-linux --enable-languages=c,c++ --enable-static

How-To-Repeat:
arm-cvs-linux-g++ -S aubergine.cpp -o aubergine2.s

The aubergine2.s file clearly contains two identical constructors and two identical destructors.
Comment 1 philb 2001-06-14 20:06:17 UTC
From: Philip Blundell <philb@gnu.org>
To: peter@empeg.com
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: c++/3187: gcc-3.0 prerelease lays down two copies of constructors 
Date: Thu, 14 Jun 2001 20:06:17 +0100

 --==_Exmh_-1613115566P
 Content-Type: text/plain; charset=us-ascii
 
 >>How-To-Repeat:
 >arm-cvs-linux-g++ -S aubergine.cpp -o aubergine2.s
 
 This is not much use unless you supply a copy of aubergine.cpp with the bug 
 report.  Or rather the preprocessed source; see http://gcc.gnu.org/bugs.html 
 for instructions.
 
 p.
 
 
 
 --==_Exmh_-1613115566P
 Content-Type: application/pgp-signature
 
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.0.5 (GNU/Linux)
 Comment: Exmh version 2.1.1 10/15/1999 (debian)
 
 iD8DBQE7KQspVTLPJe9CT30RAp7KAJsF+Ye3KBDcc28GQHj1dRWMPT6kcQCgh20a
 Yy/bbj5PqmEgfsHx+utTu5o=
 =56d5
 -----END PGP SIGNATURE-----
 
 --==_Exmh_-1613115566P--
Comment 2 Nathan Sidwell 2001-06-15 00:53:06 UTC
State-Changed-From-To: open->analyzed
State-Changed-Why: This is not a bug, however, it is a pessimization.
    The following ctor/dtors are (sometimes) required
    1) complete object ctor/dtor
    2) base object ctor/dtor
    3) new/delete ctor/dtor
    in the general case 1 & 2 are different, but for many
    classes they are the same. We should do something about
    that.
Comment 3 Nathan Sidwell 2003-02-19 15:16:14 UTC
From: Nathan Sidwell <nathan@codesourcery.com>
To: Steven Bosscher <s.bosscher@student.tudelft.nl>
Cc: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org,
   peter@empeg.com, gcc-prs@gcc.gnu.org
Subject: Re: c++/3187: gcc-3.0 prerelease lays down two copies of constructors
Date: Wed, 19 Feb 2003 15:16:14 +0000

 Steven Bosscher wrote:
 > http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=3187 
 > 
 > 
 > This PR is _really_ old. Last modified over a year and a half ago, so it 
 > should go in feedback.  It would be even better if this bug is fixed 
 > because it's a pretty serious bug IMO.
 no. analyzed is the right state. Why do you think it serious? It does
 not cause miscompilation or reject legal code.
 
 nathan
 
 -- 
 Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
           The voices in my head said this was stupid too
 nathan@codesourcery.com : http://www.cs.bris.ac.uk/~nathan/ : nathan@acm.org
 
 

Comment 4 s.bosscher 2003-02-19 15:59:20 UTC
From: Steven Bosscher <s.bosscher@student.tudelft.nl>
To: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org,
	peter@empeg.com, gcc-prs@gcc.gnu.org
Cc:  
Subject: Re: c++/3187: gcc-3.0 prerelease lays down two copies of constructors
Date: Wed, 19 Feb 2003 15:59:20 +0100

 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=3187
 
 This PR is _really_ old. Last modified over a year and a half ago, so it 
 should go in feedback.  It would be even better if this bug is fixed 
 because it's a pretty serious bug IMO.
 

Comment 5 s.bosscher 2003-02-19 16:21:59 UTC
From: Steven Bosscher <s.bosscher@student.tudelft.nl>
To: Nathan Sidwell <nathan@codesourcery.com>
Cc: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org,
	peter@empeg.com, gcc-prs@gcc.gnu.org
Subject: Re: c++/3187: gcc-3.0 prerelease lays down two copies of
	constructors
Date: 19 Feb 2003 16:21:59 +0100

 Op wo 19-02-2003, om 16:16 schreef Nathan Sidwell:
 > Steven Bosscher wrote:
 > > http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=3187 
 > > 
 > > 
 > > This PR is _really_ old. Last modified over a year and a half ago, so it 
 > > should go in feedback.  It would be even better if this bug is fixed 
 > > because it's a pretty serious bug IMO.
 > no. analyzed is the right state. Why do you think it serious? It does
 > not cause miscompilation or reject legal code.
 
 Nope, that's why I didn't say critical.  I think it's serious because of
 the 20% increase in code size as claimed in the PR.
Comment 6 Andrew Pinski 2003-06-01 19:13:52 UTC
There was patch to fix this but it was never approved.
Comment 7 Andrew Pinski 2003-06-24 00:50:22 UTC
simple example:

struct A{A();};
A::A(){}
Comment 8 Wolfgang Bangerth 2003-06-24 13:46:14 UTC
See also this thread about multiple entry points:
  http://gcc.gnu.org/ml/gcc/2003-06/msg01985.html

W.
Comment 9 Mark Mitchell 2003-06-24 16:23:58 UTC
It would be nice to fix this for 3.4 (by either using multiple entry points, or
something like the Apple patch submitted at one point), but any change that
large would be too much for the 3.3 branch so I've moved the target milestone
for this PR to 3.4.
Comment 10 Glenn Burkhardt 2003-11-25 21:42:14 UTC
see also this thread for problems the duplicate constructors cause gdb:

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

Boy, this one's been open for awhile!!
Comment 11 Andrew Pinski 2003-11-29 01:52:21 UTC
Most likely will not get done for 3.4.
Comment 12 Wolfgang Bangerth 2004-01-12 17:12:37 UTC
*** Bug 13660 has been marked as a duplicate of this bug. ***
Comment 13 Giovanni Bajo 2004-01-14 03:57:36 UTC
Retargeting to 3.4 - RM must decide.
Comment 14 Andrew Pinski 2004-01-14 05:31:56 UTC
I really do not know if this really qualifies as a regression as we changed the ABI to something that 
requires this.
Comment 15 Mark Mitchell 2004-01-16 18:19:53 UTC
I'm changing this to an enhancement request.

It is, in some sense, a regression -- but it's really a direct consequence of
the new ABI.  

Fixing it will require substantial effort.  I hope that we will get a chance to
do the multiple-entry point stuff -- but it's certainly not going to happen
before GCC 3.4.
Comment 16 Giovanni Bajo 2004-01-18 02:45:38 UTC
*** Bug 13725 has been marked as a duplicate of this bug. ***
Comment 17 larue 2005-06-15 21:09:17 UTC
This problem makes it difficult to debug C++ code. This is affecting the 
systemC (www.systemc.org) community. It is more than a minor issue those 
debugging C++.
Comment 18 Andrew Pinski 2005-06-15 21:19:01 UTC
(In reply to comment #17)
> This problem makes it difficult to debug C++ code. This is affecting the 
> systemC (www.systemc.org) community. It is more than a minor issue those 
> debugging C++.
The debuger should be able to handle this as it will also get templates wrong.
Comment 19 Andrew Pinski 2005-11-02 02:07:17 UTC
Supending as there was ABI work that needed to be done to fix this.
Comment 20 Ian Lance Taylor 2006-02-05 00:19:59 UTC
In many common cases, the two constructors are identical.  It should not be hard to simply emit both appropriate symbols before the function.  That does not require any change to the ABI, but it eliminates the problem in the common case.

I don't feel that this PR should be suspended, at least not until we have fixed the common case.
Comment 21 Andrew Pinski 2006-02-05 00:27:17 UTC
Subject: Re:  gcc lays down two copies of constructors


On Feb 4, 2006, at 7:20 PM, ian at airs dot com wrote:

>
>
> ------- Comment #20 from ian at airs dot com  2006-02-05 00:19 -------
> In many common cases, the two constructors are identical.  It should 
> not be
> hard to simply emit both appropriate symbols before the function.  
> That does
> not require any change to the ABI, but it eliminates the problem in 
> the common
> case.

But that does not work for some assemblers/file formats (like Darwin) as
Darwin's as finds subsections via labels.

There has been some discussion on why this suggestion would not
work on the mailing list (but I cannot find it right now).

Thanks,
Andrew Pinski

Comment 22 Ian Lance Taylor 2006-02-05 00:42:30 UTC
The fact that it does not work everywhere is not a valid reason that it should not be implemented where it can work, particularly since the places where it can work are, as it happens, most places.
Comment 23 Andrew Pinski 2006-02-05 00:55:32 UTC
For future reference:
a patch which would fix this: http://gcc.gnu.org/ml/gcc-patches/2002-08/msg00354.html

Previous discussions about this issue:
http://gcc.gnu.org/ml/gcc/2002-12/msg00474.html

More about the patch:
http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00689.html

There is more like other threads too which I cannot find right now.
Comment 24 Gabriel Dos Reis 2006-02-05 03:58:57 UTC
Subject: Re:  gcc lays down two copies of constructors

"ian at airs dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| I don't feel that this PR should be suspended, at least not until we
| have fixed the common case.

I think I agree with Ian.

-- Gaby
Comment 25 Gabriel Dos Reis 2006-02-05 04:00:07 UTC
Subject: Re:  gcc lays down two copies of constructors

"pinskia at physics dot uc dot edu" <gcc-bugzilla@gcc.gnu.org> writes:

| But that does not work for some assemblers/file formats (like Darwin) as
| Darwin's as finds subsections via labels.

then we would just have not to implement what Ian proposes for Darwin.

-- Gaby
Comment 26 Gavin Lambert 2006-09-05 07:18:00 UTC
This is very aggravating, and *NEEDS* to get fixed soon.  Even if only for the common duplicate-symbol-on-most-platforms case, that's a significant improvement over what it's doing now.
Comment 27 William Maddox 2007-05-25 04:57:09 UTC
(In reply to comment #20)

Ian proposed handling the simple case in which the two constructors ("clones") are identical by emitting the code only once, but labelling it with the symbols for both constructors.  This scheme fails in the case that the constructors must go in a comdat group. What do we name the group?  If we invent a new name, this will have ABI impact, as other implementations must not step on it.  If we use the name of one of the constructors, we may find that the section is discarded in favor of a like-named section produced by another compiler, but which need not define both symbols, resulting in link-incompatibility.  It appears, then,
that a distinct group name must be used for the combined constructor.  As I understand it, this was the principal issue that kept Stuart's patch (referenced in comment #23) from being adopted, so I don't see how this more limited optimization sidesteps the objections to that patch.  I spoke with Ian this afternoon, and he agreed with this analysis.  It was suggested in the discussion of Stuart's patch that it would be sufficient to provide for a vendor namespace.
This seems like a simple, general facility that could be leveraged in multiple contexts, e.g., to support a more general subroutinization optimization.  How difficult would it be to get such a provision into the ABI?  With a suitably chosen mangling convention, I don't think that name collisions with other ABI-conformant implementations would actually be a problem in practice, so it seems to be more of a matter of agreeing on a convention for the sake of the ABI standard rather than actually managing the transition.  Comments?

Comment 28 Jason Merrill 2008-02-19 20:20:48 UTC
unsuspending.
Comment 29 Daniel Baldin 2008-07-29 11:32:06 UTC
I wonder if there is anybody working on this bug since there is no target milestone assigned to it. This bug is definitly not an enhancement since due to this bug it is impossible/or very hard to create good software for embedded systems with very limited resources. We are currently working on an rtos designed to work only 64 kb of flash memory and the binary size is drastically increased by this bug. 

This bug is very annoying and should soon be fixed knowing that this bug has been out there for more than 7 years ... 

Greetings
Comment 30 Manuel López-Ibáñez 2008-07-29 13:57:34 UTC
Dear Daniel, we would like to fix all bugs but we cannot force volunteers to fix specific bugs and, on the other hand, hired developers fix those bugs that are most interesting for their employers. 

If this were such a huge issue for the embedded market, surely, some company or consortium would have already hired CodeSourcery or someone else to fix it long time ago.

Greetings.
Comment 31 Jorn Wolfgang Rennecke 2008-08-01 16:07:02 UTC
(In reply to comment #27)
> (In reply to comment #20)
> 
> Ian proposed handling the simple case in which the two constructors ("clones")
> are identical by emitting the code only once, but labelling it with the symbols
> for both constructors.  This scheme fails in the case that the constructors
> must go in a comdat group. What do we name the group?  If we invent a new name,
> this will have ABI impact, as other implementations must not step on it.  If we
> use the name of one of the constructors, we may find that the section is
> discarded in favor of a like-named section produced by another compiler, but
> which need not define both symbols, resulting in link-incompatibility.

link-incompatibility is often of a lesser concern in the embedded world,
where some people are prepared to rebuild all their libraries if they can
save 20% code space this way.
Thus, having an option to use the scheme where the comdat section - if any -
is named after only one of the constructors / destructors would be helpful.
For situations where there are more concerns about link compatibility, I
suppose we could initiate a transitionary period where we put a weak
definition of the *1* con/destructor in the *2* section, and vice versa.
When a user is confident that all the libraries are built at least with this
transitionary scheme, they can turn on a switch to only emit the needed
sections, thus saving at least some space where not all the constructors / destructors are coming from the libraries.
This transitionary period could be ended on a target-by-target basis when
the an ABI change is deemed safe, or extended indefinitely where it is
not considered safe.
Comment 32 Alexandre Pereira Nunes 2008-10-21 18:37:37 UTC
I was considering using C++ for an arm-elf target, but I'm dropping that in favour of plain C because of this silly thing. This sucks, because other than that g++ does a pretty decent job when generating small, optimized code: I've got no abstraction penalty at all, but a few duplicated constructors here and there lead to about 23% code increase, and that wasn't acceptable.

The other funny thing is that if this is fixed, it'll be by 4.4 or something, and so far I can't consider using gcc versions newer than 4.2.x, at least because of PR31849.


Comment 33 Jason Merrill 2008-12-22 03:11:41 UTC
Bill Maddox posted a patch at http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01147.html
Comment 34 Carrot 2009-08-27 01:40:43 UTC
There is one optimization that we can do without affecting the ABI and linker compatibility. The delete destructor(D0) always contains the content of complete desturctor(D1) followed by a function call to delete. So instead of cloning the abstract destructor function body to the delete destructor(D0), we can generate a function call to complete destructor(D1) followed by a function call to delete.
Comment 35 Jakub Jelinek 2009-11-18 09:54:08 UTC
Subject: Bug 3187

Author: jakub
Date: Wed Nov 18 09:53:52 2009
New Revision: 154284

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=154284
Log:
	PR c++/3187
	* cgraph.h (struct cgraph_node): Add same_body and same_body_alias
	fields.
	(cgraph_same_body_alias, cgraph_remove_same_body_alias): New
	prototypes.
	* cgraphunit.c (cgraph_expand_function, cgraph_emit_thunks,
	cgraph_materialize_all_clones): Handle same_body aliases.
	* cgraph.c (cgraph_allocate_node): New function.
	(cgraph_create_node): Use it.
	(cgraph_node_for_decl, cgraph_node, cgraph_get_node,
	cgraph_node_for_asm, cgraph_remove_node): Handle same_body aliases.
	(cgraph_same_body_alias, cgraph_remove_same_body_alias): New
	functions.
	* lto-cgraph.c (lto_output_node): Stream out same_body aliases.
	(input_node): Stream in same_body aliases.
	* lto-symtab.c (lto_cgraph_replace_node): Clear node pointers
	for same_body aliases.
	(lto_symtab_merge_cgraph_nodes_1): Handle same_body aliases.

	* cp-tree.h (expand_or_defer_fn_1): New prototype.
	* decl2.c (cp_write_global_declarations): Mark as !DECL_EXTERNAL
	also all same_body aliases.
	* semantics.c (expand_or_defer_fn): Move most of the function
	except registering with cgraph to ...
	(expand_or_defer_fn_1): ... here.  New function.
	* optimize.c: Include cgraph.h.
	(maybe_clone_body): If in charge parm is not used and both base
	and complete clones are created and are not comdat, tell cgraph
	they have the same body.
	* Make-lang.in (cp/optimize.o): Depend on $(CGRAPH_H).

	* g++.dg/abi/mangle26.C: Also match *C2* definition.
	* g++.dg/abi/mangle27.C: Likewise.
	* g++.dg/abi/mangle28.C: Likewise.
	* g++.dg/abi/mangle29.C: Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cgraph.c
    trunk/gcc/cgraph.h
    trunk/gcc/cgraphunit.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/Make-lang.in
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/decl2.c
    trunk/gcc/cp/optimize.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/lto-cgraph.c
    trunk/gcc/lto-symtab.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/abi/mangle26.C
    trunk/gcc/testsuite/g++.dg/abi/mangle27.C
    trunk/gcc/testsuite/g++.dg/abi/mangle28.C
    trunk/gcc/testsuite/g++.dg/abi/mangle29.C

Comment 36 Jakub Jelinek 2009-12-01 20:09:51 UTC
Subject: Bug 3187

Author: jakub
Date: Tue Dec  1 20:09:37 2009
New Revision: 154880

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=154880
Log:
	PR c++/3187
	* optimize.c (cdtor_comdat_group): New function.
	(maybe_clone_body): Also optimize DECL_COMDAT base/complete cdtors
	and in that case put also the deleting dtor in the same comdat group
	as base and complete dtor if dtor is virtual.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/optimize.c

Comment 37 Jakub Jelinek 2009-12-02 14:31:54 UTC
Subject: Bug 3187

Author: jakub
Date: Wed Dec  2 14:31:21 2009
New Revision: 154912

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=154912
Log:
Fix a backport glitch for PR c++/3187.

Modified:
    branches/redhat/gcc-4_4-branch/gcc/cp/ChangeLog
    branches/redhat/gcc-4_4-branch/gcc/cp/decl2.c

Comment 38 Arunprasad 2012-10-18 18:01:14 UTC
from which version this fix will be available?
Comment 39 Paolo Carlini 2012-10-18 23:01:59 UTC
For sure I can see the Nov 2009 commit in the 4.5.0 ChangeLog, but it would be indeed nice if either Jakub or Jason could write a few words in the audit trail here, explaining why the PR is still open and what is still missing.
Comment 40 Arunprasad 2012-10-19 06:41:04 UTC
Thank you.Is there any way to find it from nm output.?
Comment 41 Jakub Jelinek 2012-10-19 10:05:55 UTC
The reason why this hasn't been closed is that we only use an alias of one kind of ctor (resp. dtor) to the other one if they are the same (and for deleting dtor just always call the other dtor).  If they are different, then they can't be aliased together, what we could do (perhaps as an option) is to emit another function, which would take an argument what kind of ctor resp. dtor it is, and
behave differently depending on that argument, then have both kinds of ctor resp. dtor to tail call (or if not possible, just call) the other function.
I guess it could have ABI consequences though (in which comdat section to place
it).
Comment 42 Arunprasad 2012-10-19 10:34:11 UTC
So I'm assuming like the issue still exists in gcc family of tool-chains. Fix has been temporarily suspended due to ABI compatibility.