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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

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

> In https://reviews.llvm.org/D35639#816132, @tejohnson wrote:
>
> > 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?
>
>
> Note in addModuleToGlobalRes it is also added to the set of conditions where we need to set the Partition to External (similar to the places where we currently check things like VisibleToRegularObj. I have this working for a small test case, testing with my full internal test case. If that works I will clean up and send for review. I like this since it is ELF specific but doesn't require any linker changes.


Sounds fine to me.

Note that this design would preclude the linker from doing anything smarter with VisibleToRegularObj, like setting it depending on whether the input files contain an undefined reference to the `__start_` or `__stop_` symbols. We may also need section names in order to properly support certain linker script features with LTO. But I guess we can always change the design if any of that turns out to be necessary/useful.


https://reviews.llvm.org/D35639