This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Propagate dllexport to thunks
ClosedPublic

Authored by smeenai on Jul 3 2017, 7:37 PM.

Details

Summary

Under Windows Itanium, we need to export virtual and non-virtual thunks
if the functions being thunked are exported. These thunks would
previously inherit their dllexport attribute from the declaration, but
r298330 changed declarations to not have dllexport attributes. We
therefore need to add the dllexport attribute to the definition
ourselves now. This is consistent with MinGW GCC's behavior.

This redoes r306770 but limits the logic to Itanium. MicrosoftCXXABI's
setThunkLinkage ensures that thunks aren't exported under that ABI, so
I'm handling this in ItaniumCXXABI's setThunkLinkage for symmetry.

We need to export these thunks because they can be referenced outside
the library they're defined in. For example, if a child class without a
key function inherits from a parent class with a key function, the
parent's thunks will only be defined in the library with the key
function, but the construction vtable for the parent in the child might
be emitted outside the library (since the child doesn't have a key
function), and it needs to reference the parent's thunks.

We don't need to mark these thunks as imported since any references to
them will occur in data, so the compiler can't generate the IAT load
sequence anyway. Instead, we rely on the linker generating import thunks
for the thunks.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai created this revision.Jul 3 2017, 7:37 PM
smeenai edited the summary of this revision. (Show Details)Jul 21 2017, 10:11 PM
smeenai added a reviewer: majnemer.

Ping. I updated the description based on the email discussion with @majnemer.

majnemer edited edge metadata.Jul 21 2017, 10:31 PM

OK, so we are exporting the thunks so that the linker will generate import thunks for the thunks. I think that we should have a comment to that effect near the code you added.

smeenai updated this revision to Diff 107795.Jul 22 2017, 11:51 AM
smeenai edited the summary of this revision. (Show Details)

Add comment

This revision is now accepted and ready to land.Jul 24 2017, 8:57 AM
This revision was automatically updated to reflect the committed changes.