[PATCH] [ELF] LLD does not create a record in the .dynsym if a strong symbol defined in the executable file replaces a weak symbol from a shared library.

[Nick Kledzik]

On Aug 15, 2014, at 2:44 PM, Shankar Easwaran <shankare at codeaurora.org> wrote:

> On 8/15/2014 3:48 PM, Nick Kledzik wrote:
>> Good point.  Rather than a set <const DefinedAtom*>, ELFLinkingContext can just maintain
>> a StringSet.  That is, record the atom’s name instead of its address.  That approach would
>> not work for anonymous (unnamed) atoms or scopeTranslationUnit (static) atoms.  But in
>> this case, the atoms are named and scopeGlobal, so it will work.
> Nick,
> 
> I would prefer the data not saved in the LinkingContext as the whole information is lost on what data was dynamically exported when using --output-filetype=YAML.
> 
> The dynamic export functionality cannot be tested outside the Reader.
> 
> Having a reference type inbuilt into the linker would be much more useful, IMO.
Yes, having the info in the graph would be better than in a side table.

The problem is the nodes in the graph are read-only (expect for the target of edges (References)).

Yes, adding references is a standard technique for adding platform specific info to atoms.  But
that is usually done when the atom is created - not later.  In this case it is particularly cumbersome because either we need a way to add references to atoms or every atom must have an empty reference waiting for this use.  Then we still don’t have a way to modify that reference (only the target is modifiable).  And in this case using references is extra weird because there is already a DefinedAtom method dynamicExport() that already should return what we want.

Maybe we should just bite the bullet and add a setter to DefinedAtom:

    virtual void setDynamicExport(bool);

Update the subclasses of DefineAtom to handle this setter.  
Then use const_cast<DefinedAtom*> to call it.

Since when setDynamicExport() should be called is platform specific, the Resolver still needs a call out to the context.  My previous idea of adding to LinkingContext:
  	void validatedCoalesce(const Atom &existingAtom, const Atom &newAtom, bool &useNew);
still works for that.

-Nick