[lld] [LLD][COFF] Process all live import symbols in getSymbols() (PR #109117)
Jacek Caban via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 18 02:48:02 PDT 2024
https://github.com/cjacek created https://github.com/llvm/llvm-project/pull/109117
The current logic assumes that the import file is pulled by object files, and the loop for import files only needs to handle cases where the `__imp_` symbol is implicitly pulled by an import thunk. This is fragile, as the symbol may also be pulled through other means, such as the -export argument in tests. Additionally, this logic is insufficient for ARM64EC, which exposes multiple symbols through an import file, and referencing any one of them causes all of them to be defined.
With this change, import symbols are added to `syms` more often, but we ensure that output symbols remain unique later in the process
>From 4beffdd8de68f764a0d86bac2be6bb7db5ae196a Mon Sep 17 00:00:00 2001
From: Jacek Caban <jacek at codeweavers.com>
Date: Tue, 17 Sep 2024 23:45:44 +0200
Subject: [PATCH 1/2] [LLD][COFF] Store __imp_ symbols as Defined in InputFile
---
lld/COFF/InputFiles.h | 4 ++--
lld/COFF/MapFile.cpp | 3 +--
lld/COFF/SymbolTable.cpp | 6 +++---
lld/COFF/SymbolTable.h | 4 ++--
4 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 5fa93f57ef9e3a..a20b097cbe04af 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -349,7 +349,7 @@ class ImportFile : public InputFile {
MachineTypes getMachineType() const override;
DefinedImportData *impSym = nullptr;
- Symbol *thunkSym = nullptr;
+ Defined *thunkSym = nullptr;
ImportThunkChunkARM64EC *impchkThunk = nullptr;
std::string dllName;
@@ -365,7 +365,7 @@ class ImportFile : public InputFile {
// Auxiliary IAT symbols and chunks on ARM64EC.
DefinedImportData *impECSym = nullptr;
Chunk *auxLocation = nullptr;
- Symbol *auxThunkSym = nullptr;
+ Defined *auxThunkSym = nullptr;
DefinedImportData *auxImpCopySym = nullptr;
Chunk *auxCopyLocation = nullptr;
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index 52e9ce996f2390..69842ced284cbd 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -128,8 +128,7 @@ static void getSymbols(const COFFLinkerContext &ctx,
if (!file->thunkSym->isLive())
continue;
- if (auto *thunkSym = dyn_cast<Defined>(file->thunkSym))
- syms.push_back(thunkSym);
+ syms.push_back(thunkSym);
if (auto *impSym = dyn_cast_or_null<Defined>(file->impSym))
syms.push_back(impSym);
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index efea16ccbbfea0..fc169ec211a9fe 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -815,13 +815,13 @@ DefinedImportData *SymbolTable::addImportData(StringRef n, ImportFile *f,
return nullptr;
}
-Symbol *SymbolTable::addImportThunk(StringRef name, DefinedImportData *id,
- ImportThunkChunk *chunk) {
+Defined *SymbolTable::addImportThunk(StringRef name, DefinedImportData *id,
+ ImportThunkChunk *chunk) {
auto [s, wasInserted] = insert(name, nullptr);
s->isUsedInRegularObj = true;
if (wasInserted || isa<Undefined>(s) || s->isLazy()) {
replaceSymbol<DefinedImportThunk>(s, ctx, name, id, chunk);
- return s;
+ return cast<Defined>(s);
}
reportDuplicate(s, id->file);
diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index bf97cf442039e0..e3f674b8098f8b 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -105,8 +105,8 @@ class SymbolTable {
CommonChunk *c = nullptr);
DefinedImportData *addImportData(StringRef n, ImportFile *f,
Chunk *&location);
- Symbol *addImportThunk(StringRef name, DefinedImportData *s,
- ImportThunkChunk *chunk);
+ Defined *addImportThunk(StringRef name, DefinedImportData *s,
+ ImportThunkChunk *chunk);
void addLibcall(StringRef name);
void addEntryThunk(Symbol *from, Symbol *to);
void addExitThunk(Symbol *from, Symbol *to);
>From 22125606aac91300557769db2a8ce0ca6c24af80 Mon Sep 17 00:00:00 2001
From: Jacek Caban <jacek at codeweavers.com>
Date: Wed, 18 Sep 2024 00:00:34 +0200
Subject: [PATCH 2/2] [LLD][COFF] Process all live import symbols in
getSymbols()
The current logic assumes that the import file is pulled by object files,
and the loop for import files only needs to handle cases where the __imp_
symbol is implicitly pulled by an import thunk. This is fragile, as the
symbol may also be pulled through other means, such as the -export argument
in tests. Additionally, this logic is insufficient for ARM64EC, which
exposes multiple symbols through an import file, and referencing any one
of them causes all of them to be defined.
With this change, import symbols are added to syms more often, but we
ensure that output symbols remain unique later in the process.
---
lld/COFF/MapFile.cpp | 13 +++----------
lld/test/COFF/export-imp.test | 11 +++++++++++
2 files changed, 14 insertions(+), 10 deletions(-)
create mode 100644 lld/test/COFF/export-imp.test
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index 69842ced284cbd..b9ef32da113c4f 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -122,16 +122,9 @@ static void getSymbols(const COFFLinkerContext &ctx,
if (!file->live)
continue;
- if (!file->thunkSym)
- continue;
-
- if (!file->thunkSym->isLive())
- continue;
-
- syms.push_back(thunkSym);
-
- if (auto *impSym = dyn_cast_or_null<Defined>(file->impSym))
- syms.push_back(impSym);
+ syms.push_back(file->impSym);
+ if (file->thunkSym && file->thunkSym->isLive())
+ syms.push_back(file->thunkSym);
}
sortUniqueSymbols(syms, ctx.config.imageBase);
diff --git a/lld/test/COFF/export-imp.test b/lld/test/COFF/export-imp.test
new file mode 100644
index 00000000000000..db71ef74641639
--- /dev/null
+++ b/lld/test/COFF/export-imp.test
@@ -0,0 +1,11 @@
+; REQUIRES: x86
+
+; RUN: llvm-lib -machine:amd64 -out:%t.imp.lib -def:%s
+; RUN: lld-link -machine:amd64 -out:%t.dll %t.imp.lib -dll -noentry -export:__imp_func,DATA -map
+
+; FileCheck %s < %t.imp.map
+; CHECK: 0001:00000098 __imp_func 0000000180001098 export-imp-thunk.test.tmp.imp:imp.dll
+
+LIBRARY imp.dll
+EXPORTS
+ func
More information about the llvm-commits
mailing list