This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: new linker option for shared library COMDAT optimization

On Thu, Mar 01, 2012 at 09:39:27AM -0700, Sandra Loosemore wrote:
> * The ref_regular and ref_dynamic bits on the symbol don't
> distinguish between references within the group (or within another
> group that is a candidate for removal), and "real" references.  So,
> without some more sophisticated form of symbol GC, this isn't likely
> to produce results that are transparent either to users or library
> implementors.

Yes, it's not as easy as it could be.  See the comment on ref_regular,
which means each backend would need to set this flag in its
check_relocs function to catch all refs.  A change to
RELOC_FOR_GLOBAL_SYMBOL would work for most.

> * Consider the degenerate case when none of the symbols defined in a
> group are referenced.  Throwing the whole group away seems like the
> wrong thing -- e.g., consider that you're building a shared library
> that specifically wants to provide and export some template
> instantiations to its consumers, and is itself built with
> --shared-comdat.

You aren't throwing the whole group away, just using a local copy
instead of some group in another shared library.  In fact, the more
strict you make the symbol matching, the more likely any given group
will behave as if --shared-comdat wasn't given.  I'm not pushing to
limit your symbol checks to strictly correct "real" references, just
those symbols where either ref_dynamic or ref_regular (with the fix
above) are set.  Finding out whether that works shouldn't take too

> * In the original version of the patch I posted previously, I'd left
> some debugging hooks in place.  One of them printed a message if it
> found a partial match -- some of the symbols defined in a comdat
> group were provided by a shared library, but not all of them.  In
> practice, when I had the debugging enabled I never once saw that
> condition triggered in all the testing I did, so I suspect this is
> really a corner case and not critical functionality.

Did you try many C++ testcases?  I chose to look at libstdc++ with my
nm hack for good reason.  libstdc++ uses -gc-sections and visibility,
and so is likely to run into mismatches when comparing dynamic symbols
to those in object files.

> The more I think about it, allowing for a library to provide only a
> subset of symbols in a group just seems like a Bad Idea, and in
> violation of the spirit that groups are supposed to be manipulated
> as a unit.  So I'd like to drop this suggestion, and just go with
> the documentation change to my original patch instead.  OK?
> -Sandra

Alan Modra
Australia Development Lab, IBM

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]