[lld] [lld][COFF] Fix: Merge `.drectve` sections in ObjFile::readSections (PR #86380)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 22 22:39:52 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lld-coff
Author: Rickey Bowers Jr. (bitRAKE)
<details>
<summary>Changes</summary>
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.
This fix aligns lld-link's behavior with Microsoft's linker, improving compatibility and ensuring that all directives from object files are correctly processed.
---
Full diff: https://github.com/llvm/llvm-project/pull/86380.diff
5 Files Affected:
- (modified) lld/COFF/Driver.cpp (+101-96)
- (modified) lld/COFF/InputFiles.cpp (+4-2)
- (modified) lld/COFF/InputFiles.h (+3-3)
- (added) lld/test/COFF/Inputs/directive-multiple.obj ()
- (added) lld/test/COFF/directives-multiple.test (+16)
``````````diff
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 00000000000000..a2c0085a165662
Binary files /dev/null and b/lld/test/COFF/Inputs/directive-multiple.obj differ
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: }
``````````
</details>
https://github.com/llvm/llvm-project/pull/86380
More information about the llvm-commits
mailing list