[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#815208, @davide wrote:

> In https://reviews.llvm.org/D35639#815191, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D35639#815079, @davide wrote:
> >
> > > Do you know whether LTO gets this case right?
> >
> >
> > It does not get it correct. I could move the fix into the internalize handling under llvm::internalizeModule: that would cover ThinLTO with the new LTO API, and ThinLTO and regular LTO with the legacy LTO API. However, the regular LTO handling in LTO::runRegularLTO does not use internalizeModule. I would either need to duplicate the fix for regular LTO into both the new and legacy LTO handling, or move the fix into internalizeModule and refactor runRegularLTO to use it as well. WDYT?
>
>
> I'm not sure. This is actually exposing a bigger problem that we need to solve sooner rather than later (i.e. the fact there are two APIs)
>  Probably we can duplicate the fix for the time being but we should also have a discussion about it.


Ironically, it is not the different APIs that are an issue - it is the different internalization handling for regular vs Thin LTO in the new API (the former updates the linkage directly, the latter invokes internalizeModule), that would provoke duplication (unless I refactored the former to also use internalizeModule).


https://reviews.llvm.org/D35639