[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 4:31 PM, Nick Kledzik <kledzik at apple.com> wrote:

> 
> 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.

I’m running into a very similar issue trying to implement -exported_symbols_list option.  I’m working on a patch to add a setScope() option to DefinedAtom.  I’ll post a patch for review Monday and see if we all think this is the way to go for these sorts of problems.

-Nick