[lld] adcdc9c - [LLD][COFF] Allow overwriting directives exports with cmd-line exports
Alexandre Ganea via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 13 12:29:53 PDT 2023
Author: Alexandre Ganea
Date: 2023-06-13T15:29:46-04:00
New Revision: adcdc9cc3740adba3577b328fa3ba492cbccd3a5
URL: https://github.com/llvm/llvm-project/commit/adcdc9cc3740adba3577b328fa3ba492cbccd3a5
DIFF: https://github.com/llvm/llvm-project/commit/adcdc9cc3740adba3577b328fa3ba492cbccd3a5.diff
LOG: [LLD][COFF] Allow overwriting directives exports with cmd-line exports
MSVC link.exe allows overriding exports on the cmd-line with exports seen in OBJ directives. The typical case is what is described in #62329.
Before this patch, trying to override an export with `/export` or `/def` would generate a duplicate warning. This patches tries to replicate the MSVC behavior. A second override on the cmd-line would still generate the warning.
There's still a case which we don't cover: MSVC link.exe is able to demangle an exported OBJ directive function, and match it with a unmangled export function in a .def file. In the meanwhile, one can use the mangled export in the .def to cover that case.
This fixes #62329
Differential revision: https://reviews.llvm.org/D149611
Added:
lld/test/COFF/ordinals-override.test
Modified:
lld/COFF/Config.h
lld/COFF/DLL.cpp
lld/COFF/Driver.cpp
lld/COFF/DriverUtils.cpp
lld/test/COFF/export.test
lld/test/COFF/export32.test
Removed:
################################################################################
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 029c233e4544a..a870211d3ca6e 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -42,6 +42,13 @@ static const auto ARM64X = llvm::COFF::IMAGE_FILE_MACHINE_ARM64X;
static const auto ARMNT = llvm::COFF::IMAGE_FILE_MACHINE_ARMNT;
static const auto I386 = llvm::COFF::IMAGE_FILE_MACHINE_I386;
+enum class ExportSource {
+ Unset,
+ Directives,
+ Export,
+ ModuleDefinition,
+};
+
// Represents an /export option.
struct Export {
StringRef name; // N in /export:N or /export:E=N
@@ -60,8 +67,7 @@ struct Export {
StringRef forwardTo;
StringChunk *forwardChunk = nullptr;
- // True if this /export option was in .drectves section.
- bool directives = false;
+ ExportSource source = ExportSource::Unset;
StringRef symbolName;
StringRef exportName; // Name in DLL
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 417b7041fbf01..5977970104672 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -555,8 +555,10 @@ class DelayAddressChunk : public NonSectionChunk {
// A chunk for the export descriptor table.
class ExportDirectoryChunk : public NonSectionChunk {
public:
- ExportDirectoryChunk(int i, int j, Chunk *d, Chunk *a, Chunk *n, Chunk *o)
- : maxOrdinal(i), nameTabSize(j), dllName(d), addressTab(a), nameTab(n),
+ ExportDirectoryChunk(int baseOrdinal, int maxOrdinal, int nameTabSize,
+ Chunk *d, Chunk *a, Chunk *n, Chunk *o)
+ : baseOrdinal(baseOrdinal), maxOrdinal(maxOrdinal),
+ nameTabSize(nameTabSize), dllName(d), addressTab(a), nameTab(n),
ordinalTab(o) {}
size_t getSize() const override {
@@ -568,14 +570,15 @@ class ExportDirectoryChunk : public NonSectionChunk {
auto *e = (export_directory_table_entry *)(buf);
e->NameRVA = dllName->getRVA();
- e->OrdinalBase = 1;
- e->AddressTableEntries = maxOrdinal;
+ e->OrdinalBase = baseOrdinal;
+ e->AddressTableEntries = (maxOrdinal - baseOrdinal) + 1;
e->NumberOfNamePointers = nameTabSize;
e->ExportAddressTableRVA = addressTab->getRVA();
e->NamePointerRVA = nameTab->getRVA();
e->OrdinalTableRVA = ordinalTab->getRVA();
}
+ uint16_t baseOrdinal;
uint16_t maxOrdinal;
uint16_t nameTabSize;
Chunk *dllName;
@@ -586,17 +589,19 @@ class ExportDirectoryChunk : public NonSectionChunk {
class AddressTableChunk : public NonSectionChunk {
public:
- explicit AddressTableChunk(COFFLinkerContext &ctx, size_t maxOrdinal)
- : size(maxOrdinal), ctx(ctx) {}
+ explicit AddressTableChunk(COFFLinkerContext &ctx, size_t baseOrdinal,
+ size_t maxOrdinal)
+ : baseOrdinal(baseOrdinal), size((maxOrdinal - baseOrdinal) + 1),
+ ctx(ctx) {}
size_t getSize() const override { return size * 4; }
void writeTo(uint8_t *buf) const override {
memset(buf, 0, getSize());
for (const Export &e : ctx.config.exports) {
- assert(e.ordinal != 0 && "Export symbol has invalid ordinal");
- // OrdinalBase is 1, so subtract 1 to get the index.
- uint8_t *p = buf + (e.ordinal - 1) * 4;
+ assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
+ // Subtract the OrdinalBase to get the index.
+ uint8_t *p = buf + (e.ordinal - baseOrdinal) * 4;
uint32_t bit = 0;
// Pointer to thumb code must have the LSB set, so adjust it.
if (ctx.config.machine == ARMNT && !e.data)
@@ -612,6 +617,7 @@ class AddressTableChunk : public NonSectionChunk {
}
private:
+ size_t baseOrdinal;
size_t size;
const COFFLinkerContext &ctx;
};
@@ -634,22 +640,24 @@ class NamePointersChunk : public NonSectionChunk {
class ExportOrdinalChunk : public NonSectionChunk {
public:
- explicit ExportOrdinalChunk(const COFFLinkerContext &ctx, size_t i)
- : size(i), ctx(ctx) {}
+ explicit ExportOrdinalChunk(const COFFLinkerContext &ctx, size_t baseOrdinal,
+ size_t tableSize)
+ : baseOrdinal(baseOrdinal), size(tableSize), ctx(ctx) {}
size_t getSize() const override { return size * 2; }
void writeTo(uint8_t *buf) const override {
for (const Export &e : ctx.config.exports) {
if (e.noname)
continue;
- assert(e.ordinal != 0 && "Export symbol has invalid ordinal");
- // This table stores unbiased indices, so subtract 1 (OrdinalBase).
- write16le(buf, e.ordinal - 1);
+ assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
+ // This table stores unbiased indices, so subtract OrdinalBase.
+ write16le(buf, e.ordinal - baseOrdinal);
buf += 2;
}
}
private:
+ size_t baseOrdinal;
size_t size;
const COFFLinkerContext &ctx;
};
@@ -831,12 +839,17 @@ Chunk *DelayLoadContents::newThunkChunk(DefinedImportData *s,
}
EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
- uint16_t maxOrdinal = 0;
- for (Export &e : ctx.config.exports)
- maxOrdinal = std::max(maxOrdinal, e.ordinal);
+ unsigned baseOrdinal = 1 << 16, maxOrdinal = 0;
+ for (Export &e : ctx.config.exports) {
+ baseOrdinal = std::min(baseOrdinal, (unsigned)e.ordinal);
+ maxOrdinal = std::max(maxOrdinal, (unsigned)e.ordinal);
+ }
+ // Ordinals must start at 1 as suggested in:
+ // https://learn.microsoft.com/en-us/cpp/build/reference/export-exports-a-function?view=msvc-170
+ assert(baseOrdinal >= 1);
auto *dllName = make<StringChunk>(sys::path::filename(ctx.config.outputFile));
- auto *addressTab = make<AddressTableChunk>(ctx, maxOrdinal);
+ auto *addressTab = make<AddressTableChunk>(ctx, baseOrdinal, maxOrdinal);
std::vector<Chunk *> names;
for (Export &e : ctx.config.exports)
if (!e.noname)
@@ -851,9 +864,10 @@ EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
}
auto *nameTab = make<NamePointersChunk>(names);
- auto *ordinalTab = make<ExportOrdinalChunk>(ctx, names.size());
- auto *dir = make<ExportDirectoryChunk>(maxOrdinal, names.size(), dllName,
- addressTab, nameTab, ordinalTab);
+ auto *ordinalTab = make<ExportOrdinalChunk>(ctx, baseOrdinal, names.size());
+ auto *dir =
+ make<ExportDirectoryChunk>(baseOrdinal, maxOrdinal, names.size(), dllName,
+ addressTab, nameTab, ordinalTab);
chunks.push_back(dir);
chunks.push_back(dllName);
chunks.push_back(addressTab);
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 403030a9695df..d4115a0e85eb2 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -390,7 +390,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
if (!exp.extName.empty() && !isDecorated(exp.extName))
exp.extName = saver().save("_" + exp.extName);
}
- exp.directives = true;
+ exp.source = ExportSource::Directives;
ctx.config.exports.push_back(exp);
}
@@ -1063,6 +1063,7 @@ void LinkerDriver::parseModuleDefs(StringRef path) {
e2.data = e1.Data;
e2.isPrivate = e1.Private;
e2.constant = e1.Constant;
+ e2.source = ExportSource::ModuleDefinition;
ctx.config.exports.push_back(e2);
}
}
@@ -2259,7 +2260,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (!e.forwardTo.empty())
continue;
e.sym = addUndefined(e.name);
- if (!e.directives)
+ if (e.source != ExportSource::Directives)
e.symbolName = mangleMaybe(e.sym);
}
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index 0f4af4def670c..6a72da28d49db 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -549,6 +549,8 @@ void LinkerDriver::createSideBySideManifest() {
// Used for parsing /export arguments.
Export LinkerDriver::parseExport(StringRef arg) {
Export e;
+ e.source = ExportSource::Export;
+
StringRef rest;
std::tie(e.name, rest) = arg.split(",");
if (e.name.empty())
@@ -641,6 +643,19 @@ static StringRef killAt(StringRef sym, bool prefix) {
return sym;
}
+static StringRef exportSourceName(ExportSource s) {
+ switch (s) {
+ case ExportSource::Directives:
+ return "source file (directives)";
+ case ExportSource::Export:
+ return "/export";
+ case ExportSource::ModuleDefinition:
+ return "/def";
+ default:
+ llvm_unreachable("unknown ExportSource");
+ }
+}
+
// Performs error checking on all /export arguments.
// It also sets ordinals.
void LinkerDriver::fixupExports() {
@@ -671,19 +686,36 @@ void LinkerDriver::fixupExports() {
}
// Uniquefy by name.
- DenseMap<StringRef, Export *> map(ctx.config.exports.size());
+ DenseMap<StringRef, std::pair<Export *, unsigned>> map(
+ ctx.config.exports.size());
std::vector<Export> v;
for (Export &e : ctx.config.exports) {
- auto pair = map.insert(std::make_pair(e.exportName, &e));
+ auto pair = map.insert(std::make_pair(e.exportName, std::make_pair(&e, 0)));
bool inserted = pair.second;
if (inserted) {
+ pair.first->second.second = v.size();
v.push_back(e);
continue;
}
- Export *existing = pair.first->second;
+ Export *existing = pair.first->second.first;
if (e == *existing || e.name != existing->name)
continue;
- warn("duplicate /export option: " + e.name);
+ // If the existing export comes from .OBJ directives, we are allowed to
+ // overwrite it with /DEF: or /EXPORT without any warning, as MSVC link.exe
+ // does.
+ if (existing->source == ExportSource::Directives) {
+ *existing = e;
+ v[pair.first->second.second] = e;
+ continue;
+ }
+ if (existing->source == e.source) {
+ warn(Twine("duplicate ") + exportSourceName(existing->source) +
+ " option: " + e.name);
+ } else {
+ warn("duplicate export: " + e.name +
+ Twine(" first seen in " + exportSourceName(existing->source) +
+ Twine(", now in " + exportSourceName(e.source))));
+ }
}
ctx.config.exports = std::move(v);
diff --git a/lld/test/COFF/export.test b/lld/test/COFF/export.test
index 755a552a57ff8..cbd17c95d87ce 100644
--- a/lld/test/COFF/export.test
+++ b/lld/test/COFF/export.test
@@ -15,7 +15,7 @@ CHECK1-NEXT: 2 0x1010 exportfn2
CHECK2: Export Table:
CHECK2-NEXT: DLL name: export.test.tmp.dll
-CHECK2-NEXT: Ordinal base: 1
+CHECK2-NEXT: Ordinal base: 5
CHECK2-NEXT: Ordinal RVA Name
CHECK2-NEXT: 5 0x1008 exportfn1
CHECK2-NEXT: 6 0x1010 exportfn2
@@ -26,7 +26,7 @@ CHECK2-NEXT: 7 0x1010 exportfn3
CHECK3: Export Table:
CHECK3-NEXT: DLL name: export.test.tmp.dll
-CHECK3-NEXT: Ordinal base: 1
+CHECK3-NEXT: Ordinal base: 5
CHECK3-NEXT: Ordinal RVA Name
CHECK3-NEXT: 5 0x1008
CHECK3-NEXT: 6 0x1010 exportfn2
@@ -52,7 +52,7 @@ CHECK4-NM: 00000000 T f2
CHECK5: Export Table:
CHECK5-NEXT: DLL name: export.test.tmp.dll
-CHECK5-NEXT: Ordinal base: 1
+CHECK5-NEXT: Ordinal base: 2
CHECK5-NEXT: Ordinal RVA Name
CHECK5-NEXT: 2 0x1010 fn2
CHECK5-NEXT: 3 0x1008 exportfn1
diff --git a/lld/test/COFF/export32.test b/lld/test/COFF/export32.test
index 8e27b0834024a..5bb5fd36ef2af 100644
--- a/lld/test/COFF/export32.test
+++ b/lld/test/COFF/export32.test
@@ -26,7 +26,7 @@
# CHECK2: Export Table:
# CHECK2-NEXT: DLL name: export32.test.tmp.dll
-# CHECK2-NEXT: Ordinal base: 1
+# CHECK2-NEXT: Ordinal base: 5
# CHECK2-NEXT: Ordinal RVA Name
# CHECK2-NEXT: 5 0x1008 exportfn1
# CHECK2-NEXT: 6 0x1010 exportfn2
@@ -38,7 +38,7 @@
# CHECK3: Export Table:
# CHECK3-NEXT: DLL name: export32.test.tmp.dll
-# CHECK3-NEXT: Ordinal base: 1
+# CHECK3-NEXT: Ordinal base: 5
# CHECK3-NEXT: Ordinal RVA Name
# CHECK3-NEXT: 5 0x1008
# CHECK3-NEXT: 6 0x1010 exportfn2
@@ -66,7 +66,7 @@
# CHECK5: Export Table:
# CHECK5-NEXT: DLL name: export32.test.tmp.dll
-# CHECK5-NEXT: Ordinal base: 1
+# CHECK5-NEXT: Ordinal base: 2
# CHECK5-NEXT: Ordinal RVA Name
# CHECK5-NEXT: 2 0x1010 fn2
# CHECK5-NEXT: 3 0x1008 exportfn1
diff --git a/lld/test/COFF/ordinals-override.test b/lld/test/COFF/ordinals-override.test
new file mode 100644
index 0000000000000..026e3c7195ab5
--- /dev/null
+++ b/lld/test/COFF/ordinals-override.test
@@ -0,0 +1,115 @@
+# RUN: yaml2obj %s -o %t.obj
+#
+# RUN: lld-link /out:%t.dll /dll %t.obj
+# RUN: llvm-objdump -p %t.dll | FileCheck --check-prefix=CHECK1 %s
+#
+# CHECK1: Export Table:
+# CHECK1: DLL name: ordinals-override.test.tmp.dll
+# CHECK1: Ordinal base: 1
+# CHECK1: Ordinal RVA Name
+# CHECK1-NEXT: 1 0x1010 ?bar@@YAXXZ
+# CHECK1-NEXT: 2 0x1000 ?foo@@YAXXZ
+# CHECK1-NEXT: 3 0x1020 baz
+#
+# RUN: lld-link /out:%t.dll /dll %t.obj /EXPORT:?foo@@YAXXZ, at 55
+# RUN: llvm-objdump -p %t.dll | FileCheck --check-prefix=CHECK2 %s
+#
+# CHECK2: Export Table:
+# CHECK2: DLL name: ordinals-override.test.tmp.dll
+# CHECK2: Ordinal base: 55
+# CHECK2: Ordinal RVA Name
+# CHECK2-NEXT: 55 0x1000 ?foo@@YAXXZ
+# CHECK2-NEXT: 56 0x1010 ?bar@@YAXXZ
+# CHECK2-NEXT: 57 0x1020 baz
+#
+# RUN: lld-link /out:%t.dll /dll %t.obj /EXPORT:?foo@@YAXXZ, at 55 /EXPORT:?bar@@YAXXZ, at 122
+# RUN: llvm-objdump -p %t.dll | FileCheck --check-prefix=CHECK3 %s
+#
+# CHECK3: Export Table:
+# CHECK3: DLL name: ordinals-override.test.tmp.dll
+# CHECK3: Ordinal base: 55
+# CHECK3: Ordinal RVA Name
+# CHECK3-NEXT: 55 0x1000 ?foo@@YAXXZ
+# CHECK3-NEXT: 122 0x1010 ?bar@@YAXXZ
+# CHECK3-NEXT: 123 0x1020 baz
+#
+# RUN: echo "EXPORTS" > %t.def
+# RUN: echo "?foo@@YAXXZ @55" >> %t.def
+# RUN: echo "?bar@@YAXXZ @122" >> %t.def
+# RUN: lld-link /out:%t.dll /dll %t.obj /DEF:%t.def 2>&1 | FileCheck --check-prefix=WARN --allow-empty %s
+# WARN-NOT: lld-link: warning
+#
+# RUN: llvm-objdump -p %t.dll | FileCheck --check-prefix=CHECK3 %s
+#
+# RUN: lld-link /out:%t.dll /dll %t.obj /DEF:%t.def /EXPORT:?foo@@YAXXZ, at 10000 2>&1 | FileCheck --check-prefix=DUPLICATED %s
+# DUPLICATED: lld-link: warning: duplicate export: ?foo@@YAXXZ first seen in /export, now in /def
+#
+# RUN: llvm-objdump -p %t.dll | FileCheck --check-prefix=CHECK4 %s
+#
+# CHECK4: Export Table:
+# CHECK4: DLL name: ordinals-override.test.tmp.dll
+# CHECK4: Ordinal base: 122
+# CHECK4: Ordinal RVA Name
+# CHECK4-NEXT: 122 0x1010 ?bar@@YAXXZ
+# CHECK4-NEXT: 10000 0x1000 ?foo@@YAXXZ
+# CHECK4-NEXT: 10001 0x1020 baz
+
+# The .drectve section below contains the following:
+#
+# Linker Directives
+# -----------------
+# /export:baz=?baz@@YAXXZ
+# /EXPORT:?foo@@YAXXZ
+# /EXPORT:?bar@@YAXXZ
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .drectve
+ Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+ Alignment: 1
+ SectionData: 2f6578706f72743a62617a3d3f62617a4040594158585a202f4558504f52543a3f666f6f4040594158585a202f4558504f52543a3f6261724040594158585a
+ - Name: '.text$mn'
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+ Alignment: 16
+ SectionData: C20000CCCCCCCCCCCCCCCCCCCCCCCCCCC20000CCCCCCCCCCCCCCCCCCCCCCCCCCC20000
+symbols:
+ - Name: '.text$mn'
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 35
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 0
+ - Name: _DllMainCRTStartup
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: '?foo@@YAXXZ'
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: '?bar@@YAXXZ'
+ Value: 16
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: '?baz@@YAXXZ'
+ Value: 32
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+...
More information about the llvm-commits
mailing list