[lld][COFF] Fix: Merge `.drectve` sections in ObjFile::readSections

[bitRAKE]

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.

[github-actions[bot]]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-coff

Author: Rickey Bowers Jr. (bitRAKE)

Changes

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 --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: }

[github-actions[bot]]

:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

You can test this locally with the following command:

git-clang-format --diff 65058a8d732c3c41664a4dad1a1ae2a504d5c98e bfaa3553a649d097ac48a46284c102bdf72a2ba2 -- lld/COFF/Driver.cpp lld/COFF/InputFiles.cpp lld/COFF/InputFiles.h

View the diff from clang-format here.

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 2336acd2eb..f26e355119 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -453,13 +453,13 @@ void LinkerDriver::parseDirectives(InputFile *file) {
         break;
       case OPT_stack:
         parseNumbers(arg->getValue(), &ctx.config.stackReserve,
-                    &ctx.config.stackCommit);
+                     &ctx.config.stackCommit);
         break;
       case OPT_subsystem: {
         bool gotVersion = false;
         parseSubsystem(arg->getValue(), &ctx.config.subsystem,
-                      &ctx.config.majorSubsystemVersion,
-                      &ctx.config.minorSubsystemVersion, &gotVersion);
+                       &ctx.config.majorSubsystemVersion,
+                       &ctx.config.minorSubsystemVersion, &gotVersion);
         if (gotVersion) {
           ctx.config.majorOSVersion = ctx.config.majorSubsystemVersion;
           ctx.config.minorOSVersion = ctx.config.minorSubsystemVersion;
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index ab63de2a5c..27112eede6 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -213,7 +213,8 @@ SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
     ArrayRef<uint8_t> data;
     cantFail(coffObj->getSectionContents(sec, data));
     // MS link accumulates the directive sections in order of appearance
-    directives.push_back(StringRef(reinterpret_cast<const char*>(data.data()), data.size()));
+    directives.push_back(
+        StringRef(reinterpret_cast<const char *>(data.data()), data.size()));
     return nullptr;
   }
 
@@ -1087,7 +1088,7 @@ void BitcodeFile::parse() {
     if (objSym.isUsed())
       ctx.config.gcroot.push_back(sym);
   }
-//  directives.push_back(saver.save(obj->getCOFFLinkerOpts()).str());
+  //  directives.push_back(saver.save(obj->getCOFFLinkerOpts()).str());
   directives.push_back(obj->getCOFFLinkerOpts());
 }
 

[github-actions[bot]]

:white_check_mark: With the latest revision this PR passed the Python code formatter.

[bitRAKE]

Weird, I run that test here and I get the correct output.

File: lto-directives.dll
Format: COFF-x86-64
Arch: x86_64
AddressSize: 64bit
Export {
  Ordinal: 1
  Name: entry
  RVA: 0x1000
}