This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Emit COFF symbol type for function aliases
ClosedPublic

Authored by mstorsjo on Jan 27 2022, 12:40 AM.

Details

Summary

On the level of the generated object files, both symbols (both
original and alias) are generally indistinguishable - both are
regular defined symbols. But previously, only the original
function had the COFF ComplexType set to IMAGE_SYM_DTYPE_FUNCTION,
while the symbol created via an alias had the type set to
IMAGE_SYM_DTYPE_NULL.

This matches what GCC does, which emits directives for setting the
COFF symbol type for this kind of alias symbol too.

This makes a difference when GNU ld.bfd exports symbols without
dllexport directives or a def file - it seems to decide between
function or data exports based on the COFF symbol type. This means
that functions created via aliases, like some C++ constructors,
are exported as data symbols (missing the thunk for calling without
dllimport).

The hasnt been an issue when doing the same with LLD, as LLD decides
between function or data export based on the flags of the section
that the symbol points at.

This should fix the root cause of
https://github.com/msys2/MINGW-packages/issues/10547.

Diff Detail

Event Timeline

mstorsjo created this revision.Jan 27 2022, 12:40 AM
mstorsjo requested review of this revision.Jan 27 2022, 12:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2022, 12:40 AM
mstorsjo updated this revision to Diff 403554.Jan 27 2022, 2:18 AM

Fix clang-format, rebased further on main past unrelated breakage.

rnk accepted this revision.Jan 27 2022, 11:11 AM

lgtm

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1653

I see that these directives are emitted in the ARM, AArch64, and X86 AsmPrinter subclasses instead of here in the main asm printer. However, I think it makes more sense to put them here the way you have it. They are common to all COFF targets.

So, no need to make any changes, just an idea for a cleanup.

This revision is now accepted and ready to land.Jan 27 2022, 11:11 AM
mstorsjo added inline comments.Jan 27 2022, 11:14 AM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1653

Yep - I noticed that they were there, but I didn't want to dive into that right now, assuming they're there for either good or historical reasons...

This revision was landed with ongoing or failed builds.Jan 28 2022, 3:06 AM
This revision was automatically updated to reflect the committed changes.