Re: PATCH: Support mixing COMDAT and linkonce

[Alan Modra <amodra at bigpond dot net dot au>]

On Tue, May 18, 2004 at 09:38:28AM -0700, H. J. Lu wrote:
> On Mon, May 17, 2004 at 08:41:50PM -0700, H. J. Lu wrote:
> > On Mon, May 17, 2004 at 06:06:05PM -0700, H. J. Lu wrote:
> > > On Mon, May 17, 2004 at 02:24:23PM -0700, H. J. Lu wrote:
> > > > On Sat, May 15, 2004 at 11:09:47AM +0930, Alan Modra wrote:
> > > > > On Fri, May 14, 2004 at 02:07:37PM -0700, H. J. Lu wrote:
> > > > > > When there are mixed COMDAT and linkonce inputs, linker doesn't handle
> > > > > > them gracefully:
> > > > > 
> > > > > Ewww.  Should we even try?  I understand that such a patch might be
> > > > > useful while gcc is emitting both comdat and linkonce, but once you've
> > > > > completed the change to comdat it shouldn't be necessary.  Also, I'm not
> > > > > really happy with where you have added this code.  At least, it is the
> > > > > wrong place to be discarding duplicate sections.  That ought to happen
> > > > > in ldlang.c:section_already_linked.
> > > > 
> > > > This patch implements it.
> > > 
> > > Here is an update. I should skip checking members of section groups
> > > for already linked section.
> > 
> > Another update. Should I skip COMDAT group members on the already
> > linked list? It may happen if a COMDAT group member has the same
> > section name as a linkonce section.
> 
> I fixed a small memory leak.

Whoa!  HJ, you haven't even answered the basic question of whether the
linker should be doing anything special when confronted with a mix of
COMDAT (new style link-once) and .gnu.linkonce.* (old style link-once)
sections.  It's not clear to me that we should even try to handle this
situation, and what should be done with real groups containing multiple
sections when just one of the sections matches an old style link-once
section.

Secondly, you haven't justified your design.  I think that comparing
symbols exported from a section in order to determine whether a
section is a duplicate is a clever idea, but it doesn't exactly fit
in with the current linker handling of link-once sections.  We currently
just look at section flags and names, discarding sections that have
the SEC_LINK_ONCE flag set and the same name as another SEC_LINK_ONCE
section.  There are some more flags that determine whether the linker
should warn always on discarding, or warn when the section size doesn't
match, or warn when section contents don't match (currently
unimplemented).  Your checking of symbols really fits in with these
warning options.  (And it might be a neat way to implement
SEC_LINK_DUPLICATES_SAME_CONTENTS in the face of needing to support
link-once sections compiled with different gcc optimizations.)

So please spend some time describing why this solution of yours is
necessary, and also why this solution is the best one.  After these
design issues have been discussed, then perhaps we can move on to
looking at implementation details.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre