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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/LTO/LTO.cpp:391
+    // __start_<secname> and __end_<secname> symbols which may be used
+    // elsewhere.
     if (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
----------------
mehdi_amini wrote:
> tejohnson wrote:
> > mehdi_amini wrote:
> > > I don't understand why this is the right place to do this? Why isn't the responsibility of the linker resolution to mark these symbols as externally visible?
> > The __start_ and __end_ symbols for these sections are implicitly generated by the linker. At least in the gold plugin, we may never see the references to these special symbols (e.g. if they were in a regular object not even processed by the plugin). So the symbols that are declared in such sections (provoking the linker's generation of these symbols) need to be treated conservatively. Doing it here covers all ELF linkers.
> But this is the reason we have a resolution API: we shouldn't have such "magic" here.
> If these symbols must be preserved because the linker may add a reference to it later, the responsibility should be on the linker client of the API to pass the right resolution.
I agree with Mehdi, FWIW. I can see the argument for why one might choose to handle this in the API, but it's not the way that I would implement it.


https://reviews.llvm.org/D35639