[LLD][COFF] Add support for ARM64EC import call thunks. #107931
Merged
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-platform-windows @llvm/pr-subscribers-lld-coff Author: Jacek Caban (cjacek) ChangesThese thunks can be accessed using Full diff: https://github.com/llvm/llvm-project/pull/107931.diff 15 Files Affected:
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 060eb6c32004d2..2bf0d644f9820d 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -1093,4 +1093,21 @@ void CHPERedirectionChunk::writeTo(uint8_t *buf) const {
}
}
+ImportThunkChunkARM64EC::ImportThunkChunkARM64EC(ImportFile *file)
+ : ImportThunkChunk(file->ctx, file->impSym), file(file) {}
+
+void ImportThunkChunkARM64EC::writeTo(uint8_t *buf) const {
+ memcpy(buf, importThunkARM64EC, sizeof(importThunkARM64EC));
+ applyArm64Addr(buf, file->impSym->getRVA(), rva, 12);
+ applyArm64Ldr(buf + 4, file->impSym->getRVA() & 0xfff);
+
+ uint32_t exitThunkRVA = exitThunk ? exitThunk->getRVA() : 0;
+ applyArm64Addr(buf + 8, exitThunkRVA, rva + 8, 12);
+ applyArm64Imm(buf + 12, exitThunkRVA & 0xfff, 0);
+
+ Defined *helper = dyn_cast<Defined>(file->ctx.config.arm64ECIcallHelper);
+ if (helper)
+ applyArm64Branch26(buf + 16, helper->getRVA() - rva - 16);
+}
+
} // namespace lld::coff
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 30e5b538c352ea..28e0fd68ac5159 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -544,6 +544,14 @@ static const uint8_t importThunkARM64[] = {
0x00, 0x02, 0x1f, 0xd6, // br x16
};
+static const uint32_t importThunkARM64EC[] = {
+ 0x9000000b, // adrp x11, 0x0
+ 0xf940016b, // ldr x11, [x11]
+ 0x9000000a, // adrp x10, 0x0
+ 0x9100014a, // add x10, x10, #0x0
+ 0x14000000 // b 0x0
+};
+
// Windows-specific.
// A chunk for DLL import jump table entry. In a final output, its
// contents will be a JMP instruction to some __imp_ symbol.
@@ -599,6 +607,22 @@ class ImportThunkChunkARM64 : public ImportThunkChunk {
MachineTypes getMachine() const override { return ARM64; }
};
+// ARM64EC __impchk_* thunk implementation.
+// Performs an indirect call to an imported function pointer
+// using the __icall_helper_arm64ec helper function.
+class ImportThunkChunkARM64EC : public ImportThunkChunk {
+public:
+ explicit ImportThunkChunkARM64EC(ImportFile *file);
+ size_t getSize() const override { return sizeof(importThunkARM64EC); };
+ MachineTypes getMachine() const override { return ARM64EC; }
+ void writeTo(uint8_t *buf) const override;
+
+ Defined *exitThunk;
+
+private:
+ ImportFile *file;
+};
+
class RangeExtensionThunkARM : public NonSectionCodeChunk {
public:
explicit RangeExtensionThunkARM(COFFLinkerContext &ctx, Defined *t)
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 947f3fead54e03..738776a971ea39 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -164,6 +164,7 @@ struct Configuration {
std::set<std::string> delayLoads;
std::map<std::string, int> dllOrder;
Symbol *delayLoadHelper = nullptr;
+ Symbol *arm64ECIcallHelper = nullptr;
bool saveTemps = false;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 3ef9fa3f65c6a6..a1fe6444991a36 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1383,6 +1383,11 @@ void LinkerDriver::createECExportThunks() {
}
}
+void LinkerDriver::pullArm64ECIcallHelper() {
+ if (!ctx.config.arm64ECIcallHelper)
+ ctx.config.arm64ECIcallHelper = addUndefined("__icall_helper_arm64ec");
+}
+
// In MinGW, if no symbols are chosen to be exported, then all symbols are
// automatically exported by default. This behavior can be forced by the
// -export-all-symbols option, so that it happens even when exports are
@@ -2685,7 +2690,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (auto *arg = args.getLastArg(OPT_print_symbol_order))
config->printSymbolOrder = arg->getValue();
- ctx.symtab.initializeEntryThunks();
+ ctx.symtab.initializeECThunks();
// Identify unreferenced COMDAT sections.
if (config->doGC) {
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index b5cf8e2f18fd4e..0c195a7cc3148c 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -101,6 +101,8 @@ class LinkerDriver {
std::unique_ptr<llvm::TarWriter> tar; // for /linkrepro
+ void pullArm64ECIcallHelper();
+
private:
// Searches a file from search paths.
std::optional<StringRef> findFileIfNew(StringRef filename);
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index fa2d230075d9d3..e3cf5f03630b06 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -190,6 +190,8 @@ void ObjFile::initializeECThunks() {
ctx.symtab.addEntryThunk(getSymbol(entry->src), getSymbol(entry->dst));
break;
case Arm64ECThunkType::Exit:
+ ctx.symtab.addExitThunk(getSymbol(entry->src), getSymbol(entry->dst));
+ break;
case Arm64ECThunkType::GuestExit:
break;
default:
@@ -1069,10 +1071,33 @@ void ImportFile::parse() {
// DLL functions just like regular non-DLL functions.)
if (hdr->getType() == llvm::COFF::IMPORT_CODE) {
if (ctx.config.machine != ARM64EC) {
- thunkSym = ctx.symtab.addImportThunk(name, impSym, hdr->Machine);
+ ImportThunkChunk *chunk;
+ switch (hdr->Machine) {
+ case AMD64:
+ chunk = make<ImportThunkChunkX64>(ctx, impSym);
+ break;
+ case I386:
+ chunk = make<ImportThunkChunkX86>(ctx, impSym);
+ break;
+ case ARM64:
+ chunk = make<ImportThunkChunkARM64>(ctx, impSym);
+ break;
+ case ARMNT:
+ chunk = make<ImportThunkChunkARM>(ctx, impSym);
+ break;
+ default:
+ llvm_unreachable("unknown machine type");
+ }
+ thunkSym = ctx.symtab.addImportThunk(name, impSym, chunk);
} else {
- thunkSym = ctx.symtab.addImportThunk(name, impSym, AMD64);
+ thunkSym = ctx.symtab.addImportThunk(
+ name, impSym, make<ImportThunkChunkX64>(ctx, impSym));
// FIXME: Add aux IAT symbols.
+
+ StringRef impChkName = saver().save("__impchk_" + name);
+ impchkThunk = make<ImportThunkChunkARM64EC>(this);
+ ctx.symtab.addImportThunk(impChkName, impSym, impchkThunk);
+ ctx.driver.pullArm64ECIcallHelper();
}
}
}
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 8b3303a8d87f45..af2f85739febbb 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -55,6 +55,7 @@ class Defined;
class DefinedImportData;
class DefinedImportThunk;
class DefinedRegular;
+class ImportThunkChunkARM64EC;
class SectionChunk;
class Symbol;
class Undefined;
@@ -348,6 +349,7 @@ class ImportFile : public InputFile {
DefinedImportData *impSym = nullptr;
Symbol *thunkSym = nullptr;
+ ImportThunkChunkARM64EC *impchkThunk = nullptr;
std::string dllName;
private:
diff --git a/lld/COFF/MarkLive.cpp b/lld/COFF/MarkLive.cpp
index 06079a98f2d00a..8af58780e13582 100644
--- a/lld/COFF/MarkLive.cpp
+++ b/lld/COFF/MarkLive.cpp
@@ -43,13 +43,23 @@ void markLive(COFFLinkerContext &ctx) {
worklist.push_back(c);
};
- auto addSym = [&](Symbol *b) {
- if (auto *sym = dyn_cast<DefinedRegular>(b))
+ std::function<void(Symbol *)> addSym;
+
+ auto addImportFile = [&](ImportFile *file) {
+ file->live = true;
+ if (file->impchkThunk && file->impchkThunk->exitThunk)
+ addSym(file->impchkThunk->exitThunk);
+ };
+
+ addSym = [&](Symbol *b) {
+ if (auto *sym = dyn_cast<DefinedRegular>(b)) {
enqueue(sym->getChunk());
- else if (auto *sym = dyn_cast<DefinedImportData>(b))
- sym->file->live = true;
- else if (auto *sym = dyn_cast<DefinedImportThunk>(b))
- sym->wrappedSym->file->live = sym->wrappedSym->file->thunkLive = true;
+ } else if (auto *sym = dyn_cast<DefinedImportData>(b)) {
+ addImportFile(sym->file);
+ } else if (auto *sym = dyn_cast<DefinedImportThunk>(b)) {
+ addImportFile(sym->wrappedSym->file);
+ sym->wrappedSym->file->thunkLive = true;
+ }
};
// Add GC root chunks.
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index bb7583bb9a7df4..a6575ecac3bb44 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -557,7 +557,11 @@ void SymbolTable::addEntryThunk(Symbol *from, Symbol *to) {
entryThunks.push_back({from, to});
}
-void SymbolTable::initializeEntryThunks() {
+void SymbolTable::addExitThunk(Symbol *from, Symbol *to) {
+ exitThunks[from] = to;
+}
+
+void SymbolTable::initializeECThunks() {
for (auto it : entryThunks) {
auto *to = dyn_cast<Defined>(it.second);
if (!to)
@@ -573,6 +577,16 @@ void SymbolTable::initializeEntryThunks() {
}
from->getChunk()->setEntryThunk(to);
}
+
+ for (ImportFile *file : ctx.importFileInstances) {
+ if (!file->impchkThunk)
+ continue;
+
+ Symbol *sym = exitThunks.lookup(file->thunkSym);
+ if (!sym)
+ sym = exitThunks.lookup(file->impSym);
+ file->impchkThunk->exitThunk = dyn_cast_or_null<Defined>(sym);
+ }
}
Symbol *SymbolTable::addUndefined(StringRef name, InputFile *f,
@@ -784,11 +798,11 @@ DefinedImportData *SymbolTable::addImportData(StringRef n, ImportFile *f) {
}
Symbol *SymbolTable::addImportThunk(StringRef name, DefinedImportData *id,
- uint16_t machine) {
+ ImportThunkChunk *chunk) {
auto [s, wasInserted] = insert(name, nullptr);
s->isUsedInRegularObj = true;
if (wasInserted || isa<Undefined>(s) || s->isLazy()) {
- replaceSymbol<DefinedImportThunk>(s, ctx, name, id, machine);
+ replaceSymbol<DefinedImportThunk>(s, ctx, name, id, chunk);
return s;
}
diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index 51c6c79ec14463..13e151e3a8c501 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -28,6 +28,7 @@ class COFFLinkerContext;
class Defined;
class DefinedAbsolute;
class DefinedRegular;
+class ImportThunkChunk;
class LazyArchive;
class SectionChunk;
class Symbol;
@@ -104,10 +105,11 @@ class SymbolTable {
CommonChunk *c = nullptr);
DefinedImportData *addImportData(StringRef n, ImportFile *f);
Symbol *addImportThunk(StringRef name, DefinedImportData *s,
- uint16_t machine);
+ ImportThunkChunk *chunk);
void addLibcall(StringRef name);
void addEntryThunk(Symbol *from, Symbol *to);
- void initializeEntryThunks();
+ void addExitThunk(Symbol *from, Symbol *to);
+ void initializeECThunks();
void reportDuplicate(Symbol *existing, InputFile *newFile,
SectionChunk *newSc = nullptr,
@@ -140,6 +142,7 @@ class SymbolTable {
std::unique_ptr<BitcodeCompiler> lto;
bool ltoCompilationDone = false;
std::vector<std::pair<Symbol *, Symbol *>> entryThunks;
+ llvm::DenseMap<Symbol *, Symbol *> exitThunks;
COFFLinkerContext &ctx;
};
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index b098abb80d6f1e..5f4d797f74a2dd 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -107,22 +107,10 @@ COFFSymbolRef DefinedCOFF::getCOFFSymbol() {
uint64_t DefinedAbsolute::getRVA() { return va - ctx.config.imageBase; }
-static Chunk *makeImportThunk(COFFLinkerContext &ctx, DefinedImportData *s,
- uint16_t machine) {
- if (machine == AMD64)
- return make<ImportThunkChunkX64>(ctx, s);
- if (machine == I386)
- return make<ImportThunkChunkX86>(ctx, s);
- if (machine == ARM64)
- return make<ImportThunkChunkARM64>(ctx, s);
- assert(machine == ARMNT);
- return make<ImportThunkChunkARM>(ctx, s);
-}
-
DefinedImportThunk::DefinedImportThunk(COFFLinkerContext &ctx, StringRef name,
- DefinedImportData *s, uint16_t machine)
- : Defined(DefinedImportThunkKind, name), wrappedSym(s),
- data(makeImportThunk(ctx, s, machine)) {}
+ DefinedImportData *s,
+ ImportThunkChunk *chunk)
+ : Defined(DefinedImportThunkKind, name), wrappedSym(s), data(chunk) {}
Defined *Undefined::getWeakAlias() {
// A weak alias may be a weak alias to another symbol, so check recursively.
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index c427a062dc82b2..724330e4bab958 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -388,7 +388,7 @@ class DefinedImportData : public Defined {
class DefinedImportThunk : public Defined {
public:
DefinedImportThunk(COFFLinkerContext &ctx, StringRef name,
- DefinedImportData *s, uint16_t machine);
+ DefinedImportData *s, ImportThunkChunk *chunk);
static bool classof(const Symbol *s) {
return s->kind() == DefinedImportThunkKind;
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 3cb9b3b512ead6..b589a16bca32a3 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1248,6 +1248,8 @@ void Writer::appendImportThunks() {
DefinedImportThunk *thunk = cast<DefinedImportThunk>(file->thunkSym);
if (file->thunkLive)
textSec->addChunk(thunk->getChunk());
+ if (file->impchkThunk)
+ textSec->addChunk(file->impchkThunk);
}
if (!delayIdata.empty()) {
diff --git a/lld/test/COFF/Inputs/loadconfig-arm64ec.s b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
index 78e7fba43a0a4d..75dc6105301d00 100644
--- a/lld/test/COFF/Inputs/loadconfig-arm64ec.s
+++ b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
@@ -30,6 +30,8 @@ __os_arm64x_dispatch_ret:
.xword 0
__os_arm64x_check_call:
.xword 0
+ .globl __os_arm64x_dispatch_icall
+__os_arm64x_dispatch_icall:
__os_arm64x_check_icall:
.xword 0
__os_arm64x_get_x64_information:
diff --git a/lld/test/COFF/arm64ec-import.test b/lld/test/COFF/arm64ec-import.test
index b1c47d785e445b..3a7ef74900f679 100644
--- a/lld/test/COFF/arm64ec-import.test
+++ b/lld/test/COFF/arm64ec-import.test
@@ -2,17 +2,19 @@ REQUIRES: aarch64, x86
RUN: split-file %s %t.dir && cd %t.dir
RUN: llvm-mc -filetype=obj -triple=arm64ec-windows test.s -o test.obj
+RUN: llvm-mc -filetype=obj -triple=arm64ec-windows icall.s -o icall.obj
+RUN: llvm-mc -filetype=obj -triple=arm64ec-windows hybmp.s -o hybmp.obj
RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %S/Inputs/loadconfig-arm64ec.s -o loadconfig-arm64ec.obj
RUN: llvm-lib -machine:arm64ec -def:test.def -out:test-arm64ec.lib
RUN: llvm-lib -machine:arm64ec -def:test2.def -out:test2-arm64ec.lib
RUN: llvm-lib -machine:x64 -def:test.def -out:test-x86_64.lib
Link using ARM64EC import library:
-RUN: lld-link -machine:arm64ec -dll -noentry -out:out.dll loadconfig-arm64ec.obj \
+RUN: lld-link -machine:arm64ec -dll -noentry -out:out.dll loadconfig-arm64ec.obj icall.obj hybmp.obj \
RUN: test.obj test-arm64ec.lib test2-arm64ec.lib
Link using x86_64 import library:
-RUN: lld-link -machine:arm64ec -dll -noentry -out:out2.dll loadconfig-arm64ec.obj \
+RUN: lld-link -machine:arm64ec -dll -noentry -out:out2.dll loadconfig-arm64ec.obj icall.obj hybmp.obj \
RUN: test.obj test-x86_64.lib test2-arm64ec.lib
RUN: llvm-readobj --coff-imports out.dll | FileCheck --check-prefix=IMPORTS %s
@@ -20,7 +22,7 @@ RUN: llvm-readobj --coff-imports out2.dll | FileCheck --check-prefix=IMPORTS %s
IMPORTS: Import {
IMPORTS-NEXT: Name: test.dll
IMPORTS-NEXT: ImportLookupTableRVA:
-IMPORTS-NEXT: ImportAddressTableRVA: 0x2000
+IMPORTS-NEXT: ImportAddressTableRVA: 0x3000
IMPORTS-NEXT: Symbol: data (0)
IMPORTS-NEXT: Symbol: func (0)
IMPORTS-NEXT: Symbol: func2 (0)
@@ -28,24 +30,43 @@ IMPORTS-NEXT: }
IMPORTS-NEXT: Import {
IMPORTS-NEXT: Name: test2.dll
IMPORTS-NEXT: ImportLookupTableRVA:
-IMPORTS-NEXT: ImportAddressTableRVA: 0x2020
+IMPORTS-NEXT: ImportAddressTableRVA: 0x3020
IMPORTS-NEXT: Symbol: t2func (0)
IMPORTS-NEXT: }
RUN: llvm-objdump -d out.dll | FileCheck --check-prefix=DISASM %s
RUN: llvm-objdump -d out2.dll | FileCheck --check-prefix=DISASM %s
-DISASM: 0000000180001000 <.text>:
-DISASM-NEXT: 180001000: ff 25 02 10 00 00 jmpq *0x1002(%rip) # 0x180002008
+DISASM: 180001000: 52800000 mov w0, #0x0 // =0
+DISASM-NEXT: 180001004: d65f03c0 ret
+DISASM-NEXT: 180001008: d000000b adrp x11, 0x180003000
+DISASM-NEXT: 18000100c: f940056b ldr x11, [x11, #0x8]
+DISASM-NEXT: 180001010: 9000000a adrp x10, 0x180001000 <.text>
+DISASM-NEXT: 180001014: 9101114a add x10, x10, #0x44
+DISASM-NEXT: 180001018: 17fffffa b 0x180001000 <.text>
+DISASM-NEXT: 18000101c: d000000b adrp x11, 0x180003000
+DISASM-NEXT: 180001020: f940096b ldr x11, [x11, #0x10]
+DISASM-NEXT: 180001024: f0ffffea adrp x10, 0x180000000
+DISASM-NEXT: 180001028: 9100014a add x10, x10, #0x0
+DISASM-NEXT: 18000102c: 17fffff5 b 0x180001000 <.text>
+DISASM-NEXT: 180001030: d000000b adrp x11, 0x180003000
+DISASM-NEXT: 180001034: f940116b ldr x11, [x11, #0x20]
+DISASM-NEXT: 180001038: 9000000a adrp x10, 0x180001000 <.text>
+DISASM-NEXT: 18000103c: 9101314a add x10, x10, #0x4c
+DISASM-NEXT: 180001040: 17fffff0 b 0x180001000 <.text>
+DISASM-NEXT: 180001044: 52800020 mov w0, #0x1 // =1
+DISASM-NEXT: 180001048: d65f03c0 ret
+DISASM-NEXT: 18000104c: 52800040 mov w0, #0x2 // =2
+DISASM-NEXT: 180001050: d65f03c0 ret
RUN: llvm-readobj --hex-dump=.test out.dll | FileCheck --check-prefix=TESTSEC %s
RUN: llvm-readobj --hex-dump=.test out2.dll | FileCheck --check-prefix=TESTSEC %s
-TESTSEC: 0x180005000 08200000 00200000 10200000 20200000
-TESTSEC-NEXT: 0x180005010 00100000
+TESTSEC: 0x180006000 08300000 00300000 10300000 20300000
+TESTSEC-NEXT: 0x180006010 08100000 1c100000 00200000
RUN: llvm-readobj --headers out.dll | FileCheck -check-prefix=HEADERS %s
-HEADERS: LoadConfigTableRVA: 0x3008
-HEADERS: IATRVA: 0x2000
+HEADERS: LoadConfigTableRVA: 0x4010
+HEADERS: IATRVA: 0x3000
HEADERS: IATSize: 0x1000
#--- test.s
@@ -57,8 +78,49 @@ arm64ec_data_sym:
.rva __imp_data
.rva __imp_func2
.rva __imp_t2func
+ .rva __impchk_func
+ .rva __impchk_func2
.rva func
+#--- icall.s
+ .text
+ .globl __icall_helper_arm64ec
+ .p2align 2, 0x0
+__icall_helper_arm64ec:
+ mov w0, #0
+ ret
+
+#--- hybmp.s
+ .section .hybmp$x, "yi"
+ // __imp_func exit thunk is ignored when func is defined as well
+ .symidx __imp_func
+ .symidx dead_exit_thunk
+ .word 4
+ .symidx func
+ .symidx func_exit_thunk
+ .word 4
+ .symidx __imp_t2func
+ .symidx t2func_exit_thunk
+ .word 4
+
+ .section .wowthk$aa,"xr",discard,func_exit_thunk
+ .globl func_exit_thunk
+func_exit_thunk:
+ mov w0, #1
+ ret
+
+ .section .wowthk$aa,"xr",discard,t2func_exit_thunk
+ .globl t2func_exit_thunk
+t2func_exit_thunk:
+ mov w0, #2
+ ret
+
+ .section .wowthk$aa,"xr",discard,dead_exit_thunk
+ .globl dead_exit_thunk
+dead_exit_thunk:
+ mov w0, #0xdead
+ ret
+
#--- test.def
NAME test.dll
EXPORTS
|
|
Depends on #107929. In its current form, this may cause a relocation error if the offset The |
cjacek
force-pushed
the
arm64ec-imp-thunk
branch
from
37a1591 to
9c53673
Compare
mstorsjo
approved these changes
Sep 10, 2024
| applyArm64Addr(buf, file->impSym->getRVA(), rva, 12); | ||
| applyArm64Ldr(buf + 4, file->impSym->getRVA() & 0xfff); | ||
|
|
||
| uint32_t exitThunkRVA = exitThunk ? exitThunk->getRVA() : 0; |
There was a problem hiding this comment.
If this is 0, won't this end up with a rather bogus instruction sequence? Shouldn't we replace the adrp+add with a mov x0, #0 or similar instead, in that case?
There was a problem hiding this comment.
Yes, this behavior would be bogus. I followed MSVC's approach in these cases and implemented it the same way (since it's an RVA, it sets x10 to the image base, which requires addrp + add). We could either zero x10 or leave it unchanged in these cases. It's unlikely to matter for the OS loader, so I can make this change if you prefer.
Emitting a warning in such scenarios might be tempting, but we can’t do that because there are valid situations where an exit thunk may be missing. For example, if an application only ever takes the function’s address and never calls it directly via the import table. In these cases, the application uses __imp_aux_* symbols instead of the regular __imp_* symbols. We still need to populate the auxiliary IAT, which requires those thunks, even if they’re never used.
If we tracked __imp_ and __imp_aux_ usage separately, we might be able to skip the thunk and fill the auxiliary IAT with zeroes or something similar. However, since this behavior isn’t documented, skipping thunks would require ensuring the OS loader can handle it in all cases, which is tricky to verify with black-box testing. Therefore, it feels safer to follow MSVC’s behavior.
Another situation where exit thunks may not be available is when hand-written assembly is used to make the call. In such cases, if the callee is ARM64EC, it should still work because the thunk would usually be skipped. It’s possible the thunk wouldn’t be skipped (for example, if patching is detected in the callee’s area but not the callee itself). If the call goes through the thunk, __icall_helper_arm64ec wouldn’t use an exit thunk if the callee is ARM64EC. It would only fail if the callee isn’t ARM64EC, which the developer may know will never be the case.
There was a problem hiding this comment.
Ok, fair enough. So if the symbol is missing, we end up with RVA 0, which ends up in producing an adrp+add pair that produces the image base address, which is passed to __icall_helper_arm64ec, which wouldn't use it in this case - that sounds ok with me.
Perhaps a comment, saying that this 0 case can happen, but in these cases, the address ends up ignored - if I'm understanding the issue correctly?
There was a problem hiding this comment.
Yes, it may happen in valid cases, but be either ignored by __icall_helper_arm64ec or the thole thunk may be unused/skipped.
There was a problem hiding this comment.
I will add a comment, thanks!
lld/COFF/Chunks.cpp
Outdated
|
|
||
| Defined *helper = dyn_cast<Defined>(file->ctx.config.arm64ECIcallHelper); | ||
| if (helper) | ||
| applyArm64Branch26(buf + 16, helper->getRVA() - rva - 16); |
There was a problem hiding this comment.
Same here - if this is undefined, won't the instruction end up quite bogus? But I guess this shouldn't really happen?
There was a problem hiding this comment.
I intended to handle the -force argument but made an error. I'll drop the check. In its current form, it will fail to relocate if the helper isn't defined, as 0 is out of range, but it will work once range extension is implemented.
cjacek
force-pushed
the
arm64ec-imp-thunk
branch
from
9c53673 to
09d5f9b
Compare
These thunks can be accessed using __impchk_* symbols, though they are typically not called directly. Instead, they are used to populate the auxiliary IAT. When the imported function is x86_64 (or an ARM64EC function with a patched export thunk), the thunk is used to call it. Otherwise, the OS may replace the thunk at runtime with a direct pointer to the ARM64EC function to avoid the overhead.
cjacek
force-pushed
the
arm64ec-imp-thunk
branch
from
09d5f9b to
dd33de1
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/143/builds/2076 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/2598 Here is the relevant piece of the build log for the reference |
|
The test is failing on s390x with this output: I expect the issue is that you're assuming a little-endian host -- the data in importThunkARM64EC should probably be specified using bytes instead of int32_t? |
|
Yes, good point, it should use bytes instead, sorry about that. I will push a fixup, thanks! |
cjacek
added a commit
that referenced
this pull request
Sep 12, 2024
…ian hosts. Fixes #107931, spotted by Nikita Popov.
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.
These thunks can be accessed using
__impchk_*symbols, though they aretypically not called directly. Instead, they are used to populate the
auxiliary IAT. When the imported function is x86_64 (or an ARM64EC
function with a patched export thunk), the thunk is used to call it.
Otherwise, the OS may replace the thunk at runtime with a direct
pointer to the ARM64EC function to avoid the overhead.