[lld] [lld][COFF] Fix: Merge `.drectve` sections in ObjFile::readSections (PR #86380)

Rickey Bowers Jr. via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 25 13:45:34 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/7] 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/7] 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/7] 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() {

>From 867cb66057d746a8a37b7f223b6fc691638d9391 Mon Sep 17 00:00:00 2001
From: "Rickey Bowers Jr." <bitRAKE at gmail.com>
Date: Sun, 24 Mar 2024 11:29:30 -0600
Subject: [PATCH 4/7] Split iterator from processing. Revised naming to
 differentiate these functions from ArgParser::parseDirectives. File section
 processing more distinct from parsing of text. Include assembly source for
 test object file.

---
 lld/COFF/Driver.cpp                           | 202 +++++++++---------
 lld/COFF/Driver.h                             |   7 +-
 lld/COFF/InputFiles.cpp                       |   4 +-
 lld/COFF/InputFiles.h                         |   4 +-
 lld/COFF/SymbolTable.cpp                      |   2 +-
 lld/test/COFF/Inputs/directives-multiple.asm  |  43 ++++
 ...e-multiple.obj => directives-multiple.obj} | Bin
 lld/test/COFF/directives-multiple.test        |   2 +-
 8 files changed, 156 insertions(+), 108 deletions(-)
 create mode 100644 lld/test/COFF/Inputs/directives-multiple.asm
 rename lld/test/COFF/Inputs/{directive-multiple.obj => directives-multiple.obj} (100%)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 2336acd2eb3187..b48709b0e2715c 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -365,119 +365,121 @@ bool LinkerDriver::isDecorated(StringRef sym) {
          (!ctx.config.mingw && sym.contains('@'));
 }
 
-// Parses .drectve section contents and returns a list of files
-// specified by /defaultlib.
-void LinkerDriver::parseDirectives(InputFile *file) {
-  std::vector<StringRef> directivesList = file->getDirectives();
+void LinkerDriver::processDrectveSections(InputFile *file) {
+  std::vector<StringRef> directivesList = file->getDrectves();
   if (directivesList.empty())
     return;
-
   for (StringRef s : directivesList) {
     if (s.empty())
       continue;
+    parseDrectveSectionContents(file, s);
+  }
+}
 
-    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);
+// Parses .drectve section contents and returns a list of files
+// specified by /defaultlib.
+void LinkerDriver::parseDrectveSectionContents(InputFile *file, StringRef s) {
+  log("Directives: " + toString(file) + ": " + 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)
-        continue;
+  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)
+      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);
+    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);
+  }
 
-    // Handle /include: in bulk.
-    for (StringRef inc : directives.includes)
-      addUndefined(inc);
+  // 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));
-    }
+  // 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) + ")");
+  // 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/Driver.h b/lld/COFF/Driver.h
index fa54de05befb58..6ffa0c03bd6f12 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -86,8 +86,8 @@ class LinkerDriver {
 
   void addClangLibSearchPaths(const std::string &argv0);
 
-  // Used by the resolver to parse .drectve section contents.
-  void parseDirectives(InputFile *file);
+  // Used by the resolver to iterate through .drectve section(s).
+  void processDrectveSections(InputFile *file);
 
   // Used by ArchiveFile to enqueue members.
   void enqueueArchiveMember(const Archive::Child &c, const Archive::Symbol &sym,
@@ -111,6 +111,9 @@ class LinkerDriver {
 
   bool findUnderscoreMangle(StringRef sym);
 
+  // Used by the processDrectveSections() to parse .drectve section contents.
+  void parseDrectveSectionContents(InputFile *file, StringRef s);
+
   // Determines the location of the sysroot based on `args`, environment, etc.
   void detectWinSysRoot(const llvm::opt::InputArgList &args);
 
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index d8fa734fd62d5f..b09823a3f7c599 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -213,7 +213,7 @@ 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()));
+    drectves.push_back(StringRef(reinterpret_cast<const char*>(data.data()), data.size()));
     return nullptr;
   }
 
@@ -1087,7 +1087,7 @@ void BitcodeFile::parse() {
     if (objSym.isUsed())
       ctx.config.gcroot.push_back(sym);
   }
-  directives.push_back(saver.save(obj->getCOFFLinkerOpts()));
+  drectves.push_back(saver.save(obj->getCOFFLinkerOpts()));
 }
 
 void BitcodeFile::parseLazy() {
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 22bd7734bf96c7..50bbdc26f8742b 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -90,7 +90,7 @@ class InputFile {
   StringRef parentName;
 
   // Returns .drectve section(s) content if exist.
-  std::vector<StringRef> getDirectives() { return directives; }
+  std::vector<StringRef> getDrectves() { return drectves; }
 
   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) {}
 
-  std::vector<StringRef> directives;
+  std::vector<StringRef> drectves;
 
 private:
   const Kind fileKind;
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 44aa506d2c35da..a343802d204b4c 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -81,7 +81,7 @@ void SymbolTable::addFile(InputFile *file) {
     return;
   }
 
-  ctx.driver.parseDirectives(file);
+  ctx.driver.processDrectveSections(file);
 }
 
 static void errorOrWarn(const Twine &s, bool forceUnresolved) {
diff --git a/lld/test/COFF/Inputs/directives-multiple.asm b/lld/test/COFF/Inputs/directives-multiple.asm
new file mode 100644
index 00000000000000..51a096fa46bce7
--- /dev/null
+++ b/lld/test/COFF/Inputs/directives-multiple.asm
@@ -0,0 +1,43 @@
+; fasm2 directives-multiple.asm
+; lld-link /dll directives-multiple.obj
+
+format MS64 COFF
+
+; NOTE: Each function has its own .drectve section
+
+section '.text' code readable executable align 16
+
+; BOOL WINAPI
+; DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpReserved)
+DllMain:
+	mov eax, 1 ; TRUE
+	retn
+
+public DllMain as '_DllMainCRTStartup' ; linker expects this default name
+
+
+
+section '.text' code readable executable align 16
+align 16
+_Add:	; __declspec(dllexport) int Add(int a, int b) {
+	lea eax, [rcx + rdx]
+	retn
+
+public _Add as '?Add@@YAHHH at Z' ; decorated name for linker
+
+section '.drectve' linkinfo linkremove
+db "/EXPORT:Add=?Add@@YAHHH at Z "
+
+
+
+section '.text' code readable executable align 16
+align 16
+_Sub:	; __declspec(dllexport) int Subtract(int a, int b) {
+	xchg eax, ecx
+	sub eax, edx
+	retn
+
+public _Sub as '?Subtract@@YAHHH at Z' ; decorated name for linker
+
+section '.drectve' linkinfo linkremove
+db "/EXPORT:Subtract=?Subtract@@YAHHH at Z "
diff --git a/lld/test/COFF/Inputs/directive-multiple.obj b/lld/test/COFF/Inputs/directives-multiple.obj
similarity index 100%
rename from lld/test/COFF/Inputs/directive-multiple.obj
rename to lld/test/COFF/Inputs/directives-multiple.obj
diff --git a/lld/test/COFF/directives-multiple.test b/lld/test/COFF/directives-multiple.test
index 416b0b99882d14..593c13fce59ec5 100644
--- a/lld/test/COFF/directives-multiple.test
+++ b/lld/test/COFF/directives-multiple.test
@@ -1,4 +1,4 @@
-# RUN: lld-link /dll %p/Inputs/directive-multiple.obj /out:%t.dll
+# RUN: lld-link /dll %p/Inputs/directives-multiple.obj /out:%t.dll
 # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix=EXPORTS %s
 
 EXPORTS: Format: COFF-x86-64

>From 6880ff8ff6601c8b1865c40974e88bd353e9cdbd Mon Sep 17 00:00:00 2001
From: "Rickey Bowers Jr." <bitRAKE at gmail.com>
Date: Sun, 24 Mar 2024 22:51:04 -0600
Subject: [PATCH 5/7] bringing style in line with review, minor edit

---
 lld/COFF/Driver.cpp                          | 11 ++++-------
 lld/COFF/Driver.h                            |  6 +++---
 lld/COFF/InputFiles.cpp                      |  4 ++--
 lld/COFF/InputFiles.h                        |  4 ++--
 lld/COFF/SymbolTable.cpp                     |  2 +-
 lld/test/COFF/Inputs/directives-multiple.asm |  4 ++++
 6 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index b48709b0e2715c..df747a12bbc15c 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -365,20 +365,17 @@ bool LinkerDriver::isDecorated(StringRef sym) {
          (!ctx.config.mingw && sym.contains('@'));
 }
 
-void LinkerDriver::processDrectveSections(InputFile *file) {
-  std::vector<StringRef> directivesList = file->getDrectves();
-  if (directivesList.empty())
-    return;
-  for (StringRef s : directivesList) {
+void LinkerDriver::processDirectivesSection(InputFile *file) {
+  for (StringRef s : file->getDrectves()) {
     if (s.empty())
       continue;
-    parseDrectveSectionContents(file, s);
+    processDirectivesSection(file, s);
   }
 }
 
 // Parses .drectve section contents and returns a list of files
 // specified by /defaultlib.
-void LinkerDriver::parseDrectveSectionContents(InputFile *file, StringRef s) {
+void LinkerDriver::processDirectivesSection(InputFile *file, StringRef s) {
   log("Directives: " + toString(file) + ": " + s);
 
   ArgParser parser(ctx);
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 6ffa0c03bd6f12..85696017c4c3af 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -87,7 +87,7 @@ class LinkerDriver {
   void addClangLibSearchPaths(const std::string &argv0);
 
   // Used by the resolver to iterate through .drectve section(s).
-  void processDrectveSections(InputFile *file);
+  void processDirectivesSection(InputFile *file);
 
   // Used by ArchiveFile to enqueue members.
   void enqueueArchiveMember(const Archive::Child &c, const Archive::Symbol &sym,
@@ -111,8 +111,8 @@ class LinkerDriver {
 
   bool findUnderscoreMangle(StringRef sym);
 
-  // Used by the processDrectveSections() to parse .drectve section contents.
-  void parseDrectveSectionContents(InputFile *file, StringRef s);
+  // Used by the processDirectivesSection() to parse .drectve section contents.
+  void processDirectivesSection(InputFile *file, StringRef s);
 
   // Determines the location of the sysroot based on `args`, environment, etc.
   void detectWinSysRoot(const llvm::opt::InputArgList &args);
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index b09823a3f7c599..d8fa734fd62d5f 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -213,7 +213,7 @@ 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
-    drectves.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 +1087,7 @@ void BitcodeFile::parse() {
     if (objSym.isUsed())
       ctx.config.gcroot.push_back(sym);
   }
-  drectves.push_back(saver.save(obj->getCOFFLinkerOpts()));
+  directives.push_back(saver.save(obj->getCOFFLinkerOpts()));
 }
 
 void BitcodeFile::parseLazy() {
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 50bbdc26f8742b..055165a2e9ed68 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -90,7 +90,7 @@ class InputFile {
   StringRef parentName;
 
   // Returns .drectve section(s) content if exist.
-  std::vector<StringRef> getDrectves() { return drectves; }
+  std::vector<StringRef> getDrectves() { 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) {}
 
-  std::vector<StringRef> drectves;
+  std::vector<StringRef> directives;
 
 private:
   const Kind fileKind;
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index a343802d204b4c..6b5aaac66079cd 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -81,7 +81,7 @@ void SymbolTable::addFile(InputFile *file) {
     return;
   }
 
-  ctx.driver.processDrectveSections(file);
+  ctx.driver.processDirectivesSection(file);
 }
 
 static void errorOrWarn(const Twine &s, bool forceUnresolved) {
diff --git a/lld/test/COFF/Inputs/directives-multiple.asm b/lld/test/COFF/Inputs/directives-multiple.asm
index 51a096fa46bce7..08b1969547660b 100644
--- a/lld/test/COFF/Inputs/directives-multiple.asm
+++ b/lld/test/COFF/Inputs/directives-multiple.asm
@@ -1,3 +1,7 @@
+; Presently, the LLVM tooling incorrectly coalesces '.drectve' sections. So,
+; there is no way to generate an OBJ with multiple sections. This (and the
+; resulting OBJ) should be removed in the future when resolved.
+
 ; fasm2 directives-multiple.asm
 ; lld-link /dll directives-multiple.obj
 

>From 80eae2bbf8fc592a6de7560b62421077aff4b120 Mon Sep 17 00:00:00 2001
From: "Rickey Bowers Jr." <bitRAKE at gmail.com>
Date: Mon, 25 Mar 2024 13:41:20 -0600
Subject: [PATCH 6/7] Use `yaml2obj` to refine test case.

---
 lld/test/COFF/Inputs/directives-multiple.asm |  47 --------
 lld/test/COFF/Inputs/directives-multiple.obj | Bin 496 -> 0 bytes
 lld/test/COFF/directives-multiple.test       | 110 ++++++++++++++++---
 3 files changed, 96 insertions(+), 61 deletions(-)
 delete mode 100644 lld/test/COFF/Inputs/directives-multiple.asm
 delete mode 100644 lld/test/COFF/Inputs/directives-multiple.obj

diff --git a/lld/test/COFF/Inputs/directives-multiple.asm b/lld/test/COFF/Inputs/directives-multiple.asm
deleted file mode 100644
index 08b1969547660b..00000000000000
--- a/lld/test/COFF/Inputs/directives-multiple.asm
+++ /dev/null
@@ -1,47 +0,0 @@
-; Presently, the LLVM tooling incorrectly coalesces '.drectve' sections. So,
-; there is no way to generate an OBJ with multiple sections. This (and the
-; resulting OBJ) should be removed in the future when resolved.
-
-; fasm2 directives-multiple.asm
-; lld-link /dll directives-multiple.obj
-
-format MS64 COFF
-
-; NOTE: Each function has its own .drectve section
-
-section '.text' code readable executable align 16
-
-; BOOL WINAPI
-; DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpReserved)
-DllMain:
-	mov eax, 1 ; TRUE
-	retn
-
-public DllMain as '_DllMainCRTStartup' ; linker expects this default name
-
-
-
-section '.text' code readable executable align 16
-align 16
-_Add:	; __declspec(dllexport) int Add(int a, int b) {
-	lea eax, [rcx + rdx]
-	retn
-
-public _Add as '?Add@@YAHHH at Z' ; decorated name for linker
-
-section '.drectve' linkinfo linkremove
-db "/EXPORT:Add=?Add@@YAHHH at Z "
-
-
-
-section '.text' code readable executable align 16
-align 16
-_Sub:	; __declspec(dllexport) int Subtract(int a, int b) {
-	xchg eax, ecx
-	sub eax, edx
-	retn
-
-public _Sub as '?Subtract@@YAHHH at Z' ; decorated name for linker
-
-section '.drectve' linkinfo linkremove
-db "/EXPORT:Subtract=?Subtract@@YAHHH at Z "
diff --git a/lld/test/COFF/Inputs/directives-multiple.obj b/lld/test/COFF/Inputs/directives-multiple.obj
deleted file mode 100644
index a2c0085a1656628c60e912ca67a138a763a149b8..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

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=

diff --git a/lld/test/COFF/directives-multiple.test b/lld/test/COFF/directives-multiple.test
index 593c13fce59ec5..b0a6e543fb8071 100644
--- a/lld/test/COFF/directives-multiple.test
+++ b/lld/test/COFF/directives-multiple.test
@@ -1,16 +1,98 @@
-# RUN: lld-link /dll %p/Inputs/directives-multiple.obj /out:%t.dll
+# RUN: yaml2obj %s -o %t.obj
+# RUN: lld-link /dll %t.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: }
+# 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: }
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_LINE_NUMS_STRIPPED, IMAGE_FILE_BYTES_REVERSED_LO, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     B801000000C3
+    SizeOfRawData:   6
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     8D0411C3
+    SizeOfRawData:   4
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       4
+    SectionData:     2F4558504F52543A4164643D3F41646440405941484848405A20
+    SizeOfRawData:   26
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     9129D0C3
+    SizeOfRawData:   4
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       4
+    SectionData:     2F4558504F52543A53756274726163743D3F537562747261637440405941484848405A20
+    SizeOfRawData:   36
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            _DllMainCRTStartup
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .text
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            '?Add@@YAHHH at Z'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .drectve
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .text
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            '?Subtract@@YAHHH at Z'
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .drectve
+    Value:           0
+    SectionNumber:   5
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+...

>From 502b348649a41421238d9fde7d8a99f839609baf Mon Sep 17 00:00:00 2001
From: "Rickey Bowers Jr." <bitRAKE at gmail.com>
Date: Mon, 25 Mar 2024 14:45:18 -0600
Subject: [PATCH 7/7] Use `SmallVector` to optimize common 0/1 case.

---
 lld/COFF/InputFiles.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 055165a2e9ed68..502669459b872b 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -90,7 +90,7 @@ class InputFile {
   StringRef parentName;
 
   // Returns .drectve section(s) content if exist.
-  std::vector<StringRef> getDrectves() { return directives; }
+  llvm::SmallVector<StringRef, 0>  getDrectves() { 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) {}
 
-  std::vector<StringRef> directives;
+  llvm::SmallVector<StringRef, 0> directives;
 
 private:
   const Kind fileKind;



More information about the llvm-commits mailing list