[lld] [LLD][COFF] Add -build-id flag to generate .buildid section. (PR #71433)

Zequan Wu via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 8 09:36:25 PST 2023


https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/71433

>From 3dcfc47ec996693896f18c6aa893c91eb50202aa Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Mon, 6 Nov 2023 14:27:50 -0500
Subject: [PATCH 1/5] [LLD][COFF] Add -build-id flag to genrate .buildid
 section.

---
 lld/COFF/Config.h       |  1 +
 lld/COFF/Driver.cpp     |  5 ++++
 lld/COFF/Options.td     |  2 ++
 lld/COFF/Writer.cpp     | 22 +++++++++++------
 lld/MinGW/Options.td    |  2 +-
 lld/test/COFF/rsds.test | 54 ++++++++++++++++++++++++++---------------
 6 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 1c338cc63fa87d2..4e4811357d26a46 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -317,6 +317,7 @@ struct Configuration {
   bool writeCheckSum = false;
   EmitKind emit = EmitKind::Obj;
   bool allowDuplicateWeak = false;
+  bool buildID = false;
 };
 
 } // namespace lld::coff
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 5613c2e6993a5af..0fa6fd6d34cf6a7 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2332,6 +2332,11 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     }
   }
 
+  // Generate build id hash in .buildid section when:
+  // 1. /build-id
+  // 2. /lldmingw + /debug and not generating pdb file.
+  config->buildID = args.hasArg(OPT_build_id) ||
+                    (config->mingw && config->debug && !shouldCreatePDB);
   // Set default image base if /base is not given.
   if (config->imageBase == uint64_t(-1))
     config->imageBase = getDefaultImageBase();
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 977657a433dc581..cc7ce4fdbeea037 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -299,6 +299,8 @@ def : Flag<["--"], "time-trace">, Alias<time_trace_eq>,
 def time_trace_granularity_eq: Joined<["--"], "time-trace-granularity=">,
     HelpText<"Minimum time granularity (in microseconds) traced by time profiler">;
 
+def build_id: F<"build-id">, HelpText<"Generate build ID">;
+
 // Flags for debugging
 def lldmap : F<"lldmap">;
 def lldmap_file : P_priv<"lldmap">;
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 43d8e7c1d530859..dca148a62e9d171 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1041,14 +1041,14 @@ void Writer::createMiscChunks() {
 
   // Create Debug Information Chunks
   OutputSection *debugInfoSec = config->mingw ? buildidSec : rdataSec;
-  if (config->debug || config->repro || config->cetCompat) {
+  if (config->buildID || config->debug || config->repro || config->cetCompat) {
     debugDirectory =
         make<DebugDirectoryChunk>(ctx, debugRecords, config->repro);
     debugDirectory->setAlignment(4);
     debugInfoSec->addChunk(debugDirectory);
   }
 
-  if (config->debug) {
+  if (config->debug || config->buildID) {
     // Make a CVDebugRecordChunk even when /DEBUG:CV is not specified.  We
     // output a PDB no matter what, and this chunk provides the only means of
     // allowing a debugger to match a PDB and an executable.  So we need it even
@@ -1069,6 +1069,16 @@ void Writer::createMiscChunks() {
     debugInfoSec->addChunk(r.second);
   }
 
+  // If .buildid section was not chosen and /build-id is given, create .buildid
+  // section.
+  if (config->buildID && debugInfoSec != buildidSec) {
+    buildidSec->addChunk(debugDirectory);
+    for (std::pair<COFF::DebugType, Chunk *> r : debugRecords) {
+      r.second->setAlignment(4);
+      buildidSec->addChunk(r.second);
+    }
+  }
+
   // Create SEH table. x86-only.
   if (config->safeSEH)
     createSEHTable();
@@ -2028,7 +2038,7 @@ void Writer::writeBuildId() {
   // PE contents.
   Configuration *config = &ctx.config;
 
-  if (config->debug) {
+  if (config->debug || config->buildID) {
     assert(buildId && "BuildId is not set!");
     // BuildId->BuildId was filled in when the PDB was written.
   }
@@ -2043,16 +2053,14 @@ void Writer::writeBuildId() {
 
   uint32_t timestamp = config->timestamp;
   uint64_t hash = 0;
-  bool generateSyntheticBuildId =
-      config->mingw && config->debug && config->pdbPath.empty();
 
-  if (config->repro || generateSyntheticBuildId)
+  if (config->repro || config->buildID)
     hash = xxh3_64bits(outputFileData);
 
   if (config->repro)
     timestamp = static_cast<uint32_t>(hash);
 
-  if (generateSyntheticBuildId) {
+  if (config->buildID) {
     // For MinGW builds without a PDB file, we still generate a build id
     // to allow associating a crash dump to the executable.
     buildId->buildId->PDB70.CVSignature = OMF::Signature::PDB70;
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index fa4c4ecc75d6543..87b5c4fff68cef6 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -196,6 +196,7 @@ defm guard_longjmp : B<"guard-longjmp",
   "Do not enable Control Flow Guard long jump hardening">;
 defm error_limit:
   EqLong<"error-limit", "Maximum number of errors to emit before stopping (0 = no limit)">;
+def build_id: F<"build-id">, HelpText<"Generate build ID">;
 
 // Alias
 def alias_Bdynamic_call_shared: Flag<["-"], "call_shared">, Alias<Bdynamic>;
@@ -213,7 +214,6 @@ def alias_undefined_u: JoinedOrSeparate<["-"], "u">, Alias<undefined>;
 // Ignored options
 def: Joined<["-"], "O">;
 def: F<"as-needed">;
-def: F<"build-id">;
 def: F<"disable-auto-image-base">;
 def: F<"enable-auto-image-base">;
 def: F<"end-group">;
diff --git a/lld/test/COFF/rsds.test b/lld/test/COFF/rsds.test
index 475249ca4056669..783ac08255c031b 100644
--- a/lld/test/COFF/rsds.test
+++ b/lld/test/COFF/rsds.test
@@ -22,9 +22,23 @@
 # RUN: lld-link /Brepro /debug /dll /out:%t.dll /entry:DllMain %t.obj
 # RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix REPRODEBUG %s
 
+# Generate .buildid section when /lldmingw + /debug:dwarf are given and not generating pdb file.
 # RUN: rm -f %t.dll %t.pdb
 # RUN: lld-link /lldmingw /debug:dwarf /dll /out:%t.dll /entry:DllMain %t.obj
-# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix MINGW %s
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
+
+# RUN: rm -f %t.dll %t.pdb
+# RUN: lld-link /lldmingw /debug:dwarf /build-id /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
+
+# Generate .buildid section when /build-id is given.
+# RUN: rm -f %t.dll %t.pdb
+# RUN: lld-link /build-id /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
+
+# RUN: rm -f %t.dll %t.pdb
+# RUN: lld-link /build-id /debug /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
 
 # CHECK: File: [[FILE:.*]].dll
 # CHECK: DebugDirectory [
@@ -148,25 +162,25 @@
 # REPRODEBUG:   }
 # REPRODEBUG: ]
 
-# MINGW: File: {{.*}}.dll
-# MINGW: DebugDirectory [
-# MINGW:   DebugEntry {
-# MINGW:     Characteristics: 0x0
-# MINGW:     TimeDateStamp:
-# MINGW:     MajorVersion: 0x0
-# MINGW:     MinorVersion: 0x0
-# MINGW:     Type: CodeView (0x2)
-# MINGW:     SizeOfData: 0x{{[^0]}}
-# MINGW:     AddressOfRawData: 0x{{[^0]}}
-# MINGW:     PointerToRawData: 0x{{[^0]}}
-# MINGW:     PDBInfo {
-# MINGW:       PDBSignature: 0x53445352
-# MINGW:       PDBGUID: [[GUID:\(([A-Za-z0-9]{2} ?){16}\)]]
-# MINGW:       PDBAge: 1
-# MINGW:       PDBFileName:
-# MINGW:     }
-# MINGW:   }
-# MINGW: ]
+# BUILDID: File: {{.*}}.dll
+# BUILDID: DebugDirectory [
+# BUILDID:   DebugEntry {
+# BUILDID:     Characteristics: 0x0
+# BUILDID:     TimeDateStamp:
+# BUILDID:     MajorVersion: 0x0
+# BUILDID:     MinorVersion: 0x0
+# BUILDID:     Type: CodeView (0x2)
+# BUILDID:     SizeOfData: 0x{{[^0]}}
+# BUILDID:     AddressOfRawData: 0x{{[^0]}}
+# BUILDID:     PointerToRawData: 0x{{[^0]}}
+# BUILDID:     PDBInfo {
+# BUILDID:       PDBSignature: 0x53445352
+# BUILDID:       PDBGUID: [[GUID:\(([A-Za-z0-9]{2} ?){16}\)]]
+# BUILDID:       PDBAge: 1
+# BUILDID:       PDBFileName:
+# BUILDID:     }
+# BUILDID:   }
+# BUILDID: ]
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_I386

>From 0460323822e26c1d4f83642c2e48e38799358e22 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Tue, 7 Nov 2023 15:39:50 -0500
Subject: [PATCH 2/5] Address comments.

---
 lld/COFF/Config.h       |  9 +++++-
 lld/COFF/Driver.cpp     | 20 ++++++++-----
 lld/COFF/Options.td     |  2 +-
 lld/COFF/Writer.cpp     | 65 ++++++++++++++++++++++++++---------------
 lld/MinGW/Options.td    |  1 -
 lld/test/COFF/rsds.test | 21 +++++++++++++
 6 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 4e4811357d26a46..4186cc934b24f9a 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -102,6 +102,12 @@ enum class ICFLevel {
         // behavior.
 };
 
+enum class BuildIDHash {
+  None,
+  PDB,
+  Binary,
+};
+
 // Global configuration.
 struct Configuration {
   enum ManifestKind { Default, SideBySide, Embed, No };
@@ -317,7 +323,8 @@ struct Configuration {
   bool writeCheckSum = false;
   EmitKind emit = EmitKind::Obj;
   bool allowDuplicateWeak = false;
-  bool buildID = false;
+  bool shouldCreatePDB = false;
+  BuildIDHash buildIDHash = BuildIDHash::None;
 };
 
 } // namespace lld::coff
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 0fa6fd6d34cf6a7..b0991c023f20aed 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1672,10 +1672,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
       config->driverUponly || config->driverWdm || args.hasArg(OPT_driver);
 
   // Handle /pdb
-  bool shouldCreatePDB =
+  config->shouldCreatePDB =
       (debug == DebugKind::Full || debug == DebugKind::GHash ||
        debug == DebugKind::NoGHash);
-  if (shouldCreatePDB) {
+  if (config->shouldCreatePDB) {
     if (auto *arg = args.getLastArg(OPT_pdb))
       config->pdbPath = arg->getValue();
     if (auto *arg = args.getLastArg(OPT_pdbaltpath))
@@ -2309,7 +2309,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     config->lldmapFile.clear();
   }
 
-  if (shouldCreatePDB) {
+  if (config->shouldCreatePDB) {
     // Put the PDB next to the image if no /pdb flag was passed.
     if (config->pdbPath.empty()) {
       config->pdbPath = config->outputFile;
@@ -2332,11 +2332,15 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     }
   }
 
-  // Generate build id hash in .buildid section when:
-  // 1. /build-id
-  // 2. /lldmingw + /debug and not generating pdb file.
-  config->buildID = args.hasArg(OPT_build_id) ||
-                    (config->mingw && config->debug && !shouldCreatePDB);
+  // Generate build id hash in .buildid section if /build-id is given or
+  // /lldmingw + /debug are given:
+  // 1. If PDB is generated, the build id hash will be the hash of PDB content.
+  // 2. Otherwise, the build id hash will be the hash of executable content.
+  if (args.hasFlag(OPT_build_id, OPT_build_id_no,
+                   config->mingw && config->debug))
+    config->buildIDHash =
+        config->shouldCreatePDB ? BuildIDHash::PDB : BuildIDHash::Binary;
+
   // Set default image base if /base is not given.
   if (config->imageBase == uint64_t(-1))
     config->imageBase = getDefaultImageBase();
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index cc7ce4fdbeea037..436b65b22d838d6 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -299,7 +299,7 @@ def : Flag<["--"], "time-trace">, Alias<time_trace_eq>,
 def time_trace_granularity_eq: Joined<["--"], "time-trace-granularity=">,
     HelpText<"Minimum time granularity (in microseconds) traced by time profiler">;
 
-def build_id: F<"build-id">, HelpText<"Generate build ID">;
+defm build_id: B<"build-id", "Generate build ID", "Do not Generate build ID">;
 
 // Flags for debugging
 def lldmap : F<"lldmap">;
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index dca148a62e9d171..490c265a9dd0c7d 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -86,10 +86,8 @@ namespace {
 
 class DebugDirectoryChunk : public NonSectionChunk {
 public:
-  DebugDirectoryChunk(const COFFLinkerContext &c,
-                      const std::vector<std::pair<COFF::DebugType, Chunk *>> &r,
-                      bool writeRepro)
-      : records(r), writeRepro(writeRepro), ctx(c) {}
+  DebugDirectoryChunk(const COFFLinkerContext &c, bool writeRepro)
+      : writeRepro(writeRepro), ctx(c) {}
 
   size_t getSize() const override {
     return (records.size() + int(writeRepro)) * sizeof(debug_directory);
@@ -121,6 +119,18 @@ class DebugDirectoryChunk : public NonSectionChunk {
       *tds = timeDateStamp;
   }
 
+  void addRecord(COFF::DebugType type, Chunk *c) {
+    records.emplace_back(type, c);
+  }
+
+  Chunk* getBuildIdChunk() {
+    for (const std::pair<COFF::DebugType, Chunk *> &record : records) {
+      if (record.first == COFF::IMAGE_DEBUG_TYPE_CODEVIEW)
+        return record.second;
+    }
+    return nullptr;
+  }
+
 private:
   void fillEntry(debug_directory *d, COFF::DebugType debugType, size_t size,
                  uint64_t rva, uint64_t offs) const {
@@ -137,7 +147,7 @@ class DebugDirectoryChunk : public NonSectionChunk {
   }
 
   mutable std::vector<support::ulittle32_t *> timeDateStamps;
-  const std::vector<std::pair<COFF::DebugType, Chunk *>> &records;
+  std::vector<std::pair<COFF::DebugType, Chunk *>> records;
   bool writeRepro;
   const COFFLinkerContext &ctx;
 };
@@ -284,7 +294,6 @@ class Writer {
   uint32_t tlsAlignment = 0;
 
   DebugDirectoryChunk *debugDirectory = nullptr;
-  std::vector<std::pair<COFF::DebugType, Chunk *>> debugRecords;
   CVDebugRecordChunk *buildId = nullptr;
   ArrayRef<uint8_t> sectionTable;
 
@@ -1040,15 +1049,16 @@ void Writer::createMiscChunks() {
   }
 
   // Create Debug Information Chunks
-  OutputSection *debugInfoSec = config->mingw ? buildidSec : rdataSec;
-  if (config->buildID || config->debug || config->repro || config->cetCompat) {
-    debugDirectory =
-        make<DebugDirectoryChunk>(ctx, debugRecords, config->repro);
+  std::vector<std::pair<COFF::DebugType, Chunk *>> debugRecords;
+  OutputSection *debugInfoSec = config->shouldCreatePDB ? rdataSec : buildidSec;
+  if (config->buildIDHash != BuildIDHash::None || config->debug ||
+      config->repro || config->cetCompat) {
+    debugDirectory = make<DebugDirectoryChunk>(ctx, config->repro);
     debugDirectory->setAlignment(4);
     debugInfoSec->addChunk(debugDirectory);
   }
 
-  if (config->debug || config->buildID) {
+  if (config->debug || config->buildIDHash != BuildIDHash::None) {
     // Make a CVDebugRecordChunk even when /DEBUG:CV is not specified.  We
     // output a PDB no matter what, and this chunk provides the only means of
     // allowing a debugger to match a PDB and an executable.  So we need it even
@@ -1067,16 +1077,19 @@ void Writer::createMiscChunks() {
   for (std::pair<COFF::DebugType, Chunk *> r : debugRecords) {
     r.second->setAlignment(4);
     debugInfoSec->addChunk(r.second);
+    debugDirectory->addRecord(r.first, r.second);
   }
 
-  // If .buildid section was not chosen and /build-id is given, create .buildid
-  // section.
-  if (config->buildID && debugInfoSec != buildidSec) {
+  // Create extra .buildid section if build id was stored in .rdata.
+  if (config->buildIDHash == BuildIDHash::PDB) {
+    DebugDirectoryChunk *debugDirectory =
+        make<DebugDirectoryChunk>(ctx, config->repro);
+    debugDirectory->setAlignment(4);
+    CVDebugRecordChunk *buildId = make<CVDebugRecordChunk>(ctx);
+    buildId->setAlignment(4);
+    debugDirectory->addRecord(COFF::IMAGE_DEBUG_TYPE_CODEVIEW, buildId);
     buildidSec->addChunk(debugDirectory);
-    for (std::pair<COFF::DebugType, Chunk *> r : debugRecords) {
-      r.second->setAlignment(4);
-      buildidSec->addChunk(r.second);
-    }
+    buildidSec->addChunk(buildId);
   }
 
   // Create SEH table. x86-only.
@@ -2038,7 +2051,7 @@ void Writer::writeBuildId() {
   // PE contents.
   Configuration *config = &ctx.config;
 
-  if (config->debug || config->buildID) {
+  if (config->debug || config->buildIDHash != BuildIDHash::None) {
     assert(buildId && "BuildId is not set!");
     // BuildId->BuildId was filled in when the PDB was written.
   }
@@ -2054,15 +2067,13 @@ void Writer::writeBuildId() {
   uint32_t timestamp = config->timestamp;
   uint64_t hash = 0;
 
-  if (config->repro || config->buildID)
+  if (config->repro || config->buildIDHash != BuildIDHash::None)
     hash = xxh3_64bits(outputFileData);
 
   if (config->repro)
     timestamp = static_cast<uint32_t>(hash);
 
-  if (config->buildID) {
-    // For MinGW builds without a PDB file, we still generate a build id
-    // to allow associating a crash dump to the executable.
+  if (config->buildIDHash == BuildIDHash::Binary) {
     buildId->buildId->PDB70.CVSignature = OMF::Signature::PDB70;
     buildId->buildId->PDB70.Age = 1;
     memcpy(buildId->buildId->PDB70.Signature, &hash, 8);
@@ -2070,6 +2081,14 @@ void Writer::writeBuildId() {
     memcpy(&buildId->buildId->PDB70.Signature[8], "LLD PDB.", 8);
   }
 
+  // If using PDB hash, build id in .buildid section is not set yet.
+  if (config->buildIDHash == BuildIDHash::PDB) {
+    auto *buildIdChunk = buildidSec->chunks.back();
+    codeview::DebugInfo *buildIdInfo =
+        cast<CVDebugRecordChunk>(buildIdChunk)->buildId;
+    memcpy(buildIdInfo, buildId->buildId, sizeof(codeview::DebugInfo));
+  }
+
   if (debugDirectory)
     debugDirectory->setTimeDateStamp(timestamp);
 
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 87b5c4fff68cef6..3f3de22f2bd10f2 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -196,7 +196,6 @@ defm guard_longjmp : B<"guard-longjmp",
   "Do not enable Control Flow Guard long jump hardening">;
 defm error_limit:
   EqLong<"error-limit", "Maximum number of errors to emit before stopping (0 = no limit)">;
-def build_id: F<"build-id">, HelpText<"Generate build ID">;
 
 // Alias
 def alias_Bdynamic_call_shared: Flag<["-"], "call_shared">, Alias<Bdynamic>;
diff --git a/lld/test/COFF/rsds.test b/lld/test/COFF/rsds.test
index 783ac08255c031b..0ee015574cbeaff 100644
--- a/lld/test/COFF/rsds.test
+++ b/lld/test/COFF/rsds.test
@@ -35,10 +35,26 @@
 # RUN: rm -f %t.dll %t.pdb
 # RUN: lld-link /build-id /dll /out:%t.dll /entry:DllMain %t.obj
 # RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
+# RUN: llvm-readobj --coff-debug-directory %t.dll | grep "PDBGUID: " > %t.binary.guid
 
+# When generating PDB, generate extra .buildid section.
 # RUN: rm -f %t.dll %t.pdb
 # RUN: lld-link /build-id /debug /dll /out:%t.dll /entry:DllMain %t.obj
 # RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
+# RUN: llvm-readobj -S %t.dll | FileCheck --check-prefix BUILDID_SEC %s
+
+# RUN: llvm-readobj --coff-debug-directory %t.dll | grep "PDBGUID: " > %t.pdb.guid
+# PDB hash should be different from binary hash.
+# RUN: not diff %t.binary.guid %t.pdb.guid
+
+# Do not generate .buildid section under /lldmingw + /debug:dwarf + /build-id:no
+# RUN: rm -f %t.dll %t.pdb
+# RUN: lld-link /lldmingw /debug:dwarf /build-id:no /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix NO_BUILDID %s
+
+# RUN: rm -f %t.dll %t.pdb
+# RUN: lld-link /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix NO_BUILDID %s
 
 # CHECK: File: [[FILE:.*]].dll
 # CHECK: DebugDirectory [
@@ -181,6 +197,11 @@
 # BUILDID:     }
 # BUILDID:   }
 # BUILDID: ]
+
+# NO_BUILDID: DebugDirectory [
+# NO_BUILDID: ]
+
+# BUILDID_SEC: Name: .buildid
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_I386

>From c50f440417289071194074ea38873f0965be1bf0 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Tue, 7 Nov 2023 16:42:53 -0500
Subject: [PATCH 3/5] Always add -build-id in MinGW driver unless -no-build-id
 is given

---
 lld/COFF/Driver.cpp        |  6 ++----
 lld/COFF/Writer.cpp        |  2 +-
 lld/MinGW/Driver.cpp       |  1 +
 lld/MinGW/Options.td       |  1 +
 lld/test/COFF/rsds.test    | 13 ++-----------
 lld/test/MinGW/driver.test |  6 ++++++
 6 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index b0991c023f20aed..116c4a1f8617225 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2332,12 +2332,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     }
   }
 
-  // Generate build id hash in .buildid section if /build-id is given or
-  // /lldmingw + /debug are given:
+  // Generate build id hash in .buildid section if /build-id is given:
   // 1. If PDB is generated, the build id hash will be the hash of PDB content.
   // 2. Otherwise, the build id hash will be the hash of executable content.
-  if (args.hasFlag(OPT_build_id, OPT_build_id_no,
-                   config->mingw && config->debug))
+  if (args.hasFlag(OPT_build_id, OPT_build_id_no, false))
     config->buildIDHash =
         config->shouldCreatePDB ? BuildIDHash::PDB : BuildIDHash::Binary;
 
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 490c265a9dd0c7d..eff292bd9449fc2 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -123,7 +123,7 @@ class DebugDirectoryChunk : public NonSectionChunk {
     records.emplace_back(type, c);
   }
 
-  Chunk* getBuildIdChunk() {
+  Chunk *getBuildIdChunk() {
     for (const std::pair<COFF::DebugType, Chunk *> &record : records) {
       if (record.first == COFF::IMAGE_DEBUG_TYPE_CODEVIEW)
         return record.second;
diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 19bf2d1617057eb..48e5f434093c0b3 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -302,6 +302,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   } else if (!args.hasArg(OPT_strip_all)) {
     add("-debug:dwarf");
   }
+  add(args.hasArg(OPT_no_build_id) ? "-build-id:no" : "-build-id");
 
   if (args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false))
     add("-WX");
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 3f3de22f2bd10f2..0cdc9fea7de3a93 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -196,6 +196,7 @@ defm guard_longjmp : B<"guard-longjmp",
   "Do not enable Control Flow Guard long jump hardening">;
 defm error_limit:
   EqLong<"error-limit", "Maximum number of errors to emit before stopping (0 = no limit)">;
+defm build_id: B<"build-id", "Generate build ID", "Do not Generate build ID">;
 
 // Alias
 def alias_Bdynamic_call_shared: Flag<["-"], "call_shared">, Alias<Bdynamic>;
diff --git a/lld/test/COFF/rsds.test b/lld/test/COFF/rsds.test
index 0ee015574cbeaff..9b82b6f700919c1 100644
--- a/lld/test/COFF/rsds.test
+++ b/lld/test/COFF/rsds.test
@@ -22,15 +22,6 @@
 # RUN: lld-link /Brepro /debug /dll /out:%t.dll /entry:DllMain %t.obj
 # RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix REPRODEBUG %s
 
-# Generate .buildid section when /lldmingw + /debug:dwarf are given and not generating pdb file.
-# RUN: rm -f %t.dll %t.pdb
-# RUN: lld-link /lldmingw /debug:dwarf /dll /out:%t.dll /entry:DllMain %t.obj
-# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
-
-# RUN: rm -f %t.dll %t.pdb
-# RUN: lld-link /lldmingw /debug:dwarf /build-id /dll /out:%t.dll /entry:DllMain %t.obj
-# RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix BUILDID %s
-
 # Generate .buildid section when /build-id is given.
 # RUN: rm -f %t.dll %t.pdb
 # RUN: lld-link /build-id /dll /out:%t.dll /entry:DllMain %t.obj
@@ -47,9 +38,9 @@
 # PDB hash should be different from binary hash.
 # RUN: not diff %t.binary.guid %t.pdb.guid
 
-# Do not generate .buildid section under /lldmingw + /debug:dwarf + /build-id:no
+# Do not generate .buildid section under /build-id:no
 # RUN: rm -f %t.dll %t.pdb
-# RUN: lld-link /lldmingw /debug:dwarf /build-id:no /dll /out:%t.dll /entry:DllMain %t.obj
+# RUN: lld-link /build-id:no /dll /out:%t.dll /entry:DllMain %t.obj
 # RUN: llvm-readobj --coff-debug-directory %t.dll | FileCheck --check-prefix NO_BUILDID %s
 
 # RUN: rm -f %t.dll %t.pdb
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index a07c95edb580da1..bab2fdca9b834ac 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -389,3 +389,9 @@ Test GCC specific LTO options that GCC passes unconditionally, that we ignore.
 
 RUN: ld.lld -### foo.o -m i386pep -plugin /usr/lib/gcc/x86_64-w64-mingw32/10-posix/liblto_plugin.so -plugin-opt=/usr/lib/gcc/x86_64-w64-mingw32/10-posix/lto-wrapper -plugin-opt=-fresolution=/tmp/ccM9d4fP.res -plugin-opt=-pass-through=-lmingw32 2> /dev/null
 RUN: ld.lld -### foo.o -m i386pep -plugin C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/liblto_plugin.dll -plugin-opt=C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/lto-wrapper.exe -plugin-opt=-fresolution=C:/msys64/tmp/cckbC7wB.res -plugin-opt=-pass-through=-lmingw32 2> /dev/null
+
+RUN: ld.lld -### foo.o -m i386pep 2>&1 | FileCheck -check-prefix=BUILD_ID %s
+BUILD_ID:-build-id
+
+RUN: ld.lld -### foo.o -m i386pep -no-build-id 2>&1 | FileCheck -check-prefix=NO_BUILD_ID %s
+NO_BUILD_ID:-build-id:no

>From e2dfe8af35a587e034923e706e39c1fc4168073c Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Tue, 7 Nov 2023 17:59:52 -0500
Subject: [PATCH 4/5] Use -build-id=none format

---
 lld/MinGW/Driver.cpp       | 11 ++++++++++-
 lld/MinGW/Options.td       |  3 ++-
 lld/test/MinGW/driver.test |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 48e5f434093c0b3..922ea2a035ba657 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -302,7 +302,16 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   } else if (!args.hasArg(OPT_strip_all)) {
     add("-debug:dwarf");
   }
-  add(args.hasArg(OPT_no_build_id) ? "-build-id:no" : "-build-id");
+  if (auto *a = args.getLastArg(OPT_build_id)) {
+    StringRef v = a->getValue();
+    if (v == "none")
+      add("-build-id:no");
+    else if (v.empty())
+      add("-build-id");
+    else
+      warn("unsupported build id hashing: " + a->getSpelling());
+  } else
+    add("-build-id");
 
   if (args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false))
     add("-WX");
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 0cdc9fea7de3a93..29f3da25d8be1b9 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -196,7 +196,8 @@ defm guard_longjmp : B<"guard-longjmp",
   "Do not enable Control Flow Guard long jump hardening">;
 defm error_limit:
   EqLong<"error-limit", "Maximum number of errors to emit before stopping (0 = no limit)">;
-defm build_id: B<"build-id", "Generate build ID", "Do not Generate build ID">;
+def build_id: J<"build-id=">, HelpText<"Generate build ID note (pass none to disable)">, 
+  MetaVarName<"<arg>">;
 
 // Alias
 def alias_Bdynamic_call_shared: Flag<["-"], "call_shared">, Alias<Bdynamic>;
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index bab2fdca9b834ac..ade964dc655f179 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -393,5 +393,5 @@ RUN: ld.lld -### foo.o -m i386pep -plugin C:/msys64/mingw64/bin/../lib/gcc/x86_6
 RUN: ld.lld -### foo.o -m i386pep 2>&1 | FileCheck -check-prefix=BUILD_ID %s
 BUILD_ID:-build-id
 
-RUN: ld.lld -### foo.o -m i386pep -no-build-id 2>&1 | FileCheck -check-prefix=NO_BUILD_ID %s
+RUN: ld.lld -### foo.o -m i386pep -build-id=none 2>&1 | FileCheck -check-prefix=NO_BUILD_ID %s
 NO_BUILD_ID:-build-id:no

>From 44cd8983b87747a114e5783ade14a07c3d0bb40f Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Wed, 8 Nov 2023 12:33:50 -0500
Subject: [PATCH 5/5] address comments

---
 lld/MinGW/Driver.cpp       | 8 +++++---
 lld/MinGW/Options.td       | 1 +
 lld/test/MinGW/driver.test | 6 +++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 922ea2a035ba657..91a8ff72145d500 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -306,10 +306,12 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     StringRef v = a->getValue();
     if (v == "none")
       add("-build-id:no");
-    else if (v.empty())
+    else {
+      if (!v.empty())
+        warn("unsupported build id hashing: " + v +
+             ", using default hashing.");
       add("-build-id");
-    else
-      warn("unsupported build id hashing: " + a->getSpelling());
+    }
   } else
     add("-build-id");
 
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 29f3da25d8be1b9..d4a49cdbd535935 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -198,6 +198,7 @@ defm error_limit:
   EqLong<"error-limit", "Maximum number of errors to emit before stopping (0 = no limit)">;
 def build_id: J<"build-id=">, HelpText<"Generate build ID note (pass none to disable)">, 
   MetaVarName<"<arg>">;
+def : F<"build-id">, Alias<build_id>, HelpText<"Alias for --build-id=">;
 
 // Alias
 def alias_Bdynamic_call_shared: Flag<["-"], "call_shared">, Alias<Bdynamic>;
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index ade964dc655f179..3ab9bf5a8b9542c 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -391,7 +391,11 @@ RUN: ld.lld -### foo.o -m i386pep -plugin /usr/lib/gcc/x86_64-w64-mingw32/10-pos
 RUN: ld.lld -### foo.o -m i386pep -plugin C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/liblto_plugin.dll -plugin-opt=C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/lto-wrapper.exe -plugin-opt=-fresolution=C:/msys64/tmp/cckbC7wB.res -plugin-opt=-pass-through=-lmingw32 2> /dev/null
 
 RUN: ld.lld -### foo.o -m i386pep 2>&1 | FileCheck -check-prefix=BUILD_ID %s
+RUN: ld.lld -### foo.o -m i386pep --build-id 2>&1 | FileCheck -check-prefix=BUILD_ID %s
 BUILD_ID:-build-id
 
-RUN: ld.lld -### foo.o -m i386pep -build-id=none 2>&1 | FileCheck -check-prefix=NO_BUILD_ID %s
+RUN: ld.lld -### foo.o -m i386pep --build-id=fast 2>&1 | FileCheck -check-prefix=BUILD_ID_WARN %s
+BUILD_ID_WARN: unsupported build id hashing: fast, using default hashing.
+
+RUN: ld.lld -### foo.o -m i386pep --build-id=none 2>&1 | FileCheck -check-prefix=NO_BUILD_ID %s
 NO_BUILD_ID:-build-id:no



More information about the llvm-commits mailing list