[PATCH] D43011: [ELF] Create and export symbols provided by a linker script if they referenced by DSOs.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputFiles.h:344
 extern std::vector<InputFile *> SharedFiles;
+extern llvm::DenseSet<StringRef> UndefsInShlibs;
 
----------------
ikudrin wrote:
> grimar wrote:
> > It seems like instead of adding global variable you can make helper that
> > would scan over `SharedFiles` and invoke `SharedFile::getUndefinedSymbols`
> > to build the same set and then pass it to `shouldDefineSym` ?
> We call `shouldDefineSym` not only in `declareSymbols` but also in `addSymbol`, which might be called several times. As a result, the set itself would be created several times.
> 
> Moreover, `SharedFiles` requires a template argument, so all involved methods would become templates too.
> 
> I thought about keeping PROVIDE symbols which are not created during `declareSymbols` call. `SymbolTable::scanShlibUndefined` would then ask `LinkerScript` to create referenced symbols. However, it looks like blurring of responsibility to me.
Probably `getUndefinedSymbols` can be moved to `InputFile` with corresponding member. 
And then it can assert that it is called only for `SharedKind`, like we do in other methods there.
We already moved some functionality to `InputFile` to avoid dealing with templates before.

I do not think building of such set is very expensive and so it should be ok to do that twice. 
In `processSectionCommands` and in `declareSymbols`.
 
It is also seems that after landing D43008 it should be possible to remove call from 'addSymbol`. 
Since `declareSymbols` sets `Cmd->Sym`, I think `addSymbol` can probably do

```
if (!Cmd->Sym)
  return;
```

instead of
```
  if (!shouldDefineSym(Cmd))
    return;
```

I did not try it though. But at least such approach avoids introducing one more global variable
and looks more like a correct direction IMO.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43011