[LTO] Prevent devirtualization for symbols dynamically exported

[tejohnson]

Summary

Identify dynamically exported symbols (--export-dynamic[-symbol=],
--dynamic-list=, or definitions needed to preempt shared objects) and
prevent their LTO visibility from being upgraded.
This helps avoid use of whole program devirtualization when there may
be overrides in dynamic libraries.

[MaskRay]

I'll need to study what whole program visibility does (e.g. I need to read http://lists.llvm.org/pipermail/llvm-dev/2019-December/137543.html "RFC: Safe Whole Program Devirtualization Enablement").
My gut feeling is that: --export-dynamic is not sufficient to capture whether symbols are exported.
When linking an executable,

• Symbols matched by a --dynamic-list pattern are exported to the dynamic symbol table
• Symbols matched by a --export-dynamic-symbol pattern exported to the dynamic symbol table
• Symbols defined in the executable which are referenced by a shared object are exported.

If the usage of --export-dynamic means whether the mode should apply to synthesized new symbols, I think having a mode referring to --export-dynamic makes sense.

[tejohnson]

In D91583#2398638, @MaskRay wrote:

I'll need to study what whole program visibility does (e.g. I need to read http://lists.llvm.org/pipermail/llvm-dev/2019-December/137543.html "RFC: Safe Whole Program Devirtualization Enablement").
My gut feeling is that: --export-dynamic is not sufficient to capture whether symbols are exported.
When linking an executable,

• Symbols matched by a --dynamic-list pattern are exported to the dynamic symbol table
• Symbols matched by a --export-dynamic-symbol pattern exported to the dynamic symbol table
• Symbols defined in the executable which are referenced by a shared object are exported.

If the usage of --export-dynamic means whether the mode should apply to synthesized new symbols, I think having a mode referring to --export-dynamic makes sense.

Not just synthesized new symbols. It determines whether we can convert the LTO visibility of vtables to internal visibility. The intention was that it could be applied when you are doing static linking of a binary (without shared libraries on the link line). The problem is that --export-dynamic suggests that the binary might be intending to dlopen some shared libraries that weren't necessarily linked against. It is true that --export-dynamic-symbol and --dynamic-list can have this effect, but in a much more limited fashion. The goal is to be able to apply --lto-whole-program-visibility widely based on the linking mode, but then automatically disable it in situations where it is likely problematic.

[MaskRay]

Hi, I am still learning the feature and I've just played a bit with the test. I've a couple of questions:

• The --export-dynamic usage in the test seems a bit confusing. Are the VisibleToRegularObj bits of these _ZTV* symbols the important matter and --export-dynamic is just an approach to make them true?
• If yes, I guess all these --export-dynamic can be replaced with -u _ZTV1B -u _ZTV1C -u _ZTV1D in all the RUN lines. --export-dynamic is preferred just due to its brevity. If that is the case, I think this deserves a comment considering its subtle interaction with noexportdynamic.
• When is devirtualization invalid? For example, if _ZTV1D is exported to the dynamic symbol table and a shared object inherits from class D and overrides the method?

If my last point is correct, I'd agree that we probably need something like a tri-state option. (I hope we can remove --lto-whole-program-visibility if possible)
on/noexportdynamic the value names do not capture the actual meaning. The on actually means: devirtualization is safe as long as the used _ZTV* symbols are not exported.

[tejohnson]

In D91583#2401100, @MaskRay wrote:

Hi, I am still learning the feature and I've just played a bit with the test. I've a couple of questions:

• The --export-dynamic usage in the test seems a bit confusing. Are the VisibleToRegularObj bits of these _ZTV* symbols the important matter and --export-dynamic is just an approach to make them true?

Correct. It was just a shorthand way of preventing these symbols from being eliminated.

• If yes, I guess all these --export-dynamic can be replaced with -u _ZTV1B -u _ZTV1C -u _ZTV1D in all the RUN lines. --export-dynamic is preferred just due to its brevity. If that is the case, I think this deserves a comment considering its subtle interaction with noexportdynamic.

Right, that's why for one of the tests where I'm using the new noexportdynamic value I switched to the -u sequence instead. I can add a comment.

• When is devirtualization invalid? For example, if _ZTV1D is exported to the dynamic symbol table and a shared object inherits from class D and overrides the method?

Correct. If it is both exported and then overridden.

If my last point is correct, I'd agree that we need something like a tri-state option. (I hope we can remove --lto-whole-program-visibility if possible)
on/noexportdynamic the value names do not capture the actual meaning. The on actually means: devirtualization is safe as long as the used _ZTV* symbols are not exported.

Suggestion on the name? It's basically an assertion, i.e. that there is LTO whole program visibility (no, when --export-dynamic not specified, and yes). 'on' means the user is asserting that the _ZTV* are not going to be exported and overridden.

[MaskRay]

Thanks for the clarification.

In D91583#2401116, @tejohnson wrote:

In D91583#2401100, @MaskRay wrote:

Hi, I am still learning the feature and I've just played a bit with the test. I've a couple of questions:

• The --export-dynamic usage in the test seems a bit confusing. Are the VisibleToRegularObj bits of these _ZTV* symbols the important matter and --export-dynamic is just an approach to make them true?

Correct. It was just a shorthand way of preventing these symbols from being eliminated.

• If yes, I guess all these --export-dynamic can be replaced with -u _ZTV1B -u _ZTV1C -u _ZTV1D in all the RUN lines. --export-dynamic is preferred just due to its brevity. If that is the case, I think this deserves a comment considering its subtle interaction with noexportdynamic.

Right, that's why for one of the tests where I'm using the new noexportdynamic value I switched to the -u sequence instead. I can add a comment.

• When is devirtualization invalid? For example, if _ZTV1D is exported to the dynamic symbol table and a shared object inherits from class D and overrides the method?

Correct. If it is both exported and then overridden.

If my last point is correct, I'd agree that we need something like a tri-state option. (I hope we can remove --lto-whole-program-visibility if possible)
on/noexportdynamic the value names do not capture the actual meaning. The on actually means: devirtualization is safe as long as the used _ZTV* symbols are not exported.

Suggestion on the name? It's basically an assertion, i.e. that there is LTO whole program visibility (no, when --export-dynamic not specified, and yes). 'on' means the user is asserting that the _ZTV* are not going to be exported and overridden.

I have a further question: is it realistic to add another bit along with VisibleToRegularObj to convey the information whether a symbol is includeInDynsym()?
This way virtual functions related to individual _ZTV* can be safely devirtualized, no matter how users specify --export-dynamic-symbol,--export-dynamic,--dynamic-list or add link-time shared objects to alter the includeInDynsym() state of _ZTV* symbols.

If a per-symbol bit is not realistic, I think a tri-state --lto-whole-program-visibility makes sense. However, the meaning is still subtle and it deserves some more explanation in the documentation. (perhaps https://clang.llvm.org/docs/LTOVisibility.html plus a section in docs/ld.lld.1)

For off, a more conventional name is none (--icf=none, --build-id=none)
For on, I have a tentative suggestion: assume-no-exported-vtable. The name still does not capture the concept that "if this symbol is not devirtualized, whether it is exported" does not matter.
For noexportdynamic, I am actually wondering whether we really need to make it different from on.

[tejohnson]

In D91583#2401149, @MaskRay wrote:

Thanks for the clarification.

In D91583#2401116, @tejohnson wrote:

In D91583#2401100, @MaskRay wrote:

Hi, I am still learning the feature and I've just played a bit with the test. I've a couple of questions:

• The --export-dynamic usage in the test seems a bit confusing. Are the VisibleToRegularObj bits of these _ZTV* symbols the important matter and --export-dynamic is just an approach to make them true?

Correct. It was just a shorthand way of preventing these symbols from being eliminated.

• If yes, I guess all these --export-dynamic can be replaced with -u _ZTV1B -u _ZTV1C -u _ZTV1D in all the RUN lines. --export-dynamic is preferred just due to its brevity. If that is the case, I think this deserves a comment considering its subtle interaction with noexportdynamic.

Right, that's why for one of the tests where I'm using the new noexportdynamic value I switched to the -u sequence instead. I can add a comment.

• When is devirtualization invalid? For example, if _ZTV1D is exported to the dynamic symbol table and a shared object inherits from class D and overrides the method?

Correct. If it is both exported and then overridden.

If my last point is correct, I'd agree that we need something like a tri-state option. (I hope we can remove --lto-whole-program-visibility if possible)
on/noexportdynamic the value names do not capture the actual meaning. The on actually means: devirtualization is safe as long as the used _ZTV* symbols are not exported.

Suggestion on the name? It's basically an assertion, i.e. that there is LTO whole program visibility (no, when --export-dynamic not specified, and yes). 'on' means the user is asserting that the _ZTV* are not going to be exported and overridden.

I have a further question: is it realistic to add another bit along with VisibleToRegularObj to convey the information whether a symbol is includeInDynsym()?
This way virtual functions related to individual _ZTV* can be safely devirtualized, no matter how users specify --export-dynamic-symbol,--export-dynamic,--dynamic-list or add link-time shared objects to alter the includeInDynsym() state of _ZTV* symbols.

I looked into this some more and it turns out that vtables themselves don't actually need to be exported to be overridden, so this won't be more accurate. So I think a tri-state is the best option, with the new value tied to the --export-dynamic being a strong signal that vtables could be overridden.

If a per-symbol bit is not realistic, I think a tri-state --lto-whole-program-visibility makes sense. However, the meaning is still subtle and it deserves some more explanation in the documentation. (perhaps https://clang.llvm.org/docs/LTOVisibility.html plus a section in docs/ld.lld.1)

Ok let me come up with something and add it to this patch.

For off, a more conventional name is none (--icf=none, --build-id=none)

ok

For on, I have a tentative suggestion: assume-no-exported-vtable. The name still does not capture the concept that "if this symbol is not devirtualized, whether it is exported" does not matter.

I'd prefer not to tie this to vtables specifically, in the case that we want to apply the whole program visibility concept beyond vtables in the future. Maybe 'always'?

For noexportdynamic, I am actually wondering whether we really need to make it different from on.

I think we do want to keep these separate because it would be good to have a mode to force this on if any cases pop up where --export-dynamic is used but it doesn't actually violate any requirements for the whole program optimization. I anticipate that it's best to be conservative under that case, but for some binaries it may be ok if they are carefully vetted.

[MaskRay]

I have a further question: is it realistic to add another bit along with VisibleToRegularObj to convey the information whether a symbol is includeInDynsym()?
This way virtual functions related to individual _ZTV* can be safely devirtualized, no matter how users specify --export-dynamic-symbol,--export-dynamic,--dynamic-list or add link-time shared objects to alter the includeInDynsym() state of _ZTV* symbols.

Should a new bit be introduced? If the executable defines a virtual class A which is overridden by a link-time shared object, the vtable symbol will be exported. It is a misoptimization if LTO considers devirtualizes A's member functions. How does the --lto-whole-program-visibility design deal with this pitfall?

I looked into this some more and it turns out that vtables themselves don't actually need to be exported to be overridden, so this won't be more accurate. So I think a tri-state is the best option, with the new value tied to the --export-dynamic being a strong signal that vtables could be overridden.

[pcc]

I've said before that I think that --lto-whole-program-visibility should relax visibility of vtable symbols etc to hidden. That way, --export-dynamic wouldn't actually allow you to make this kind of mistake.

[tejohnson]

In D91583#2404519, @pcc wrote:

I've said before that I think that --lto-whole-program-visibility should relax visibility of vtable symbols etc to hidden. That way, --export-dynamic wouldn't actually allow you to make this kind of mistake.

That would presumably result in an error in some of the problematic cases, whereas here we want to simply suppress --lto-whole-program-visibility to avoid any issues automatically.

But isn't it the case that you don't even need for the vtable symbol itself to be exported in order to derive from the class and override its virtual methods?

[pcc]

In D91583#2404678, @tejohnson wrote:

In D91583#2404519, @pcc wrote:

I've said before that I think that --lto-whole-program-visibility should relax visibility of vtable symbols etc to hidden. That way, --export-dynamic wouldn't actually allow you to make this kind of mistake.

That would presumably result in an error in some of the problematic cases, whereas here we want to simply suppress --lto-whole-program-visibility to avoid any issues automatically.

But isn't it the case that you don't even need for the vtable symbol itself to be exported in order to derive from the class and override its virtual methods?

That's true, but it's the same situation that we have now with --lto-whole-program-visibility and not passing --export-dynamic.

I thought that --lto-whole-program-visibility was basically intended to do the same thing as specifying __attribute__((visibility("hidden"))) on classes, except at link time. That isn't fundamentally incompatible with --export-dynamic (or, for that matter, -shared) since you can always expose an interface that doesn't involve the classes.

[tejohnson]

In D91583#2406005, @pcc wrote:

In D91583#2404678, @tejohnson wrote:

In D91583#2404519, @pcc wrote:

I've said before that I think that --lto-whole-program-visibility should relax visibility of vtable symbols etc to hidden. That way, --export-dynamic wouldn't actually allow you to make this kind of mistake.

That would presumably result in an error in some of the problematic cases, whereas here we want to simply suppress --lto-whole-program-visibility to avoid any issues automatically.

But isn't it the case that you don't even need for the vtable symbol itself to be exported in order to derive from the class and override its virtual methods?

That's true, but it's the same situation that we have now with --lto-whole-program-visibility and not passing --export-dynamic.

Right, just confirming my understanding.

I thought that --lto-whole-program-visibility was basically intended to do the same thing as specifying __attribute__((visibility("hidden"))) on classes, except at link time. That isn't fundamentally incompatible with --export-dynamic (or, for that matter, -shared) since you can always expose an interface that doesn't involve the classes.

Correct. It's an assertion. The goal of --lto-whole-program-visibility was to apply it uniformly when the build system believes it is doing static linking, but in some occasional cases a binary may require --export-dynamic and it is simpler to automatically fall back to non-lto-whole-program-visibility in that case.

[tejohnson]

While I investigate alternative mechanisms to handle --export-dynamic, I've sent D92060 to add a --no- version of the option, to simplify working around issues when --lto-whole-program-visibility is specified broadly.

[tejohnson]

I implemented @MaskRay's suggestion and added a bit to convey whether a symbol is exported to the dynamic linker (via --export-dynamic[-symbol=] or --dynamic-list=), and use that to prevent the LTO visibility upgrade for WPD. I added support to both lld and gold plugin, and associated tests. Note that I couldn't use includeInDynsym in lld because that is not set for linkonce_odr symbols that were thus have canBeOmittedFromSymbolTable set (since any referencing module must have it's own copy) - we still want to block the LTO visibility upgrade for those symbols to avoid WPD. So I am using a slightly different interface that more directly checks whether export-dynamic is in effect.

[tejohnson]

Update per previous comment

[tejohnson]

In D91583#2475421, @tejohnson wrote:

I implemented @MaskRay's suggestion and added a bit to convey whether a symbol is exported to the dynamic linker (via --export-dynamic[-symbol=] or --dynamic-list=), and use that to prevent the LTO visibility upgrade for WPD. I added support to both lld and gold plugin, and associated tests. Note that I couldn't use includeInDynsym in lld because that is not set for linkonce_odr symbols that were thus have canBeOmittedFromSymbolTable set (since any referencing module must have it's own copy) - we still want to block the LTO visibility upgrade for those symbols to avoid WPD. So I am using a slightly different interface that more directly checks whether export-dynamic is in effect.

ping - @MaskRay can you take a look?

[MaskRay]

Sorry for the delay. I did not review much over Christmas. I'll try to get to this sometime in the next couple of days.

[MaskRay]

Generally looks good.

lld/ELF/LTO.cpp

253

sym->exportDynamic || sym->inDynamicList

Then isExportDynamic does not need to be public.

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

2

Nit: in LLD tests we use ;; to differentiate regular comments from CHECK RUN markers.

43

%s -> /dev/null

lld/test/ELF/lto/devirt_vcall_vis_public.ll

8

s/\t/ /

9

s/\t/ /

The two lines can be joined.

llvm/include/llvm/LTO/LTO.h

466

How about VisibleToOtherModules?

The name VisibleToDynamicLinker is too tied to the ELF binary format.

[tejohnson]

lld/ELF/LTO.cpp

253

sym->exportDynamic is false for linkonce_odr vtables, that was what I was referencing in this comment (otherwise I could use includeInDynsym which checks that):

Note that I couldn't use includeInDynsym in lld because that is not set for linkonce_odr symbols that were thus have canBeOmittedFromSymbolTable set (since any referencing module must have it's own copy) - we still want to block the LTO visibility upgrade for those symbols to avoid WPD. So I am using a slightly different interface that more directly checks whether export-dynamic is in effect.

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

2

Can I do this in a follow on NFC commit? Otherwise it will make the diffs really noisy in this test.

llvm/include/llvm/LTO/LTO.h

466

VisibleToOtherModules sounds to me like it means LLVM Modules that are being linked together statically. I wanted to note that these are symbols that may have dynamic references not seen by the static link. Is there anything like --export-dynamic for other binary formats? If not, then it is ELF specific anyway.

[tejohnson]

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

43

Fixed here and elsewhere - but not sure why it matters?

lld/test/ELF/lto/devirt_vcall_vis_public.ll

8

Done here and elsewhere

9

ditto

[tejohnson]

Address comments

[MaskRay]

llvm/include/llvm/LTO/LTO.h

466

@rnk for thoughts on COFF.

[tejohnson]

Rebase and use ";;" instead of ";" for comments.

[tejohnson]

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

2

I went ahead and fixed this in the existing tests in 5b42fd8dd4e7e29125a09a41a33af7c9cb57d144. I have updated this with a rebased version that fixes the comments in the new tests as well.

[MaskRay]

lld/ELF/LTO.cpp

253

Sorry, I don't understand the difference. If I replace this with sym->exportDynamic, I don't get a test failure...

llvm/include/llvm/LTO/LTO.h

466

Perhaps another name is Exported.

For ELF, the does not seem to be restricted to shared objects seen as in the input file.

[tejohnson]

lld/ELF/LTO.cpp

253

Ah, this is a test deficiency, looks like I need to make one or more of the vtables linkonce_odr to expose it. Will address that. The reason it is an issue for linkonce_odr can be seen in createBitcodeSymbol in lld/ELF/InputFiles.cpp, where it does:

if (canOmitFromDynSym)
  newSym.exportDynamic = false;

The canOmitFromDynSym gets propagated via the input file but is originally set in GlobalValue::canBeOmittedFromSymbolTable() for linkonce_odr with hasAtLeastLocalUnnamedAddr().

llvm/include/llvm/LTO/LTO.h

466

"Exported" is ambiguous - we use that throughout ThinLTO to mean exported from the current module (to other modules being LTO linked).

For ELF, the does not seem to be restricted to shared objects seen as in the input file.

Sorry I don't follow?

[tejohnson]

Use linkonce_odr vtables to illustrate issue with Symbol exportDynamic

lld/ELF/LTO.cpp

253

I've improved the tests. Confirmed that the improved lld test fails if you make the change you proposed.

[tejohnson]

ping

[MaskRay]

lld/ELF/LTO.cpp

253

I see that sym->isExportDynamic is used to prevent canOmitFromDynSym (unnamed_addr linkonce_odr or local_unnamed_addr linkonce_odr constant) logic.

There is one case where sym->isExportDynamic(sym->kind(), sym->visibility) may be false while sym->exportDynamic is true: a shared object with a STV_DEFAULT reference to the symbol can set exportDynamic (InputFiles.cpp:1557).

sym->isExportDynamic(...) || sym->exportDynamic should be safe.

llvm/include/llvm/LTO/LTO.h

466

Seems that the idea is just whether the symbol is exported and can be used by other linked images. A dynamic linker is the ELF concept but the idea can be used by other binary formats. In COFF it is called "export table". In Mach-O there are linker options -exported_*, and non-exported symbols are converted to private externs.

ThinLTO has already used exported to mean symbols exchanged among LLVM modules so export should not be used. Perhaps just exportDynamic? The name stills stems from ELF but users from other binary formats can still find similarity.

[MaskRay]

lld/ELF/LTO.cpp

253

The comprehensive rule for when exportDynamic is set:

* non-local `STV_DEFAULT/STV_PROTECTED` (this means it can be hid by `--exclude-libs`)
* logical OR of the following:
  + undefined
  + (`--export-dynamic` || `-shared`) && ! (unnamed_addr linkonce_odr GlobalVariable || local_unnamed_addr linkonce_odr constant GlobalVariable)
  + matched by `--dynamic-list/--export-dynamic-symbol-list/--export-dynamic-symbol`
  + defined or referenced by a shared object as `STV_DEFAULT`
  + `STV_PROTECTED` definition in a shared object preempted by copy relocation/canonical PLT when `--ignore-{data,function}-address-equality}` is specified
  + `-z ifunc-noplt` && has at least one relocation

The last two are edge cases (but works if you use sym->exportDynamic).

About the common case "defined or referenced by a shared object as STV_DEFAULT":
for ld.lld %t.o %t1.so -o %t, if %t1.so defines (linkonce_odr) the vtable, it can be preempted by the executable definition. %t thus needs to export the vtable.

This may deserve a test (sym->isExportDynamic(...) || sym->exportDynamic and `sym->isExportDynamic(...) have different behaviors)

[tejohnson]

lld/ELF/LTO.cpp

253

I see, so essentially sym->isExportDynamic(...) and sym->exportDynamic are non-overlapping and neither is a superset of the either. I will change the code to check them both. In terms of creating a test, can any of the cases where the latter is true but the former is not be triggered for a vtable? Since this is only being used for WPD right now, I'd need to be able to test this with a vtable def visible to a virtual call that would otherwise be devirtualized.

llvm/include/llvm/LTO/LTO.h

466

Sounds good, will change to simply ExportDynamic.

[MaskRay]

lld/ELF/LTO.cpp

253

llc -filetype=obj -relocation-model=pic %t.ll -o %t.o
lld -shared %t.o -o %t.so

The output has a dynamic symbol _ZTV1B.

[tejohnson]

Rename VisibleToDynamicLinker to ExportDynamic

lld/ELF/LTO.cpp

253

sym->isExportDynamic(...) already returns true if config->shared, so in the library link it should already be handled by the current patch, or am I misunderstanding?

[MaskRay]

lld/ELF/LTO.cpp

253

llc -filetype=obj -relocation-model=pic %t.ll -o %t.o
ld.lld -shared %t.o -o %t.so
ld.lld %t.o %t.so -o %t

The ELF semantic is that %t.o preempts every default visibility definition in %t.so. So, even in the absence of --export-dynamic/--dynamic-list/--export-dynamic-symbol, the definitions in %t need to be exported (to .dynsym) to allow preemption at runtime.

This is a case where sym->isExportDynamic(...) is false while sym->exportDynamic is true.

(sym->isExportDynamic(...) is true while sym->exportDynamic is false should be impossible, so no need for a test.)

[tejohnson]

lld/ELF/LTO.cpp

253

llc -filetype=obj -relocation-model=pic %t.ll -o %t.o
ld.lld -shared %t.o -o %t.so
ld.lld %t.o %t.so -o %t

The ELF semantic is that %t.o preempts every default visibility definition in %t.so. So, even in the absence of --export-dynamic/--dynamic-list/--export-dynamic-symbol, the definitions in %t need to be exported (to .dynsym) to allow preemption at runtime.

This is a case where sym->isExportDynamic(...) is false while sym->exportDynamic is true.

Ok let me create a test for this.

(sym->isExportDynamic(...) is true while sym->exportDynamic is false should be impossible, so no need for a test.)

It can happen which is why I added the check for sym->isExportDynamic(...) and not sym->exportDynamic in the first place. The current test case (with the linkonce_odr vtables) is exactly this case, because of canOmitFromDynSym (this was what I was describing upthread).

[MaskRay]

lld/ELF/LTO.cpp

253

Ah yes, I forgot again that sym->exportDynamic can be set to false due to canOmitFromDynSym (ThinLTO auto hiding) logic.

[tejohnson]

Also check sym->exportDynamic and add test

[tejohnson]

I think I've addressed all the comments now.

lld/ELF/LTO.cpp

253

Added check of sym->exportDynamic and added test case (confirmed it fails because we get devirtualizations without this change).

[MaskRay]

LG. There are several minor required comment/description changes and test updates.

Under --export-dynamic[-symbol=] and --dynamic-list=, identify the exported symbols and prevent their LTO visibility from being upgraded.

The description needs a update now, something like: Identify the exported symbols (--export-dynamic[-symbol=], --dynamic-list=, or definitions needed to preempt shared objects)

This helps avoid use of whole program devirtualization when there may be overrides in dynamically loaded libraries.

Perhaps just dynamic libraries. "dynamically loaded libraries" gives me a feeling of dlopen, but the issue can arise with regular link-time shared objects as well: it is valid for a new version shared object to add more symbols (derived classes with vtables).

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

3

This needs an update (due to a new case: shared object preemption): exported symbols prevent devirtualization.

102

108

[MaskRay]

[LTO] Prevent devirtualization for symbols exported to dynamic linker

And the subject ("dynamic linker") needs a change

[tejohnson]

Address comments

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

102

Woops, good catch. I was testing the cases one at a time and missed fixing all the RUN lines again.

[lanza]

@tejohnson I'm not sure this change is working correctly -- either that or my builds are messed up.

BitcodeCompiler::add builds the resolutions for a bitcode module's symbols. For all symbols in the module r.ExportDynamic is set via isExportDynamic:

static bool isExportDynamic(Kind k, uint8_t visibility) {
  if (k == SharedKind)
    return visibility == llvm::ELF::STV_DEFAULT;
  return config->shared || config->exportDynamic;
}

The Kind for a BitcodeFile symbol is not SharedKind and thus config->shared causes this to always come back true. This is then used for GlobalResolutions.ExportDynamic which is used to build the DynamicExportSymbols list. Thus the DynamicExportSymbols list contains all the bitcode module's symbols and nothing gets vcall_visibility.

[tejohnson]

In D91583#2610509, @lanza wrote:

@tejohnson I'm not sure this change is working correctly -- either that or my builds are messed up.

BitcodeCompiler::add builds the resolutions for a bitcode module's symbols. For all symbols in the module r.ExportDynamic is set via isExportDynamic:

static bool isExportDynamic(Kind k, uint8_t visibility) {
  if (k == SharedKind)
    return visibility == llvm::ELF::STV_DEFAULT;
  return config->shared || config->exportDynamic;
}

The Kind for a BitcodeFile symbol is not SharedKind and thus config->shared causes this to always come back true. This is then used for GlobalResolutions.ExportDynamic which is used to build the DynamicExportSymbols list. Thus the DynamicExportSymbols list contains all the bitcode module's symbols and nothing gets vcall_visibility.

To me this is WAI. Why is "config->shared" true for your bitcode module? This should only affect when using the linker flags that assert you have whole program visibility during the link, which isn't true for a shared library and its symbols.

[lanza]

To me this is WAI. Why is "config->shared" true for your bitcode module? This should only affect when using the linker flags that assert you have whole program visibility during the link, which isn't true for a shared library and its symbols.

Got ya. For our Android apps we compute the actual import and export lists exactly and thus can compute the symbol visibility during linking and use --lto-whole-program-visibility accordingly. (Though this is not yet used in production for build system reasons). This change makes the list of symbols equivalent to the list of DynamicExportSymbols, so even though we can tell lld that _ZTVN3xyz is internal-only it won't get vcall_visibility.

[MaskRay]

In D91583#2612405, @lanza wrote:

To me this is WAI. Why is "config->shared" true for your bitcode module? This should only affect when using the linker flags that assert you have whole program visibility during the link, which isn't true for a shared library and its symbols.

Got ya. For our Android apps we compute the actual import and export lists exactly and thus can compute the symbol visibility during linking and use --lto-whole-program-visibility accordingly. (Though this is not yet used in production for build system reasons). This change makes the list of symbols equivalent to the list of DynamicExportSymbols, so even though we can tell lld that _ZTVN3xyz is internal-only it won't get vcall_visibility.

The sym->isExportDynamic(sym->kind(), sym->visibility) || sym->exportDynamic || sym->inDynamicList condition is a bit conservative.
I sent D98220 to allow WPD with hidden/internal symbols.

[tejohnson]

In D91583#2612405, @lanza wrote:

To me this is WAI. Why is "config->shared" true for your bitcode module? This should only affect when using the linker flags that assert you have whole program visibility during the link, which isn't true for a shared library and its symbols.

Got ya. For our Android apps we compute the actual import and export lists exactly and thus can compute the symbol visibility during linking and use --lto-whole-program-visibility accordingly. (Though this is not yet used in production for build system reasons). This change makes the list of symbols equivalent to the list of DynamicExportSymbols, so even though we can tell lld that _ZTVN3xyz is internal-only it won't get vcall_visibility.

I see - so just to confirm, when compiling it isn't clear that these symbols are have internal or hidden visibility, but only during linking? Because otherwise clang should already have applied an appropriate vcall_visibility that allows WPD.

[lanza]

I see - so just to confirm, when compiling it isn't clear that these symbols are have internal or hidden visibility, but only during linking? Because otherwise clang should already have applied an appropriate vcall_visibility that allows WPD.

Correct, when compiling we don't know what's exported and what's not, so nothing is applied. This is fundamentally similar to a problem you mentioned at a talk last year -- a source file shared between multiple apps might not be used the same everywhere, so you can only know after actually linking how it's used. So we apply stricter visibility via checking what was actually imported from the library. If _ZTV5Thing isn't used outside the current library we can mark it as such and then WPD operate with this extra info.

[MaskRay]

In D91583#2612525, @lanza wrote:

I see - so just to confirm, when compiling it isn't clear that these symbols are have internal or hidden visibility, but only during linking? Because otherwise clang should already have applied an appropriate vcall_visibility that allows WPD.

Correct, when compiling we don't know what's exported and what's not, so nothing is applied. This is fundamentally similar to a problem you mentioned at a talk last year -- a source file shared between multiple apps might not be used the same everywhere, so you can only know after actually linking how it's used. So we apply stricter visibility via checking what was actually imported from the library. If _ZTV5Thing isn't used outside the current library we can mark it as such and then WPD operate with this extra info.

Do you use a local: version node in a version script to make vtable symbols local in a -shared link? LTO does not know the effective binding has become local in that case and can lose devirtualization opportunities.

The other possibility is -flto={full,thin} compile in one translation unit and -flto={full,thin} -fvisibility=hidden in another translation unit. Due to ELF visibility rule the most constraining one wins which may be more constraining than !vcall_visibility.

[tejohnson]

In D91583#2612525, @lanza wrote:

I see - so just to confirm, when compiling it isn't clear that these symbols are have internal or hidden visibility, but only during linking? Because otherwise clang should already have applied an appropriate vcall_visibility that allows WPD.

Correct, when compiling we don't know what's exported and what's not, so nothing is applied. This is fundamentally similar to a problem you mentioned at a talk last year -- a source file shared between multiple apps might not be used the same everywhere, so you can only know after actually linking how it's used. So we apply stricter visibility via checking what was actually imported from the library. If _ZTV5Thing isn't used outside the current library we can mark it as such and then WPD operate with this extra info.

Ok thanks. Dumb question - how do you know when you are linking the shared library what will be used outside of it (because presumably you don't know this until it is linked into the consuming binaries later on in the build process)? And do you communicate this info via --export-dynamic-symbol or the like?

[lanza]

Do you use a local: version node in a version script to make vtable symbols local in a -shared link? LTO does not know the effective binding has become local in that case and can lose devirtualization opportunities.

Yup. This is exactly what I've started looking for over the past few days. LTO is clearly not taking full advantage of the fact that we can guarantee these symbols are hidden and local only. The ExportDynamic was the first obvious thing I ran into. I'm sure there's a good bit more.

Ok thanks. Dumb question - how do you know when you are linking the shared library what will be used outside of it (because presumably you don't know this until it is linked into the consuming binaries later on in the build process)? And do you communicate this info via --export-dynamic-symbol or the like?

Not a dumb question, it's a pretty reasonable one. We run the link twice. Once in the normal order -- leaf to root in the dependency graph and lld tells us the list of imported symbols. From those lists we generate the full list of symbols that need to be exported and pass a version script with local:\n*; included.

lld/ELF/LTO.cpp

for (size_t i = 0, e = syms.size(); i != e; ++i) {
     // 1) All symbols when doing relocatable link, so that them can be used
     //    for doing final link.
     // 2) Symbols that are used in regular objects.
     // 3) C named sections if we have corresponding __start_/__stop_ symbol.
     // 4) Symbols that are defined in bitcode files and used for dynamic linking.
     r.VisibleToRegularObj = config->relocatable || sym->isUsedInRegularObj ||
                             (r.Prevailing && sym->includeInDynsym()) ||
                             usedStartStop.count(objSym.getSectionName());
+    // Identify symbols exported dynamically, and that therefore could be
+    // referenced by a shared library not visible to the linker.
+    r.ExportDynamic = sym->isExportDynamic(sym->kind(), sym->visibility) ||
+                      sym->exportDynamic || sym->inDynamicList;
     const auto *dr = dyn_cast<Defined>(sym);
     r.FinalDefinitionInLinkageUnit =
         (isExec || sym->visibility != STV_DEFAULT) && dr &&
         // Skip absolute symbols from ELF objects, otherwise PC-rel relocations
         // will be generated by for them, triggering linker errors.
         // Symbol section is always null for bitcode symbols, hence the check
         // for isElf(). Skip linker script defined symbols as well: they have
         // no File defined.

lld/ELF/Symbols.h

public:
   void mergeProperties(const Symbol &other);
   void resolve(const Symbol &other);
 
   // If this is a lazy symbol, fetch an input file and add the symbol
   // in the file to the symbol table. Calling this function on
   // non-lazy object causes a runtime error.
   void fetch() const;
 
-private:
   static bool isExportDynamic(Kind k, uint8_t visibility) {
     if (k == SharedKind)
       return visibility == llvm::ELF::STV_DEFAULT;
     return config->shared || config->exportDynamic;
   }
 
+private:
   void resolveUndefined(const Undefined &other);
   void resolveCommon(const CommonSymbol &other);
   void resolveDefined(const Defined &other);
   template <class LazyT> void resolveLazy(const LazyT &other);
   void resolveShared(const SharedSymbol &other);
 
   int compare(const Symbol *other) const;
 

lld/test/ELF/lto/devirt_vcall_vis_export_dynamic.ll

+; REQUIRES: x86
+;; Test that dynamically exported symbols prevent devirtualization.
+
+;; First check that we get devirtualization without any export dynamic options.
+
+;; Index based WPD
+;; Generate unsplit module with summary for ThinLTO index-based WPD.
+; RUN: opt --thinlto-bc -o %t2.o %s
+; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+;; Hybrid WPD
+;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
+; RUN: opt --thinlto-bc --thinlto-split-lto-unit -o %t.o %s
+; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+;; Regular LTO WPD
+; RUN: opt -o %t4.o %s
+; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
+
+;; Check that all WPD fails with --export-dynamic.
+
+;; Index based WPD
+; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
+
+;; Hybrid WPD
+; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
+
+;; Regular LTO WPD
+; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
+
+;; Check that WPD fails for target _ZN1D1mEi with --export-dynamic-symbol=_ZTV1D.
+
+;; Index based WPD
+; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Hybrid WPD
+; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Regular LTO WPD
+; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+; REMARK-AONLY-NOT: single-impl:
+; REMARK-AONLY: single-impl: devirtualized a call to _ZN1A1nEi
+; REMARK-AONLY-NOT: single-impl:
+
+;; Check that WPD fails for target _ZN1D1mEi with _ZTV1D in --dynamic-list.
+; RUN: echo "{ _ZTV1D; };" > %t.list
+
+;; Index based WPD
+; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Hybrid WPD
+; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Regular LTO WPD
+; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+
+;; Check that all WPD fails with when linking against a shared library containing
+;; preemptible versions of the vtables. In this case the symbols in the object being
+;; linked against the shared library must be exported to .dynsym to allow the runtime
+;; preemption, even without any options.
+
+;; Index based WPD
+; RUN: opt -relocation-model=pic -o %t5.o %s
+; RUN: ld.lld %t5.o -o %t5.so -shared
+; RUN: ld.lld %t5.o %t5.so -o %t5 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+
+;; Hybrid WPD
+; RUN: opt -relocation-model=pic --thinlto-bc -o %t5.o %s
+; RUN: ld.lld %t5.o -o %t5.so -shared
+; RUN: ld.lld %t5.o %t5.so -o %t5 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+
+;; Regular LTO WPD
+; RUN: opt -relocation-model=pic --thinlto-bc --thinlto-split-lto-unit -o %t5.o %s
+; RUN: ld.lld %t5.o -o %t5.so -shared
+; RUN: ld.lld %t5.o %t5.so -o %t5 -save-temps --lto-whole-program-visibility \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+%struct.A = type { i32 (...)** }
+%struct.B = type { %struct.A }
+%struct.C = type { %struct.A }
+%struct.D = type { i32 (...)** }
+
+@_ZTV1B = linkonce_odr unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !5
+@_ZTV1C = linkonce_odr unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.C*, i32)* @_ZN1C1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !2, !vcall_visibility !5
+@_ZTV1D = linkonce_odr unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.D*, i32)* @_ZN1D1mEi to i8*)] }, !type !3, !vcall_visibility !5
+
+;; Prevent the vtables from being dead code eliminated.
[e-mail] = appending global [3 x i8*] [ i8* bitcast ( { [4 x i8*] }* @_ZTV1B to i8*), i8* bitcast ( { [4 x i8*] }* @_ZTV1C to i8*), i8* bitcast ( { [3 x i8*] }* @_ZTV1D to i8*)]
+
+; CHECK-IR-LABEL: define dso_local i32 @_start
+define i32 @_start(%struct.A* %obj, %struct.D* %obj2, i32 %a) {
+entry:
+  %0 = bitcast %struct.A* %obj to i8***
+  %vtable = load i8**, i8*** %0
+  %1 = bitcast i8** %vtable to i8*
+  %p = call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1A")
+  call void @llvm.assume(i1 %p)
+  %fptrptr = getelementptr i8*, i8** %vtable, i32 1
+  %2 = bitcast i8** %fptrptr to i32 (%struct.A*, i32)**
+  %fptr1 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %2, align 8
+
+  ;; Check that the call was devirtualized.
+  ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-AONLY-IR: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-NODEVIRT-IR: %call = tail call i32 %fptr1
+  %call = tail call i32 %fptr1(%struct.A* nonnull %obj, i32 %a)
+
+  %3 = bitcast i8** %vtable to i32 (%struct.A*, i32)**
+  %fptr22 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %3, align 8
+
+  ;; We still have to call it as virtual.
+  ; CHECK-IR: %call3 = tail call i32 %fptr22
+  ; CHECK-AONLY-IR: %call3 = tail call i32 %fptr22
+  ; CHECK-NODEVIRT-IR: %call3 = tail call i32 %fptr22
+  %call3 = tail call i32 %fptr22(%struct.A* nonnull %obj, i32 %call)
+
+  %4 = bitcast %struct.D* %obj2 to i8***
+  %vtable2 = load i8**, i8*** %4
+  %5 = bitcast i8** %vtable2 to i8*
+  %p2 = call i1 @llvm.type.test(i8* %5, metadata !4)
+  call void @llvm.assume(i1 %p2)
+
+  %6 = bitcast i8** %vtable2 to i32 (%struct.D*, i32)**
+  %fptr33 = load i32 (%struct.D*, i32)*, i32 (%struct.D*, i32)** %6, align 8
+
+  ;; Check that the call was devirtualized.
+  ; CHECK-IR: %call4 = tail call i32 @_ZN1D1mEi
+  ; CHECK-AONLY-IR: %call4 = tail call i32 %fptr33
+  ; CHECK-NODEVIRT-IR: %call4 = tail call i32 %fptr33
+  %call4 = tail call i32 %fptr33(%struct.D* nonnull %obj2, i32 %call3)
+  ret i32 %call4
+}
+; CHECK-IR-LABEL: ret i32
+; CHECK-IR-LABEL: }
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1)
+
+define i32 @_ZN1B1fEi(%struct.B* %this, i32 %a) #0 {
+   ret i32 0;
+}
+
+define i32 @_ZN1A1nEi(%struct.A* %this, i32 %a) #0 {
+   ret i32 0;
+}
+
+define i32 @_ZN1C1fEi(%struct.C* %this, i32 %a) #0 {
+   ret i32 0;
+}
+
+define i32 @_ZN1D1mEi(%struct.D* %this, i32 %a) #0 {
+   ret i32 0;
+}
+
+;; Make sure we don't inline or otherwise optimize out the direct calls.
+attributes #0 = { noinline optnone }
+
+!0 = !{i64 16, !"_ZTS1A"}
+!1 = !{i64 16, !"_ZTS1B"}
+!2 = !{i64 16, !"_ZTS1C"}
+!3 = !{i64 16, !4}
+!4 = distinct !{}
+!5 = !{i64 0}

lld/test/ELF/lto/devirt_vcall_vis_public.ll

 ; REQUIRES: x86
 ;; Test that --lto-whole-program-visibility enables devirtualization.
 
-;; Note that the --export-dynamic used below is simply to ensure symbols are
-;; retained during linking.
-
 ;; Index based WPD
 ;; Generate unsplit module with summary for ThinLTO index-based WPD.
 ; RUN: opt --thinlto-bc -o %t2.o %s
 ; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Hybrid WPD
 ;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt --thinlto-bc --thinlto-split-lto-unit -o %t.o %s
 ; RUN: ld.lld %t.o -o %t3 -save-temps --lto-whole-program-visibility \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Regular LTO WPD
 ; RUN: opt -o %t4.o %s
 ; RUN: ld.lld %t4.o -o %t3 -save-temps --lto-whole-program-visibility \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 
 ;; Try everything again but without -whole-program-visibility to confirm
 ;; WPD fails
 
 ;; Index based WPD
 ; RUN: ld.lld %t2.o -o %t3 -save-temps \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 ;; Ensure --no-lto-whole-program-visibility overrides explicit --lto-whole-program-visibility.
 ; RUN: ld.lld %t2.o -o %t3 -save-temps --lto-whole-program-visibility --no-lto-whole-program-visibility \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Hybrid WPD
 ; RUN: ld.lld %t.o -o %t3 -save-temps \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Regular LTO WPD
 ; RUN: ld.lld %t4.o -o %t3 -save-temps \
-; RUN:   -mllvm -pass-remarks=. --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   -mllvm -pass-remarks=. \
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.A = type { i32 (...)** }
 %struct.B = type { %struct.A }
 %struct.C = type { %struct.A }
 %struct.D = type { i32 (...)** }
 
 @_ZTV1B = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !5
 @_ZTV1C = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.C*, i32)* @_ZN1C1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !2, !vcall_visibility !5
 @_ZTV1D = constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.D*, i32)* @_ZN1D1mEi to i8*)] }, !type !3, !vcall_visibility !5
 
+; Prevent the vtables from being dead code eliminated.
[e-mail] = appending global [3 x i8*] [ i8* bitcast ( { [4 x i8*] }* @_ZTV1B to i8*), i8* bitcast ( { [4 x i8*] }* @_ZTV1C to i8*), i8* bitcast ( { [3 x i8*] }* @_ZTV1D to i8*)]
 
 ; CHECK-IR-LABEL: define dso_local i32 @_start
 define i32 @_start(%struct.A* %obj, %struct.D* %obj2, i32 %a) {
 entry:
   %0 = bitcast %struct.A* %obj to i8***
   %vtable = load i8**, i8*** %0
   %1 = bitcast i8** %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1A")

llvm/include/llvm/LTO/LTO.h

struct GlobalResolution {
     /// The unmangled name of the global.
     std::string IRName;
 
     /// Keep track if the symbol is visible outside of a module with a summary
     /// (i.e. in either a regular object or a regular LTO module without a
     /// summary).
     bool VisibleOutsideSummary = false;
 
+    /// The symbol was exported dynamically, and therefore could be referenced
+    /// by a shared library not visible to the linker.
+    bool ExportDynamic = false;
+
     bool UnnamedAddr = true;
 
     /// True if module contains the prevailing definition.
     bool Prevailing = false;
 
     /// Returns true if module contains the prevailing definition and symbol is
     /// an IR symbol. For example when module-level inline asm block is used,
     /// symbol can be prevailing in module but have no IR name.
Error runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols);
 
   Error checkPartiallySplit();
 
   mutable bool CalledGetMaxTasks = false;
 
   // Use Optional to distinguish false from not yet initialized.
   Optional<bool> EnableSplitLTOUnit;
+
+  // Identify symbols exported dynamically, and that therefore could be
+  // referenced by a shared library not visible to the linker.
+  DenseSet<GlobalValue::GUID> DynamicExportSymbols;
 };
 
 /// The resolution for a symbol. The linker must provide a SymbolResolution for
 /// each global symbol based on its internal resolution of that symbol.
 struct SymbolResolution {
   SymbolResolution()
       : Prevailing(0), FinalDefinitionInLinkageUnit(0), VisibleToRegularObj(0),
-        LinkerRedefined(0) {}
+        ExportDynamic(0), LinkerRedefined(0) {}
 
   /// The linker has chosen this definition of the symbol.
   unsigned Prevailing : 1;
 
   /// The definition of this symbol is unpreemptable at runtime and is known to
   /// be in this linkage unit.
   unsigned FinalDefinitionInLinkageUnit : 1;
 
   /// The definition of this symbol is visible outside of the LTO unit.
   unsigned VisibleToRegularObj : 1;
 
+  /// The symbol was exported dynamically, and therefore could be referenced
+  /// by a shared library not visible to the linker.
+  unsigned ExportDynamic : 1;
+
   /// Linker redefined version of the symbol which appeared in -wrap or -defsym
   /// linker option.
   unsigned LinkerRedefined : 1;
 };
 
 } // namespace lto
 } // namespace llvm
 
 #endif

llvm/include/llvm/Transforms/IPO/WholeProgramDevirt.h

struct WholeProgramDevirtPass : public PassInfoMixin<WholeProgramDevirtPass> {
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
 };
 
 struct VTableSlotSummary {
   StringRef TypeID;
   uint64_t ByteOffset;
 };
 
-void updateVCallVisibilityInModule(Module &M,
-                                   bool WholeProgramVisibilityEnabledInLTO);
-void updateVCallVisibilityInIndex(ModuleSummaryIndex &Index,
-                                  bool WholeProgramVisibilityEnabledInLTO);
+void updateVCallVisibilityInModule(
+    Module &M, bool WholeProgramVisibilityEnabledInLTO,
+    const DenseSet<GlobalValue::GUID> &DynamicExportSymbols);
+void updateVCallVisibilityInIndex(
+    ModuleSummaryIndex &Index, bool WholeProgramVisibilityEnabledInLTO,
+    const DenseSet<GlobalValue::GUID> &DynamicExportSymbols);
 
 /// Perform index-based whole program devirtualization on the \p Summary
 /// index. Any devirtualized targets used by a type test in another module
 /// are added to the \p ExportedGUIDs set. For any local devirtualized targets
 /// only used within the defining module, the information necessary for
 /// locating the corresponding WPD resolution is recorded for the ValueInfo
 /// in case it is exported by cross module importing (in which case the
 /// devirtualized target name will need adjustment).

llvm/lib/LTO/LTO.cpp

for (const InputFile::Symbol &Sym : Syms) {
     } else
       // First recorded reference, save the current partition.
       GlobalRes.Partition = Partition;
 
     // Flag as visible outside of summary if visible from a regular object or
     // from a module that does not have a summary.
     GlobalRes.VisibleOutsideSummary |=
         (Res.VisibleToRegularObj || Sym.isUsed() || !InSummary);
+
+    GlobalRes.ExportDynamic |= Res.ExportDynamic;
   }
 }
 
 static void writeToResolutionFile(raw_ostream &OS, InputFile *Input,
                                   ArrayRef<SymbolResolution> Res) {
   StringRef Path = Input->getName();
   OS << Path << '\n';
   auto ResI = Res.begin();
if (Res.second.IRName.empty())
       continue;
 
     GlobalValue::GUID GUID = GlobalValue::getGUID(
         GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
 
     if (Res.second.VisibleOutsideSummary && Res.second.Prevailing)
       GUIDPreservedSymbols.insert(GUID);
 
+    if (Res.second.ExportDynamic)
+      DynamicExportSymbols.insert(GUID);
+
     GUIDPrevailingResolutions[GUID] =
         Res.second.Prevailing ? PrevailingType::Yes : PrevailingType::No;
   }
 
   auto isPrevailing = [&](GlobalValue::GUID G) {
     auto It = GUIDPrevailingResolutions.find(G);
     if (It == GUIDPrevailingResolutions.end())
       return PrevailingType::Unknown;
for (auto &I : RegularLTO.Commons) {
     } else {
       GV->setName(I.first);
     }
   }
 
   // If allowed, upgrade public vcall visibility metadata to linkage unit
   // visibility before whole program devirtualization in the optimizer.
   updateVCallVisibilityInModule(*RegularLTO.CombinedModule,
-                                Conf.HasWholeProgramVisibility);
+                                Conf.HasWholeProgramVisibility,
+                                DynamicExportSymbols);
 
   if (Conf.PreOptModuleHook &&
       !Conf.PreOptModuleHook(0, *RegularLTO.CombinedModule))
     return Error::success();
 
   if (!Conf.CodeGenOnly) {
     for (const auto &R : GlobalResolutions) {
       if (!R.second.isPrevailingIRSymbol())
Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
   if (DumpThinCGSCCs)
     ThinLTO.CombinedIndex.dumpSCCs(outs());
 
   std::set<GlobalValue::GUID> ExportedGUIDs;
 
   // If allowed, upgrade public vcall visibility to linkage unit visibility in
   // the summaries before whole program devirtualization below.
   updateVCallVisibilityInIndex(ThinLTO.CombinedIndex,
-                               Conf.HasWholeProgramVisibility);
+                               Conf.HasWholeProgramVisibility,
+                               DynamicExportSymbols);
 
   // Perform index-based WPD. This will return immediately if there are
   // no index entries in the typeIdMetadata map (e.g. if we are instead
   // performing IR-based WPD in hybrid regular/thin LTO mode).
   std::map<ValueInfo, std::vector<VTableSlotSummary>> LocalWPDTargetsMap;
   runWholeProgramDevirtOnIndex(ThinLTO.CombinedIndex, ExportedGUIDs,
                                LocalWPDTargetsMap);
 

llvm/lib/LTO/LTOCodeGenerator.cpp

bool LTOCodeGenerator::optimize() {
   }
   StatsFile = std::move(StatsFileOrErr.get());
 
   // Currently there is no support for enabling whole program visibility via a
   // linker option in the old LTO API, but this call allows it to be specified
   // via the internal option. Must be done before WPD invoked via the optimizer
   // pipeline run below.
   updateVCallVisibilityInModule(*MergedModule,
-                                /* WholeProgramVisibilityEnabledInLTO */ false);
+                                /* WholeProgramVisibilityEnabledInLTO */ false,
+                                // FIXME: This needs linker information via a
+                                // TBD new interface.
+                                /* DynamicExportSymbols */ {});
 
   // We always run the verifier once on the merged module, the `DisableVerify`
   // parameter only applies to subsequent verify.
   verifyMergedModuleOnce();
 
   // Mark which symbols can not be internalized
   this->applyScopeRestrictions();
 

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

void ThinLTOCodeGenerator::run() {
 
   // Synthesize entry counts for functions in the combined index.
   computeSyntheticCounts(*Index);
 
   // Currently there is no support for enabling whole program visibility via a
   // linker option in the old LTO API, but this call allows it to be specified
   // via the internal option. Must be done before WPD below.
   updateVCallVisibilityInIndex(*Index,
-                               /* WholeProgramVisibilityEnabledInLTO */ false);
+                               /* WholeProgramVisibilityEnabledInLTO */ false,
+                               // FIXME: This needs linker information via a
+                               // TBD new interface.
+                               /* DynamicExportSymbols */ {});
 
   // Perform index-based WPD. This will return immediately if there are
   // no index entries in the typeIdMetadata map (e.g. if we are instead
   // performing IR-based WPD in hybrid regular/thin LTO mode).
   std::map<ValueInfo, std::vector<VTableSlotSummary>> LocalWPDTargetsMap;
   std::set<GlobalValue::GUID> ExportedGUIDs;
   runWholeProgramDevirtOnIndex(*Index, ExportedGUIDs, LocalWPDTargetsMap);
   for (auto GUID : ExportedGUIDs)

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

return (WholeProgramVisibilityEnabledInLTO || WholeProgramVisibility) &&
          !DisableWholeProgramVisibility;
 }
 
 namespace llvm {
 
 /// If whole program visibility asserted, then upgrade all public vcall
 /// visibility metadata on vtable definitions to linkage unit visibility in
 /// Module IR (for regular or hybrid LTO).
-void updateVCallVisibilityInModule(Module &M,
-                                   bool WholeProgramVisibilityEnabledInLTO) {
+void updateVCallVisibilityInModule(
+    Module &M, bool WholeProgramVisibilityEnabledInLTO,
+    const DenseSet<GlobalValue::GUID> &DynamicExportSymbols) {
   if (!hasWholeProgramVisibility(WholeProgramVisibilityEnabledInLTO))
     return;
   for (GlobalVariable &GV : M.globals())
     // Add linkage unit visibility to any variable with type metadata, which are
     // the vtable definitions. We won't have an existing vcall_visibility
     // metadata on vtable definitions with public visibility.
     if (GV.hasMetadata(LLVMContext::MD_type) &&
-        GV.getVCallVisibility() == GlobalObject::VCallVisibilityPublic)
+        GV.getVCallVisibility() == GlobalObject::VCallVisibilityPublic &&
+        // Don't upgrade the visibility for symbols exported to the dynamic
+        // linker, as we have no information on their eventual use.
+        !DynamicExportSymbols.count(GV.getGUID()))
       GV.setVCallVisibilityMetadata(GlobalObject::VCallVisibilityLinkageUnit);
 }
 
 /// If whole program visibility asserted, then upgrade all public vcall
 /// visibility metadata on vtable definition summaries to linkage unit
 /// visibility in Module summary index (for ThinLTO).
-void updateVCallVisibilityInIndex(ModuleSummaryIndex &Index,
-                                  bool WholeProgramVisibilityEnabledInLTO) {
+void updateVCallVisibilityInIndex(
+    ModuleSummaryIndex &Index, bool WholeProgramVisibilityEnabledInLTO,
+    const DenseSet<GlobalValue::GUID> &DynamicExportSymbols) {
   if (!hasWholeProgramVisibility(WholeProgramVisibilityEnabledInLTO))
     return;
   for (auto &P : Index) {
     for (auto &S : P.second.SummaryList) {
       auto *GVar = dyn_cast<GlobalVarSummary>(S.get());
       if (!GVar || GVar->vTableFuncs().empty() ||
-          GVar->getVCallVisibility() != GlobalObject::VCallVisibilityPublic)
+          GVar->getVCallVisibility() != GlobalObject::VCallVisibilityPublic ||
+          // Don't upgrade the visibility for symbols exported to the dynamic
+          // linker, as we have no information on their eventual use.
+          DynamicExportSymbols.count(P.first))
         continue;
       GVar->setVCallVisibility(GlobalObject::VCallVisibilityLinkageUnit);
     }
   }
 }
 
 void runWholeProgramDevirtOnIndex(
     ModuleSummaryIndex &Summary, std::set<GlobalValue::GUID> &ExportedGUIDs,

llvm/test/tools/gold/X86/devirt_vcall_vis_export_dynamic.ll

-;; Test that plugin option whole-program-visibility enables devirtualization.
+;; Test that --export-dynamic[-symbol] and --dynamic-list prevents devirtualization.
+
+;; First check that we get devirtualization without any export dynamic options.
 
 ;; Index based WPD
 ;; Generate unsplit module with summary for ThinLTO index-based WPD.
 ; RUN: opt -thinlto-bc -o %t2.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t2.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t2.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Hybrid WPD
 ;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Regular LTO WPD
 ; RUN: opt -o %t4.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t4.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t4.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 
-;; Try everything again but without -whole-program-visibility to confirm
-;; WPD fails
+;; Check that all WPD fails with --export-dynamic.
 
 ;; Index based WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t2.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Hybrid WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Regular LTO WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t4.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   --export-dynamic 2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
+;; Check that WPD fails for target _ZN1D1mEi with --export-dynamic-symbol=_ZTV1D.
+
+;; Index based WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t2.o -o %t3 \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Hybrid WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t.o -o %t3 \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Regular LTO WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t4.o -o %t3 \
+; RUN:   --export-dynamic-symbol=_ZTV1D 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+; REMARK-AONLY-NOT: single-impl:
+; REMARK-AONLY: single-impl: devirtualized a call to _ZN1A1nEi
+; REMARK-AONLY-NOT: single-impl:
+
+;; Check that WPD fails for target _ZN1D1mEi with _ZTV1D in --dynamic-list.
+; RUN: echo "{ _ZTV1D; };" > %t.list
+
+;; Index based WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t2.o -o %t3 \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Hybrid WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t.o -o %t3 \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
+;; Regular LTO WPD
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:   --plugin-opt=whole-program-visibility \
+; RUN:   --plugin-opt=save-temps \
+; RUN:   --plugin-opt=-pass-remarks=. \
+; RUN:   %t4.o -o %t3 \
+; RUN:   --dynamic-list=%t.list 2>&1 | FileCheck %s --check-prefix=REMARK-AONLY
+; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-AONLY-IR
+
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.A = type { i32 (...)** }
 %struct.B = type { %struct.A }
 %struct.C = type { %struct.A }
 %struct.D = type { i32 (...)** }
 
-@_ZTV1B = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !5
-@_ZTV1C = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.C*, i32)* @_ZN1C1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !2, !vcall_visibility !5
-@_ZTV1D = constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.D*, i32)* @_ZN1D1mEi to i8*)] }, !type !3, !vcall_visibility !5
+@_ZTV1B = linkonce_odr unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !5
+@_ZTV1C = linkonce_odr unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.C*, i32)* @_ZN1C1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !2, !vcall_visibility !5
+@_ZTV1D = linkonce_odr unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.D*, i32)* @_ZN1D1mEi to i8*)] }, !type !3, !vcall_visibility !5
 
+;; Prevent the vtables from being dead code eliminated.
[e-mail] = appending global [3 x i8*] [ i8* bitcast ( { [4 x i8*] }* @_ZTV1B to i8*), i8* bitcast ( { [4 x i8*] }* @_ZTV1C to i8*), i8* bitcast ( { [3 x i8*] }* @_ZTV1D to i8*)]
 
 ; CHECK-IR-LABEL: define dso_local i32 @_start
 define i32 @_start(%struct.A* %obj, %struct.D* %obj2, i32 %a) {
 entry:
   %0 = bitcast %struct.A* %obj to i8***
   %vtable = load i8**, i8*** %0
   %1 = bitcast i8** %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1A")
   call void @llvm.assume(i1 %p)
   %fptrptr = getelementptr i8*, i8** %vtable, i32 1
   %2 = bitcast i8** %fptrptr to i32 (%struct.A*, i32)**
   %fptr1 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %2, align 8
 
   ;; Check that the call was devirtualized.
   ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-AONLY-IR: %call = tail call i32 @_ZN1A1nEi
   ; CHECK-NODEVIRT-IR: %call = tail call i32 %fptr1
   %call = tail call i32 %fptr1(%struct.A* nonnull %obj, i32 %a)
 
   %3 = bitcast i8** %vtable to i32 (%struct.A*, i32)**
   %fptr22 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %3, align 8
 
   ;; We still have to call it as virtual.
   ; CHECK-IR: %call3 = tail call i32 %fptr22
+  ; CHECK-AONLY-IR: %call3 = tail call i32 %fptr22
   ; CHECK-NODEVIRT-IR: %call3 = tail call i32 %fptr22
   %call3 = tail call i32 %fptr22(%struct.A* nonnull %obj, i32 %call)
 
   %4 = bitcast %struct.D* %obj2 to i8***
   %vtable2 = load i8**, i8*** %4
   %5 = bitcast i8** %vtable2 to i8*
   %p2 = call i1 @llvm.type.test(i8* %5, metadata !4)
   call void @llvm.assume(i1 %p2)
 
   %6 = bitcast i8** %vtable2 to i32 (%struct.D*, i32)**
   %fptr33 = load i32 (%struct.D*, i32)*, i32 (%struct.D*, i32)** %6, align 8
 
   ;; Check that the call was devirtualized.
   ; CHECK-IR: %call4 = tail call i32 @_ZN1D1mEi
+  ; CHECK-AONLY-IR: %call4 = tail call i32 %fptr33
   ; CHECK-NODEVIRT-IR: %call4 = tail call i32 %fptr33
   %call4 = tail call i32 %fptr33(%struct.D* nonnull %obj2, i32 %call3)
   ret i32 %call4
 }
 ; CHECK-IR-LABEL: ret i32
 ; CHECK-IR-LABEL: }
 
 declare i1 @llvm.type.test(i8*, metadata)

llvm/test/tools/gold/X86/devirt_vcall_vis_public.ll

 ;; Test that plugin option whole-program-visibility enables devirtualization.
 
 ;; Index based WPD
 ;; Generate unsplit module with summary for ThinLTO index-based WPD.
 ; RUN: opt -thinlto-bc -o %t2.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t2.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t2.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Hybrid WPD
 ;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ;; Regular LTO WPD
 ; RUN: opt -o %t4.o %s
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=whole-program-visibility \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
-; RUN:   %t4.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   %t4.o -o %t3 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 
 ;; Try everything again but without -whole-program-visibility to confirm
 ;; WPD fails
 
 ;; Index based WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t2.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t2.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Hybrid WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t.o.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 ;; Regular LTO WPD
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:   --plugin-opt=save-temps \
 ; RUN:   --plugin-opt=-pass-remarks=. \
 ; RUN:   %t4.o -o %t3 \
-; RUN:   --export-dynamic 2>&1 | FileCheck %s --implicit-check-not single-impl --allow-empty
+; RUN:   2>&1 | FileCheck /dev/null --implicit-check-not single-impl --allow-empty
 ; RUN: llvm-dis %t3.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-NODEVIRT-IR
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.A = type { i32 (...)** }
 %struct.B = type { %struct.A }
 %struct.C = type { %struct.A }
 %struct.D = type { i32 (...)** }
 
 @_ZTV1B = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !5
 @_ZTV1C = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.C*, i32)* @_ZN1C1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !2, !vcall_visibility !5
 @_ZTV1D = constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.D*, i32)* @_ZN1D1mEi to i8*)] }, !type !3, !vcall_visibility !5
 
+; Prevent the vtables from being dead code eliminated.
[e-mail] = appending global [3 x i8*] [ i8* bitcast ( { [4 x i8*] }* @_ZTV1B to i8*), i8* bitcast ( { [4 x i8*] }* @_ZTV1C to i8*), i8* bitcast ( { [3 x i8*] }* @_ZTV1D to i8*)]
 
 ; CHECK-IR-LABEL: define dso_local i32 @_start
 define i32 @_start(%struct.A* %obj, %struct.D* %obj2, i32 %a) {
 entry:
   %0 = bitcast %struct.A* %obj to i8***
   %vtable = load i8**, i8*** %0
   %1 = bitcast i8** %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1A")

llvm/tools/gold/gold-plugin.cpp

for (ld_plugin_symbol &Sym : F.syms) {
 
     case LDPR_PREVAILING_DEF:
       R.Prevailing = !isUndefined(Sym);
       R.VisibleToRegularObj = true;
       break;
 
     case LDPR_PREVAILING_DEF_IRONLY_EXP:
       R.Prevailing = !isUndefined(Sym);
+      // Identify symbols exported dynamically, and that therefore could be
+      // referenced by a shared library not visible to the linker.
+      R.ExportDynamic = true;
       if (!Res.CanOmitFromDynSym)
         R.VisibleToRegularObj = true;
       break;
     }
 
     // If the symbol has a C identifier section name, we need to mark
     // it as visible to a regular object so that LTO will keep it around
     // to ensure the linker generates special __start_<secname> and

llvm/tools/opt/opt.cpp

if (!NoVerify && verifyModule(*M, &errs())) {
     return 1;
   }
 
   // Enable testing of whole program devirtualization on this module by invoking
   // the facility for updating public visibility to linkage unit visibility when
   // specified by an internal option. This is normally done during LTO which is
   // not performed via opt.
   updateVCallVisibilityInModule(*M,
-                                /* WholeProgramVisibilityEnabledInLTO */ false);
+                                /* WholeProgramVisibilityEnabledInLTO */ false,
+                                /* DynamicExportSymbols */ {});
 
   // Figure out what stream we are supposed to write to...
   std::unique_ptr<ToolOutputFile> Out;
   std::unique_ptr<ToolOutputFile> ThinLinkOut;
   if (NoOutput) {
     if (!OutputFilename.empty())
       errs() << "WARNING: The -o (output filename) option is ignored when\n"
                 "the --disable-output option is used.\n";