[PATCH] D35639: [ThinLTO] Prevent dead stripping and internalization of symbols with sections

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D35639#815632, @tejohnson wrote:

> In https://reviews.llvm.org/D35639#815591, @pcc wrote:
>
> > In https://reviews.llvm.org/D35639#815587, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D35639#815579, @pcc wrote:
> > >
> > > > In https://reviews.llvm.org/D35639#815578, @tejohnson wrote:
> > > >
> > > > > In https://reviews.llvm.org/D35639#815234, @pcc wrote:
> > > > >
> > > > > > Another possibility is to handle this in the client (i.e. the linker) instead. The client has the best idea of whether a given section is a GC root; ELF linkers can set VisibleToRegularObj on any symbol in a section named like a C identifier, and other linkers can do whatever is appropriate for their object format.
> > > > >
> > > > >
> > > > > I don't see how to get the presence of a section or the section name for a symbol when walking the symbols? I.e. either in gold-plugin via the symbol info from the linker, or in LTO::addModuleToGlobalRes via the InputFile:Symbol?
> > > >
> > > >
> > > > We'd need to add that information to the irsymtab and expose it via InputFile::Symbol.
> > >
> > >
> > > That's what I was afraid of. =(  That seems quite a bit more extensive than simply checking whether the GV has a section and blocking internalization. 
> > >  If we were to go the route you are suggesting, it looks like currently we don't iterate through the InputFile until we invoke the LTO API, when adding a module. I guess we would have to then iterate through those and update the global resolutions earlier, e.g. in the gold-plugin? I.e. in order to make it ELF specific.
> >
> >
> > I don't follow your point about global resolutions, this should be purely a local analysis (from the linker's perspective at least). In both the gold plugin and lld an InputFile is available at the point where we compute the resolution for each symbol, so it should be a matter of copying the property from the InputFile symbol to the resolution.
>
>
> Yeah I meant the SymbolResolution not the GlobalResolution.


Actually, I have a variant of this that I prefer. Instead of setting VisibleToRegularObj in each linker, we could handle it similar to the way some COFF specific behavior is handled. I.e. when building the symbol table, if isOSBinFormatELF, and the symbol has a C identifier section name, then set a (new) flag on the symbol indicating as such (e.g. ELFHasCIdentifierSectionName). Then in LTO::addModuleToGlobalRes, we add a check of this flag to the conditions under which VisibleOutsideSummary is set. That has the advantage of avoiding the need to add checking to each ELF linker, and also doesn't overload the VisibleToRegularObj flag with a condition that isn't being visible to a regular object, but rather visible outside the summary in a different way. Sound ok?


https://reviews.llvm.org/D35639