[lld] [lld][COFF] Fix: Merge `.drectve` sections in ObjFile::readSections (PR #86380)
Rickey Bowers Jr. via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 23 00:15:49 PDT 2024
https://github.com/bitRAKE updated https://github.com/llvm/llvm-project/pull/86380
>From bfaa3553a649d097ac48a46284c102bdf72a2ba2 Mon Sep 17 00:00:00 2001
From: "Rickey Bowers Jr." <bitRAKE at gmail.com>
Date: Fri, 22 Mar 2024 23:31:10 -0600
Subject: [PATCH 1/3] This commit addresses an issue where lld-link was not
merging the contents of multiple `.drectve` sections from OBJ files, unlike
Microsoft's linker which merges them. Previously, lld-link would overwrite
`.drectve` section content, only retaining the last section's directives.
This behavior led to incomplete directive processing.
---
lld/COFF/Driver.cpp | 197 ++++++++++----------
lld/COFF/InputFiles.cpp | 6 +-
lld/COFF/InputFiles.h | 6 +-
lld/test/COFF/Inputs/directive-multiple.obj | Bin 0 -> 496 bytes
lld/test/COFF/directives-multiple.test | 16 ++
5 files changed, 124 insertions(+), 101 deletions(-)
create mode 100644 lld/test/COFF/Inputs/directive-multiple.obj
create mode 100644 lld/test/COFF/directives-multiple.test
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 22ee2f133be98a..2336acd2eb3187 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -368,111 +368,116 @@ bool LinkerDriver::isDecorated(StringRef sym) {
// Parses .drectve section contents and returns a list of files
// specified by /defaultlib.
void LinkerDriver::parseDirectives(InputFile *file) {
- StringRef s = file->getDirectives();
- if (s.empty())
+ std::vector<StringRef> directivesList = file->getDirectives();
+ if (directivesList.empty())
return;
- log("Directives: " + toString(file) + ": " + s);
-
- ArgParser parser(ctx);
- // .drectve is always tokenized using Windows shell rules.
- // /EXPORT: option can appear too many times, processing in fastpath.
- ParsedDirectives directives = parser.parseDirectives(s);
-
- for (StringRef e : directives.exports) {
- // If a common header file contains dllexported function
- // declarations, many object files may end up with having the
- // same /EXPORT options. In order to save cost of parsing them,
- // we dedup them first.
- if (!directivesExports.insert(e).second)
+ for (StringRef s : directivesList) {
+ if (s.empty())
continue;
- Export exp = parseExport(e);
- if (ctx.config.machine == I386 && ctx.config.mingw) {
- if (!isDecorated(exp.name))
- exp.name = saver().save("_" + exp.name);
- if (!exp.extName.empty() && !isDecorated(exp.extName))
- exp.extName = saver().save("_" + exp.extName);
- }
- exp.source = ExportSource::Directives;
- ctx.config.exports.push_back(exp);
- }
+ log("Directives: " + toString(file) + ": " + s);
- // Handle /include: in bulk.
- for (StringRef inc : directives.includes)
- addUndefined(inc);
+ ArgParser parser(ctx);
+ // .drectve is always tokenized using Windows shell rules.
+ // /EXPORT: option can appear too many times, processing in fastpath.
+ ParsedDirectives directives = parser.parseDirectives(s);
- // Handle /exclude-symbols: in bulk.
- for (StringRef e : directives.excludes) {
- SmallVector<StringRef, 2> vec;
- e.split(vec, ',');
- for (StringRef sym : vec)
- excludedSymbols.insert(mangle(sym));
- }
+ for (StringRef e : directives.exports) {
+ // If a common header file contains dllexported function
+ // declarations, many object files may end up with having the
+ // same /EXPORT options. In order to save cost of parsing them,
+ // we dedup them first.
+ if (!directivesExports.insert(e).second)
+ continue;
- // https://docs.microsoft.com/en-us/cpp/preprocessor/comment-c-cpp?view=msvc-160
- for (auto *arg : directives.args) {
- switch (arg->getOption().getID()) {
- case OPT_aligncomm:
- parseAligncomm(arg->getValue());
- break;
- case OPT_alternatename:
- parseAlternateName(arg->getValue());
- break;
- case OPT_defaultlib:
- if (std::optional<StringRef> path = findLibIfNew(arg->getValue()))
- enqueuePath(*path, false, false);
- break;
- case OPT_entry:
- ctx.config.entry = addUndefined(mangle(arg->getValue()));
- break;
- case OPT_failifmismatch:
- checkFailIfMismatch(arg->getValue(), file);
- break;
- case OPT_incl:
- addUndefined(arg->getValue());
- break;
- case OPT_manifestdependency:
- ctx.config.manifestDependencies.insert(arg->getValue());
- break;
- case OPT_merge:
- parseMerge(arg->getValue());
- break;
- case OPT_nodefaultlib:
- ctx.config.noDefaultLibs.insert(findLib(arg->getValue()).lower());
- break;
- case OPT_release:
- ctx.config.writeCheckSum = true;
- break;
- case OPT_section:
- parseSection(arg->getValue());
- break;
- case OPT_stack:
- parseNumbers(arg->getValue(), &ctx.config.stackReserve,
- &ctx.config.stackCommit);
- break;
- case OPT_subsystem: {
- bool gotVersion = false;
- parseSubsystem(arg->getValue(), &ctx.config.subsystem,
- &ctx.config.majorSubsystemVersion,
- &ctx.config.minorSubsystemVersion, &gotVersion);
- if (gotVersion) {
- ctx.config.majorOSVersion = ctx.config.majorSubsystemVersion;
- ctx.config.minorOSVersion = ctx.config.minorSubsystemVersion;
+ Export exp = parseExport(e);
+ if (ctx.config.machine == I386 && ctx.config.mingw) {
+ if (!isDecorated(exp.name))
+ exp.name = saver().save("_" + exp.name);
+ if (!exp.extName.empty() && !isDecorated(exp.extName))
+ exp.extName = saver().save("_" + exp.extName);
}
- break;
+ exp.source = ExportSource::Directives;
+ ctx.config.exports.push_back(exp);
}
- // Only add flags here that link.exe accepts in
- // `#pragma comment(linker, "/flag")`-generated sections.
- case OPT_editandcontinue:
- case OPT_guardsym:
- case OPT_throwingnew:
- case OPT_inferasanlibs:
- case OPT_inferasanlibs_no:
- break;
- default:
- error(arg->getSpelling() + " is not allowed in .drectve (" +
- toString(file) + ")");
+
+ // Handle /include: in bulk.
+ for (StringRef inc : directives.includes)
+ addUndefined(inc);
+
+ // Handle /exclude-symbols: in bulk.
+ for (StringRef e : directives.excludes) {
+ SmallVector<StringRef, 2> vec;
+ e.split(vec, ',');
+ for (StringRef sym : vec)
+ excludedSymbols.insert(mangle(sym));
+ }
+
+ // https://docs.microsoft.com/en-us/cpp/preprocessor/comment-c-cpp?view=msvc-160
+ for (auto *arg : directives.args) {
+ switch (arg->getOption().getID()) {
+ case OPT_aligncomm:
+ parseAligncomm(arg->getValue());
+ break;
+ case OPT_alternatename:
+ parseAlternateName(arg->getValue());
+ break;
+ case OPT_defaultlib:
+ if (std::optional<StringRef> path = findLibIfNew(arg->getValue()))
+ enqueuePath(*path, false, false);
+ break;
+ case OPT_entry:
+ ctx.config.entry = addUndefined(mangle(arg->getValue()));
+ break;
+ case OPT_failifmismatch:
+ checkFailIfMismatch(arg->getValue(), file);
+ break;
+ case OPT_incl:
+ addUndefined(arg->getValue());
+ break;
+ case OPT_manifestdependency:
+ ctx.config.manifestDependencies.insert(arg->getValue());
+ break;
+ case OPT_merge:
+ parseMerge(arg->getValue());
+ break;
+ case OPT_nodefaultlib:
+ ctx.config.noDefaultLibs.insert(findLib(arg->getValue()).lower());
+ break;
+ case OPT_release:
+ ctx.config.writeCheckSum = true;
+ break;
+ case OPT_section:
+ parseSection(arg->getValue());
+ break;
+ case OPT_stack:
+ parseNumbers(arg->getValue(), &ctx.config.stackReserve,
+ &ctx.config.stackCommit);
+ break;
+ case OPT_subsystem: {
+ bool gotVersion = false;
+ parseSubsystem(arg->getValue(), &ctx.config.subsystem,
+ &ctx.config.majorSubsystemVersion,
+ &ctx.config.minorSubsystemVersion, &gotVersion);
+ if (gotVersion) {
+ ctx.config.majorOSVersion = ctx.config.majorSubsystemVersion;
+ ctx.config.minorOSVersion = ctx.config.minorSubsystemVersion;
+ }
+ break;
+ }
+ // Only add flags here that link.exe accepts in
+ // `#pragma comment(linker, "/flag")`-generated sections.
+ case OPT_editandcontinue:
+ case OPT_guardsym:
+ case OPT_throwingnew:
+ case OPT_inferasanlibs:
+ case OPT_inferasanlibs_no:
+ break;
+ default:
+ error(arg->getSpelling() + " is not allowed in .drectve (" +
+ toString(file) + ")");
+ }
}
}
}
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 037fae45242c6f..ab63de2a5c76a5 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -212,7 +212,8 @@ SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
if (name == ".drectve") {
ArrayRef<uint8_t> data;
cantFail(coffObj->getSectionContents(sec, data));
- directives = StringRef((const char *)data.data(), data.size());
+ // MS link accumulates the directive sections in order of appearance
+ directives.push_back(StringRef(reinterpret_cast<const char*>(data.data()), data.size()));
return nullptr;
}
@@ -1086,7 +1087,8 @@ void BitcodeFile::parse() {
if (objSym.isUsed())
ctx.config.gcroot.push_back(sym);
}
- directives = saver.save(obj->getCOFFLinkerOpts());
+// directives.push_back(saver.save(obj->getCOFFLinkerOpts()).str());
+ directives.push_back(obj->getCOFFLinkerOpts());
}
void BitcodeFile::parseLazy() {
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 3b55cd791bfda2..22bd7734bf96c7 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -89,8 +89,8 @@ class InputFile {
// An archive file name if this file is created from an archive.
StringRef parentName;
- // Returns .drectve section contents if exist.
- StringRef getDirectives() { return directives; }
+ // Returns .drectve section(s) content if exist.
+ std::vector<StringRef> getDirectives() { return directives; }
COFFLinkerContext &ctx;
@@ -98,7 +98,7 @@ class InputFile {
InputFile(COFFLinkerContext &c, Kind k, MemoryBufferRef m, bool lazy = false)
: mb(m), ctx(c), fileKind(k), lazy(lazy) {}
- StringRef directives;
+ std::vector<StringRef> directives;
private:
const Kind fileKind;
diff --git a/lld/test/COFF/Inputs/directive-multiple.obj b/lld/test/COFF/Inputs/directive-multiple.obj
new file mode 100644
index 0000000000000000000000000000000000000000..a2c0085a1656628c60e912ca67a138a763a149b8
GIT binary patch
literal 496
zcmYdkV`a$u_9|6_k%57O0R&nY^-5AJO2BLg!3L!6An`#=1%`kGbagC1 at kiLyr4*$m
zmz1T#q@;k1XD|+o<}zTwW*!63UC01zFPeEOKm{yl>KK6L?Er}#?qv}?tnV5T;2#uX
z<(QIUYY!wG93mY(JUkqt6een3fXfD#CY2N=CYRXS!&nG)$Zi7}%M5lD$j2Z65 at AAd
zKNC;{qDCB~00=-LKs88i2gx&|=mMDs(WMGxfdEJZRTnEzgqgttNXEP5<oG6L<~aw2
T1eYWhm6R4Rpm>e}(;o}~Ozuo=
literal 0
HcmV?d00001
diff --git a/lld/test/COFF/directives-multiple.test b/lld/test/COFF/directives-multiple.test
new file mode 100644
index 00000000000000..416b0b99882d14
--- /dev/null
+++ b/lld/test/COFF/directives-multiple.test
@@ -0,0 +1,16 @@
+# RUN: lld-link /dll %p/Inputs/directive-multiple.obj /out:%t.dll
+# RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix=EXPORTS %s
+
+EXPORTS: Format: COFF-x86-64
+EXPORTS: Arch: x86_64
+EXPORTS: AddressSize: 64bit
+EXPORTS: Export {
+EXPORTS: Ordinal: 1
+EXPORTS: Name: Add
+EXPORTS: RVA: 0x1010
+EXPORTS: }
+EXPORTS: Export {
+EXPORTS: Ordinal: 2
+EXPORTS: Name: Subtract
+EXPORTS: RVA: 0x1020
+EXPORTS: }
>From 436beb016c4f9f11c994c4f299925617b4030d38 Mon Sep 17 00:00:00 2001
From: Rickey Bowers Jr <bitRAKE at gmail.com>
Date: Sat, 23 Mar 2024 00:01:57 -0600
Subject: [PATCH 2/3] bogus comment
foobar
---
lld/COFF/InputFiles.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index ab63de2a5c76a5..791cece1e1b5d6 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -1087,7 +1087,6 @@ void BitcodeFile::parse() {
if (objSym.isUsed())
ctx.config.gcroot.push_back(sym);
}
-// directives.push_back(saver.save(obj->getCOFFLinkerOpts()).str());
directives.push_back(obj->getCOFFLinkerOpts());
}
>From 9cbe0cc61bbeb039b42b78d65d628a95718cef9c Mon Sep 17 00:00:00 2001
From: Rickey Bowers Jr <bitRAKE at gmail.com>
Date: Sat, 23 Mar 2024 01:15:41 -0600
Subject: [PATCH 3/3] deleted the wrong line, doh!
---
lld/COFF/InputFiles.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 791cece1e1b5d6..d8fa734fd62d5f 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -1087,7 +1087,7 @@ void BitcodeFile::parse() {
if (objSym.isUsed())
ctx.config.gcroot.push_back(sym);
}
- directives.push_back(obj->getCOFFLinkerOpts());
+ directives.push_back(saver.save(obj->getCOFFLinkerOpts()));
}
void BitcodeFile::parseLazy() {
More information about the llvm-commits
mailing list