[CodeGen][ARM64EC] Define hybrid_patchable EXP thunk symbol as a function. #102898
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-aarch64 Author: Jacek Caban (cjacek) ChangesThis is needed for MSVC link.exe to generate redirection metadata for hybrid patchable thunks. I noticed this while working on LLD support for this. Redirection metadata is a part of CHPE metadata generated by the linker. It's undocumented, but I briefly documented my findings here. Its logic seems to be separate from generating thunks themselves in the linker. While for export thunks any unresolved There is a different logic for exported symbols, those already work fine with hybrid patchable functions, but for a different reason. I think that in practice that metadata is not actually needed for functions that are not exported. Call checkers don't need it, it's probably mostly used for auxiliary IAT handling. If that's right, then this fix won't change much in practice. But since it's undocumented, I can't be sure so I think it's better to just get it right and make sure that generated metadata matches what would be seen with MSVC. Full diff: https://github.com/llvm/llvm-project/pull/102898.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 0391d518324315..b8f9b58a216446 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -1310,6 +1310,13 @@ void AArch64AsmPrinter::emitGlobalAlias(const Module &M,
StringRef ExpStr = cast<MDString>(Node->getOperand(0))->getString();
MCSymbol *ExpSym = MMI->getContext().getOrCreateSymbol(ExpStr);
MCSymbol *Sym = MMI->getContext().getOrCreateSymbol(GA.getName());
+
+ OutStreamer->beginCOFFSymbolDef(ExpSym);
+ OutStreamer->emitCOFFSymbolStorageClass(COFF::IMAGE_SYM_CLASS_EXTERNAL);
+ OutStreamer->emitCOFFSymbolType(COFF::IMAGE_SYM_DTYPE_FUNCTION
+ << COFF::SCT_COMPLEX_TYPE_SHIFT);
+ OutStreamer->endCOFFSymbolDef();
+
OutStreamer->beginCOFFSymbolDef(Sym);
OutStreamer->emitCOFFSymbolStorageClass(COFF::IMAGE_SYM_CLASS_EXTERNAL);
OutStreamer->emitCOFFSymbolType(COFF::IMAGE_SYM_DTYPE_FUNCTION
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll b/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
index 64fb5b36b2c623..1ed6a273338abb 100644
--- a/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
+++ b/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
@@ -240,6 +240,10 @@ define dso_local void @caller() nounwind {
; CHECK-NEXT: .section .drectve,"yni"
; CHECK-NEXT: .ascii " /EXPORT:exp"
+; CHECK-NEXT: .def "EXP+#func";
+; CHECK-NEXT: .scl 2;
+; CHECK-NEXT: .type 32;
+; CHECK-NEXT: .endef
; CHECK-NEXT: .def func;
; CHECK-NEXT: .scl 2;
; CHECK-NEXT: .type 32;
@@ -252,6 +256,10 @@ define dso_local void @caller() nounwind {
; CHECK-NEXT: .type 32;
; CHECK-NEXT: .endef
; CHECK-NEXT: .set "#func", "#func$hybpatch_thunk"{{$}}
+; CHECK-NEXT: .def "EXP+#has_varargs";
+; CHECK-NEXT: .scl 2;
+; CHECK-NEXT: .type 32;
+; CHECK-NEXT: .endef
; CHECK-NEXT: .def has_varargs;
; CHECK-NEXT: .scl 2;
; CHECK-NEXT: .type 32;
@@ -264,6 +272,10 @@ define dso_local void @caller() nounwind {
; CHECK-NEXT: .type 32;
; CHECK-NEXT: .endef
; CHECK-NEXT: .set "#has_varargs", "#has_varargs$hybpatch_thunk"
+; CHECK-NEXT: .def "EXP+#has_sret";
+; CHECK-NEXT: .scl 2;
+; CHECK-NEXT: .type 32;
+; CHECK-NEXT: .endef
; CHECK-NEXT: .def has_sret;
; CHECK-NEXT: .scl 2;
; CHECK-NEXT: .type 32;
@@ -276,6 +288,10 @@ define dso_local void @caller() nounwind {
; CHECK-NEXT: .type 32;
; CHECK-NEXT: .endef
; CHECK-NEXT: .set "#has_sret", "#has_sret$hybpatch_thunk"
+; CHECK-NEXT: .def "EXP+#exp";
+; CHECK-NEXT: .scl 2;
+; CHECK-NEXT: .type 32;
+; CHECK-NEXT: .endef
; CHECK-NEXT: .def exp;
; CHECK-NEXT: .scl 2;
; CHECK-NEXT: .type 32;
@@ -295,18 +311,18 @@ define dso_local void @caller() nounwind {
; SYM: [78](sec 20)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x00000000 #exp$hybpatch_thunk
; SYM: [110](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 func
; SYM-NEXT: AUX indx 112 srch 3
-; SYM-NEXT: [112](sec 0)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 EXP+#func
+; SYM-NEXT: [112](sec 0)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x00000000 EXP+#func
; SYM: [116](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 #func
; SYM-NEXT: AUX indx 53 srch 3
; SYM: [122](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 has_varargs
; SYM-NEXT: AUX indx 124 srch 3
-; SYM-NEXT: [124](sec 0)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 EXP+#has_varargs
+; SYM-NEXT: [124](sec 0)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x00000000 EXP+#has_varargs
; SYM-NEXT: [125](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 has_sret
; SYM-NEXT: AUX indx 127 srch 3
-; SYM-NEXT: [127](sec 0)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 EXP+#has_sret
+; SYM-NEXT: [127](sec 0)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x00000000 EXP+#has_sret
; SYM-NEXT: [128](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 exp
; SYM-NEXT: AUX indx 130 srch 3
-; SYM-NEXT: [130](sec 0)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 EXP+#exp
+; SYM-NEXT: [130](sec 0)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x00000000 EXP+#exp
; SYM-NEXT: [131](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 #has_varargs
; SYM-NEXT: AUX indx 58 srch 3
; SYM-NEXT: [133](sec 0)(fl 0x00)(ty 0)(scl 69) (nx 1) 0x00000000 #has_sret
|
|
/cherry-pick d550ada |
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 13, 2024
…tion. (llvm#102898) This is needed for MSVC link.exe to generate redirection metadata for hybrid patchable thunks. (cherry picked from commit d550ada)
|
/pull-request #103048 |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 15, 2024
…tion. (llvm#102898) This is needed for MSVC link.exe to generate redirection metadata for hybrid patchable thunks. (cherry picked from commit d550ada)
cjacek
added a commit
that referenced
this pull request
Aug 22, 2024
…e functions. (#105499) This implements Fast-Forward Sequences documented in ARM64EC ABI https://learn.microsoft.com/en-us/windows/arm/arm64ec-abi. There are two conditions when linker should generate such thunks: - For each exported ARM64EC functions. It applies only to ARM64EC functions (we may also have pure x64 functions, for which no thunk is needed). MSVC linker creates `EXP+<mangled export name>` symbol in those cases that points to the thunk and uses that symbol for the export. It's observable from the module: it's possible to reference such symbols as I did in the test. Note that it uses export name, not name of the symbol that's exported (as in `foo` in `/EXPORT:foo=bar`). This implies that if the same function is exported multiple times, it will have multiple thunks. I followed this MSVC behavior. - For hybrid_patchable functions. The linker tries to generate a thunk for each undefined `EXP+*` symbol (and such symbols are created by the compiler as a target of weak alias from the demangled name). MSVC linker tries to find corresponding `*$hp_target` symbol and if fails to do so, it outputs a cryptic error like `LINK : fatal error LNK1000: Internal error during IMAGE::BuildImage`. I just skip generating the thunk in such case (which causes undefined reference error). MSVC linker additionally checks that the symbol complex type is a function (see also #102898). We generally don't do such checks in LLD, so I made it less strict. It should be fine: if it's some data symbol, it will not have `$hp_target` symbol, so we will skip it anyway.
cjdb
pushed a commit
to cjdb/llvm-project
that referenced
this pull request
Aug 23, 2024
…e functions. (llvm#105499) This implements Fast-Forward Sequences documented in ARM64EC ABI https://learn.microsoft.com/en-us/windows/arm/arm64ec-abi. There are two conditions when linker should generate such thunks: - For each exported ARM64EC functions. It applies only to ARM64EC functions (we may also have pure x64 functions, for which no thunk is needed). MSVC linker creates `EXP+<mangled export name>` symbol in those cases that points to the thunk and uses that symbol for the export. It's observable from the module: it's possible to reference such symbols as I did in the test. Note that it uses export name, not name of the symbol that's exported (as in `foo` in `/EXPORT:foo=bar`). This implies that if the same function is exported multiple times, it will have multiple thunks. I followed this MSVC behavior. - For hybrid_patchable functions. The linker tries to generate a thunk for each undefined `EXP+*` symbol (and such symbols are created by the compiler as a target of weak alias from the demangled name). MSVC linker tries to find corresponding `*$hp_target` symbol and if fails to do so, it outputs a cryptic error like `LINK : fatal error LNK1000: Internal error during IMAGE::BuildImage`. I just skip generating the thunk in such case (which causes undefined reference error). MSVC linker additionally checks that the symbol complex type is a function (see also llvm#102898). We generally don't do such checks in LLD, so I made it less strict. It should be fine: if it's some data symbol, it will not have `$hp_target` symbol, so we will skip it anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is needed for MSVC link.exe to generate redirection metadata for hybrid patchable thunks.
I noticed this while working on LLD support for this. Redirection metadata is a part of CHPE metadata generated by the linker. It's undocumented, but I briefly documented my findings here. Its logic seems to be separate from generating thunks themselves in the linker. While for export thunks any unresolved
EXP+...symbol is sufficient, redirection metadata logic is more strict and, according to my experiments, function symbol type is important and enough to make it work.There is a different logic for exported symbols, those already work fine with hybrid patchable functions, but for a different reason. I think that in practice that metadata is not actually needed for functions that are not exported. Call checkers don't need it, it's probably mostly used for auxiliary IAT handling. If that's right, then this fix won't change much in practice. But since it's undocumented, I can't be sure so I think it's better to just get it right and make sure that generated metadata matches what would be seen with MSVC.