Skip to content

[LLD][COFF] add __buildid symbol. #74652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Dec 14, 2023
Merged

[LLD][COFF] add __buildid symbol. #74652

merged 8 commits into from
Dec 14, 2023

Conversation

ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Dec 6, 2023 •
edited
Loading

After #71433, lld-link is able to always generate build id even when PDB is not generated.

This adds the __buildid symbol to points to the start of 16 bytes guid (which is after RSDS) and allows profile runtime to access it and dump it to raw profile.

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023 •
edited
Loading

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Zequan Wu (ZequanWu)

Changes

After #71433, lld-link is able to always generate build id even when PDB is not generated.

This adds the __lld_buildid symbol to points to the start of build id (which starts with RSDS) and allows profile runtime to access it and dump it to raw profile.

Full diff: https://github.com/llvm/llvm-project/pull/74652.diff

18 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+5-1)
  • (modified) lld/COFF/Writer.cpp (+2)
  • (added) lld/test/COFF/build-id-sym.s (+22)
  • (modified) lld/test/COFF/pdb-comdat.test (+5-5)
  • (modified) lld/test/COFF/pdb-global-constants.test (+3-3)
  • (modified) lld/test/COFF/pdb-global-gc.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-globals.test (+12-12)
  • (modified) lld/test/COFF/pdb-import-gc.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-local-constants.test (+1-1)
  • (modified) lld/test/COFF/pdb-publics-import.test (+5-3)
  • (modified) lld/test/COFF/pdb-safeseh.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-secrel-absolute.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-symbol-types.yaml (+4-4)
  • (modified) lld/test/COFF/pdb-type-server-guid-collision-valid.test (+1-1)
  • (modified) lld/test/COFF/pdb-type-server-simple.test (+4-4)
  • (modified) lld/test/COFF/pdb.test (+11-6)
  • (modified) lld/test/COFF/precomp-link.test (+1-1)
  • (modified) lld/test/COFF/precomp-summary-fail.test (+1-1)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc..4542a129ae764 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-
+#include "/tmp/debug.h"
 #include "Driver.h"
 #include "COFFLinkerContext.h"
 #include "Config.h"
@@ -2382,6 +2382,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     ctx.symtab.addAbsolute(mangle("__DTOR_LIST__"), 0);
   }
 
+  if (config->debug || config->buildIDHash != BuildIDHash::None) {
+    ctx.symtab.addAbsolute(mangle("__lld_buildid"), 0);
+  }
+
   // This code may add new undefined symbols to the link, which may enqueue more
   // symbol resolution tasks, so we need to continue executing tasks until we
   // converge.
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 7b1ff8071e2e3..ec252058607b4 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1121,6 +1121,8 @@ void Writer::createMiscChunks() {
     // if we're ultimately not going to write CodeView data to the PDB.
     buildId = make<CVDebugRecordChunk>(ctx);
     debugRecords.emplace_back(COFF::IMAGE_DEBUG_TYPE_CODEVIEW, buildId);
+    Symbol *buildidSym = ctx.symtab.findUnderscore("__lld_buildid");
+    replaceSymbol<DefinedSynthetic>(buildidSym, buildidSym->getName(), buildId);
   }
 
   if (config->cetCompat) {
diff --git a/lld/test/COFF/build-id-sym.s b/lld/test/COFF/build-id-sym.s
new file mode 100644
index 0000000000000..9e151e807d2d0
--- /dev/null
+++ b/lld/test/COFF/build-id-sym.s
@@ -0,0 +1,22 @@
+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
+# RUN: lld-link -entry:main %t.obj -build-id -Brepro -out:%t.exe
+# RUN: llvm-objdump -s %t.exe | FileCheck %s
+
+# Check __lld_buildid points to 0x140002038 which is the start of build id.
+
+# CHECK:      Contents of section .rdata:
+# CHECK-NEXT:  140002000 00000000 8064d9b6 00000000 02000000  .....d..........
+# CHECK-NEXT:  140002010 19000000 38200000 38060000 00000000  ....8 ..8.......
+# CHECK-NEXT:  140002020 8064d9b6 00000000 10000000 00000000  .d..............
+# CHECK-NEXT:  140002030 00000000 00000000 52534453 8064d9b6  ........RSDS.d..
+
+# CHECK:      Contents of section .data:
+# CHECK-NEXT:  140003000 38200040 01000000
+
+.globl main
+main:
+  nop
+
+.data
+  .quad __lld_buildid
diff --git a/lld/test/COFF/pdb-comdat.test b/lld/test/COFF/pdb-comdat.test
index 86c07f4af2f5c..83bd8e63a8abc 100644
--- a/lld/test/COFF/pdb-comdat.test
+++ b/lld/test/COFF/pdb-comdat.test
@@ -41,15 +41,15 @@ CHECK-LABEL:   Mod 0002 | `* Linker *`:
 CHECK-LABEL:                       Global Symbols
 CHECK-NEXT:  ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:        84 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:       112 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:             module = 1, sum name = 0, offset = 120
-CHECK-NEXT:       148 | S_PROCREF [size = 20] `bar`
+CHECK-NEXT:       176 | S_PROCREF [size = 20] `bar`
 CHECK-NEXT:             module = 2, sum name = 0, offset = 120
-CHECK-NEXT:       128 | S_PROCREF [size = 20] `foo`
+CHECK-NEXT:       156 | S_PROCREF [size = 20] `foo`
 CHECK-NEXT:             module = 1, sum name = 0, offset = 208
-CHECK-NEXT:       104 | S_GDATA32 [size = 24] `global`
+CHECK-NEXT:       132 | S_GDATA32 [size = 24] `global`
 CHECK-NEXT:             type = 0x0074 (int), addr = 0003:0000
-CHECK-NEXT:       168 | S_GDATA32 [size = 24] `global`
+CHECK-NEXT:       196 | S_GDATA32 [size = 24] `global`
 CHECK-NEXT:             type = 0x0074 (int), addr = 0003:0000
 
 CHECK:                           Symbols
diff --git a/lld/test/COFF/pdb-global-constants.test b/lld/test/COFF/pdb-global-constants.test
index b646fc9c2aa3e..935bf08600a63 100644
--- a/lld/test/COFF/pdb-global-constants.test
+++ b/lld/test/COFF/pdb-global-constants.test
@@ -20,9 +20,9 @@
 # int foobar() { return Foo + Bar; }
 
 CHECK:                       Global Symbols
-CHECK:           88 | S_CONSTANT [size = 16] `Bar`
+CHECK:          116 | S_CONSTANT [size = 16] `Bar`
 CHECK-NEXT:           type = 0x1002 (const int), value = 42
-CHECK-NEXT:      72 | S_CONSTANT [size = 16] `Foo`
+CHECK-NEXT:     100 | S_CONSTANT [size = 16] `Foo`
 CHECK-NEXT:           type = 0x1002 (const int), value = 41
-CHECK-NEXT:     128 | S_CONSTANT [size = 16] `Foo`
+CHECK-NEXT:     156 | S_CONSTANT [size = 16] `Foo`
 CHECK-NEXT:           type = 0x1002 (const int), value = 42
diff --git a/lld/test/COFF/pdb-global-gc.yaml b/lld/test/COFF/pdb-global-gc.yaml
index 5351ef545df87..55e885555ace3 100644
--- a/lld/test/COFF/pdb-global-gc.yaml
+++ b/lld/test/COFF/pdb-global-gc.yaml
@@ -16,7 +16,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 28] `__wc_mb_cur`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 28] `__wc_mb_cur`
 # CHECK-NEXT:            type = 0x0070 (char), addr = 0000:0000
 
 # CHECK:                                Symbols
diff --git a/lld/test/COFF/pdb-globals.test b/lld/test/COFF/pdb-globals.test
index a8fa2f615a735..e7d567cde6f1a 100644
--- a/lld/test/COFF/pdb-globals.test
+++ b/lld/test/COFF/pdb-globals.test
@@ -20,29 +20,29 @@ RUN: llvm-pdbutil dump -symbols -globals %t.pdb | FileCheck %s
 CHECK-LABEL:                        Global Symbols
 CHECK-NEXT: ============================================================
 CHECK-NEXT:   Records
-CHECK-NEXT:      444 | S_UDT [size = 20] `HelloPoint`
+CHECK-NEXT:      476 | S_UDT [size = 20] `HelloPoint`
 CHECK-NEXT:            original type = 0x1007
-CHECK-NEXT:      240 | S_LPROCREF [size = 24] `LocalFunc`
+CHECK-NEXT:      272 | S_LPROCREF [size = 24] `LocalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 424
-CHECK-NEXT:      192 | S_PROCREF [size = 28] `GlobalFunc`
+CHECK-NEXT:      224 | S_PROCREF [size = 28] `GlobalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 136
-CHECK-NEXT:      220 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:      252 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 224
-CHECK-NEXT:      340 | S_CONSTANT [size = 24] `ConstexprVar`
+CHECK-NEXT:      372 | S_CONSTANT [size = 24] `ConstexprVar`
 CHECK-NEXT:            type = 0x100B (const int), value = 18
-CHECK-NEXT:      264 | S_GDATA32 [size = 28] `__purecall`
+CHECK-NEXT:      296 | S_GDATA32 [size = 28] `__purecall`
 CHECK-NEXT:            type = 0x0403 (void*), addr = 0003:0004
-CHECK-NEXT:      292 | S_GDATA32 [size = 24] `GlobalVar`
+CHECK-NEXT:      324 | S_GDATA32 [size = 24] `GlobalVar`
 CHECK-NEXT:            type = 0x100C (const int*), addr = 0003:0000
-CHECK-NEXT:      316 | S_GTHREAD32 [size = 24] `GlobalTLS`
+CHECK-NEXT:      348 | S_GTHREAD32 [size = 24] `GlobalTLS`
 CHECK-NEXT:            type = 0x0074 (int), addr = 0004:0000
-CHECK-NEXT:      392 | S_LTHREAD32 [size = 24] `StaticTLS`
+CHECK-NEXT:      424 | S_LTHREAD32 [size = 24] `StaticTLS`
 CHECK-NEXT:            type = 0x0074 (int), addr = 0004:0004
-CHECK-NEXT:      416 | S_UDT [size = 28] `HelloPointTypedef`
+CHECK-NEXT:      448 | S_UDT [size = 28] `HelloPointTypedef`
 CHECK-NEXT:            original type = 0x1007
-CHECK-NEXT:      364 | S_LDATA32 [size = 28] `ConstantVar`
+CHECK-NEXT:      396 | S_LDATA32 [size = 28] `ConstantVar`
 CHECK-NEXT:            type = 0x100B (const int), addr = 0002:0000
-CHECK-NEXT:      464 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
+CHECK-NEXT:      496 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 572
 
 CHECK-LABEL:                           Symbols
diff --git a/lld/test/COFF/pdb-import-gc.yaml b/lld/test/COFF/pdb-import-gc.yaml
index 8f50a111411cc..5b0f6074fe014 100644
--- a/lld/test/COFF/pdb-import-gc.yaml
+++ b/lld/test/COFF/pdb-import-gc.yaml
@@ -14,7 +14,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 32] `__imp___wc_mb_cur`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 32] `__imp___wc_mb_cur`
 # CHECK-NEXT:            type = 0x0070 (char), addr = 0000:0000
 
 # CHECK:                                Symbols
diff --git a/lld/test/COFF/pdb-local-constants.test b/lld/test/COFF/pdb-local-constants.test
index 3a9538252ed80..8a4baad293838 100644
--- a/lld/test/COFF/pdb-local-constants.test
+++ b/lld/test/COFF/pdb-local-constants.test
@@ -13,7 +13,7 @@
 #
 
 CHECK:                       Global Symbols
-CHECK:           40 | S_CONSTANT [size = 20] `g_const`
+CHECK:           68 | S_CONSTANT [size = 20] `g_const`
 CHECK-NEXT:           type = 0x1002 (const int), value = 321
 
 CHECK:                          Symbols
diff --git a/lld/test/COFF/pdb-publics-import.test b/lld/test/COFF/pdb-publics-import.test
index 83f087b662186..08b8b66730889 100644
--- a/lld/test/COFF/pdb-publics-import.test
+++ b/lld/test/COFF/pdb-publics-import.test
@@ -54,11 +54,13 @@ CHECK-NEXT: pdb file ni: 1 `{{.*}}pdb-publics-import.test.tmp2.pdb`, src file ni
 CHECK:                             Public Symbols
 CHECK-NEXT: ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:      112 | S_PUB32 [size = 20] `main`
+CHECK-NEXT:       64 | S_PUB32 [size = 28] `__lld_buildid`
+CHECK-NEXT:            flags = none, addr = 0002:0028
+CHECK-NEXT:      140 | S_PUB32 [size = 20] `main`
 CHECK-NEXT:            flags = function, addr = 0001:0000
-CHECK-NEXT:       64 | S_PUB32 [size = 24] `exportfn1`
+CHECK-NEXT:       92 | S_PUB32 [size = 24] `exportfn1`
 CHECK-NEXT:            flags = function, addr = 0001:0016
-CHECK-NEXT:       88 | S_PUB32 [size = 24] `exportfn2`
+CHECK-NEXT:      116 | S_PUB32 [size = 24] `exportfn2`
 CHECK-NEXT:            flags = function, addr = 0001:0032
 CHECK-NEXT:       32 | S_PUB32 [size = 32] `__imp_exportfn2`
 CHECK-NEXT:            flags = none, addr = 0002:0136
diff --git a/lld/test/COFF/pdb-safeseh.yaml b/lld/test/COFF/pdb-safeseh.yaml
index cc7ddb19a49c6..6914c081cff9b 100644
--- a/lld/test/COFF/pdb-safeseh.yaml
+++ b/lld/test/COFF/pdb-safeseh.yaml
@@ -10,7 +10,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 40] `___safe_se_handler_table`
+# CHECK-NEXT:       52 | S_GDATA32 [size = 40] `___safe_se_handler_table`
 # CHECK-NEXT:            type = 0x0022 (unsigned long), addr = 0003:0000
 
 --- !COFF
diff --git a/lld/test/COFF/pdb-secrel-absolute.yaml b/lld/test/COFF/pdb-secrel-absolute.yaml
index 330106b3bbed0..a8f83ec0c11bd 100644
--- a/lld/test/COFF/pdb-secrel-absolute.yaml
+++ b/lld/test/COFF/pdb-secrel-absolute.yaml
@@ -10,7 +10,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 36] `__guard_fids_table`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 36] `__guard_fids_table`
 # CHECK-NEXT:            type = 0x0022 (unsigned long), addr = 0003:0000
 
 --- !COFF
diff --git a/lld/test/COFF/pdb-symbol-types.yaml b/lld/test/COFF/pdb-symbol-types.yaml
index d8741afd8cded..9e046b2da6119 100644
--- a/lld/test/COFF/pdb-symbol-types.yaml
+++ b/lld/test/COFF/pdb-symbol-types.yaml
@@ -16,13 +16,13 @@
 # CHECK-LABEL:                  Global Symbols
 # CHECK-NEXT:  ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       48 | S_PROCREF [size = 20] `main`
+# CHECK-NEXT:       76 | S_PROCREF [size = 20] `main`
 # CHECK-NEXT:            module = 1, sum name = 0, offset = 116
-# CHECK-NEXT:       96 | S_UDT [size = 16] `UDT_Foo`
+# CHECK-NEXT:      124 | S_UDT [size = 16] `UDT_Foo`
 # CHECK-NEXT:            original type = 0x1004
-# CHECK-NEXT:      112 | S_UDT [size = 12] `Foo`
+# CHECK-NEXT:      140 | S_UDT [size = 12] `Foo`
 # CHECK-NEXT:            original type = 0x1004
-# CHECK-NEXT:       68 | S_GDATA32 [size = 28] `global_foo`
+# CHECK-NEXT:       96 | S_GDATA32 [size = 28] `global_foo`
 # CHECK-NEXT:            type = 0x1004 (Foo), addr = 0003:0000
 
 # CHECK:                           Symbols
diff --git a/lld/test/COFF/pdb-type-server-guid-collision-valid.test b/lld/test/COFF/pdb-type-server-guid-collision-valid.test
index a3819eb278860..23cfa0e0588bb 100644
--- a/lld/test/COFF/pdb-type-server-guid-collision-valid.test
+++ b/lld/test/COFF/pdb-type-server-guid-collision-valid.test
@@ -13,5 +13,5 @@ RUN: llvm-pdbutil dump -globals collision.pdb | FileCheck %s -check-prefix DUMP
 DUMP-LABEL:                       Global Symbols
 DUMP:       ============================================================
 
-DUMP:     100 | S_GDATA32 [size = 24] `bar_gv`
+DUMP:     128 | S_GDATA32 [size = 24] `bar_gv`
 DUMP-NEXT:           type = 0x104E (Bar), addr = 0002:0004
diff --git a/lld/test/COFF/pdb-type-server-simple.test b/lld/test/COFF/pdb-type-server-simple.test
index e9757d187e2f1..e437eb27f26e8 100644
--- a/lld/test/COFF/pdb-type-server-simple.test
+++ b/lld/test/COFF/pdb-type-server-simple.test
@@ -66,11 +66,11 @@ CHECK:            {{.*}}: `b.c`
 CHECK-LABEL:                       Global Symbols
 CHECK:       ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:       36 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:       64 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 104
-CHECK-NEXT:       68 | S_PROCREF [size = 16] `g`
+CHECK-NEXT:       96 | S_PROCREF [size = 16] `g`
 CHECK-NEXT:            module = 2, sum name = 0, offset = 104
-CHECK-NEXT:       56 | S_UDT [size = 12] `Foo`
+CHECK-NEXT:       84 | S_UDT [size = 12] `Foo`
 CHECK-NEXT:            original type = 0x1006
 
 CHECK-LABEL:                           Symbols
@@ -112,7 +112,7 @@ SUMMARY-NEXT:              16 Merged IPI records
 SUMMARY-NEXT:               3 Output PDB strings
 SUMMARY-NEXT:               4 Global symbol records
 SUMMARY-NEXT:              14 Module symbol records
-SUMMARY-NEXT:               2 Public symbol records
+SUMMARY-NEXT:               3 Public symbol records
 
 SUMMARY:      Top 10 types responsible for the most TPI input:
 SUMMARY-NEXT:        index     total bytes   count     size
diff --git a/lld/test/COFF/pdb.test b/lld/test/COFF/pdb.test
index 5492fececccd5..6db5e79f95d10 100644
--- a/lld/test/COFF/pdb.test
+++ b/lld/test/COFF/pdb.test
@@ -184,23 +184,28 @@ RAW-NEXT:            0x1006: ` -I"C:\Program Files (x86)\Windows Kits\8.1\includ
 RAW:                            Public Symbols
 RAW-NEXT: ============================================================
 RAW-NEXT:   Publics Header
-RAW-NEXT:     sym hash = 556, thunk table addr = 0000:0000
+RAW-NEXT:     sym hash = 568, thunk table addr = 0000:0000
 RAW-NEXT:   GSI Header
-RAW-NEXT:     sig = 0xFFFFFFFF, hdr = 0xF12F091A, hr size = 16, num buckets = 524
+RAW-NEXT:     sig = 0xFFFFFFFF, hdr = 0xF12F091A, hr size = 24, num buckets = 528
 RAW-NEXT:   Records
-RAW-NEXT:       20 | S_PUB32 [size = 20] `main`
+RAW-NEXT:        0 | S_PUB32 [size = 28] `__lld_buildid`
+RAW-NEXT:            flags = none, addr = 0002:0028
+RAW-NEXT:       48 | S_PUB32 [size = 20] `main`
 RAW-NEXT:            flags = function, addr = 0001:0000
-RAW-NEXT:        0 | S_PUB32 [size = 20] `foo`
+RAW-NEXT:       28 | S_PUB32 [size = 20] `foo`
 RAW-NEXT:            flags = function, addr = 0001:0016
 RAW-NOT:             S_PUB32
 RAW-NEXT:   Hash Entries
-RAW-NEXT:     off = 21, refcnt = 1
 RAW-NEXT:     off = 1, refcnt = 1
+RAW-NEXT:     off = 49, refcnt = 1
+RAW-NEXT:     off = 29, refcnt = 1
 RAW-NEXT:   Hash Buckets
 RAW-NEXT:     0x00000000
 RAW-NEXT:     0x0000000c
+RAW-NEXT:     0x00000018
 RAW-NEXT:   Address Map
-RAW-NEXT:     off = 20
+RAW-NEXT:     off = 48
+RAW-NEXT:     off = 28
 RAW-NEXT:     off = 0
 RAW:                            Section Headers
 RAW-NEXT: ============================================================
diff --git a/lld/test/COFF/precomp-link.test b/lld/test/COFF/precomp-link.test
index 53d981c4e6228..5985f16d12ccb 100644
--- a/lld/test/COFF/precomp-link.test
+++ b/lld/test/COFF/precomp-link.test
@@ -68,7 +68,7 @@ SUMMARY-NEXT:             170 Merged IPI records
 SUMMARY-NEXT:               5 Output PDB strings
 SUMMARY-NEXT:             167 Global symbol records
 SUMMARY-NEXT:              20 Module symbol records
-SUMMARY-NEXT:               3 Public symbol records
+SUMMARY-NEXT:               4 Public symbol records
 
 // precomp.h
 #pragma once
diff --git a/lld/test/COFF/precomp-summary-fail.test b/lld/test/COFF/precomp-summary-fail.test
index 5ebba9a1d3c74..dfafae8973ec8 100644
--- a/lld/test/COFF/precomp-summary-fail.test
+++ b/lld/test/COFF/precomp-summary-fail.test
@@ -21,4 +21,4 @@ SUMMARY-NEXT:               2 Merged IPI records
 SUMMARY-NEXT:               1 Output PDB strings
 SUMMARY-NEXT:               0 Global symbol records
 SUMMARY-NEXT:               4 Module symbol records
-SUMMARY-NEXT:               0 Public symbol records
+SUMMARY-NEXT:               1 Public symbol records

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-lld

Author: Zequan Wu (ZequanWu)

Changes

After #71433, lld-link is able to always generate build id even when PDB is not generated.

This adds the __lld_buildid symbol to points to the start of build id (which starts with RSDS) and allows profile runtime to access it and dump it to raw profile.

Full diff: https://github.com/llvm/llvm-project/pull/74652.diff

18 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+5-1)
  • (modified) lld/COFF/Writer.cpp (+2)
  • (added) lld/test/COFF/build-id-sym.s (+22)
  • (modified) lld/test/COFF/pdb-comdat.test (+5-5)
  • (modified) lld/test/COFF/pdb-global-constants.test (+3-3)
  • (modified) lld/test/COFF/pdb-global-gc.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-globals.test (+12-12)
  • (modified) lld/test/COFF/pdb-import-gc.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-local-constants.test (+1-1)
  • (modified) lld/test/COFF/pdb-publics-import.test (+5-3)
  • (modified) lld/test/COFF/pdb-safeseh.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-secrel-absolute.yaml (+1-1)
  • (modified) lld/test/COFF/pdb-symbol-types.yaml (+4-4)
  • (modified) lld/test/COFF/pdb-type-server-guid-collision-valid.test (+1-1)
  • (modified) lld/test/COFF/pdb-type-server-simple.test (+4-4)
  • (modified) lld/test/COFF/pdb.test (+11-6)
  • (modified) lld/test/COFF/precomp-link.test (+1-1)
  • (modified) lld/test/COFF/precomp-summary-fail.test (+1-1)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc..4542a129ae764 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-
+#include "/tmp/debug.h"
 #include "Driver.h"
 #include "COFFLinkerContext.h"
 #include "Config.h"
@@ -2382,6 +2382,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     ctx.symtab.addAbsolute(mangle("__DTOR_LIST__"), 0);
   }
 
+  if (config->debug || config->buildIDHash != BuildIDHash::None) {
+    ctx.symtab.addAbsolute(mangle("__lld_buildid"), 0);
+  }
+
   // This code may add new undefined symbols to the link, which may enqueue more
   // symbol resolution tasks, so we need to continue executing tasks until we
   // converge.
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 7b1ff8071e2e3..ec252058607b4 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1121,6 +1121,8 @@ void Writer::createMiscChunks() {
     // if we're ultimately not going to write CodeView data to the PDB.
     buildId = make<CVDebugRecordChunk>(ctx);
     debugRecords.emplace_back(COFF::IMAGE_DEBUG_TYPE_CODEVIEW, buildId);
+    Symbol *buildidSym = ctx.symtab.findUnderscore("__lld_buildid");
+    replaceSymbol<DefinedSynthetic>(buildidSym, buildidSym->getName(), buildId);
   }
 
   if (config->cetCompat) {
diff --git a/lld/test/COFF/build-id-sym.s b/lld/test/COFF/build-id-sym.s
new file mode 100644
index 0000000000000..9e151e807d2d0
--- /dev/null
+++ b/lld/test/COFF/build-id-sym.s
@@ -0,0 +1,22 @@
+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
+# RUN: lld-link -entry:main %t.obj -build-id -Brepro -out:%t.exe
+# RUN: llvm-objdump -s %t.exe | FileCheck %s
+
+# Check __lld_buildid points to 0x140002038 which is the start of build id.
+
+# CHECK:      Contents of section .rdata:
+# CHECK-NEXT:  140002000 00000000 8064d9b6 00000000 02000000  .....d..........
+# CHECK-NEXT:  140002010 19000000 38200000 38060000 00000000  ....8 ..8.......
+# CHECK-NEXT:  140002020 8064d9b6 00000000 10000000 00000000  .d..............
+# CHECK-NEXT:  140002030 00000000 00000000 52534453 8064d9b6  ........RSDS.d..
+
+# CHECK:      Contents of section .data:
+# CHECK-NEXT:  140003000 38200040 01000000
+
+.globl main
+main:
+  nop
+
+.data
+  .quad __lld_buildid
diff --git a/lld/test/COFF/pdb-comdat.test b/lld/test/COFF/pdb-comdat.test
index 86c07f4af2f5c..83bd8e63a8abc 100644
--- a/lld/test/COFF/pdb-comdat.test
+++ b/lld/test/COFF/pdb-comdat.test
@@ -41,15 +41,15 @@ CHECK-LABEL:   Mod 0002 | `* Linker *`:
 CHECK-LABEL:                       Global Symbols
 CHECK-NEXT:  ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:        84 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:       112 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:             module = 1, sum name = 0, offset = 120
-CHECK-NEXT:       148 | S_PROCREF [size = 20] `bar`
+CHECK-NEXT:       176 | S_PROCREF [size = 20] `bar`
 CHECK-NEXT:             module = 2, sum name = 0, offset = 120
-CHECK-NEXT:       128 | S_PROCREF [size = 20] `foo`
+CHECK-NEXT:       156 | S_PROCREF [size = 20] `foo`
 CHECK-NEXT:             module = 1, sum name = 0, offset = 208
-CHECK-NEXT:       104 | S_GDATA32 [size = 24] `global`
+CHECK-NEXT:       132 | S_GDATA32 [size = 24] `global`
 CHECK-NEXT:             type = 0x0074 (int), addr = 0003:0000
-CHECK-NEXT:       168 | S_GDATA32 [size = 24] `global`
+CHECK-NEXT:       196 | S_GDATA32 [size = 24] `global`
 CHECK-NEXT:             type = 0x0074 (int), addr = 0003:0000
 
 CHECK:                           Symbols
diff --git a/lld/test/COFF/pdb-global-constants.test b/lld/test/COFF/pdb-global-constants.test
index b646fc9c2aa3e..935bf08600a63 100644
--- a/lld/test/COFF/pdb-global-constants.test
+++ b/lld/test/COFF/pdb-global-constants.test
@@ -20,9 +20,9 @@
 # int foobar() { return Foo + Bar; }
 
 CHECK:                       Global Symbols
-CHECK:           88 | S_CONSTANT [size = 16] `Bar`
+CHECK:          116 | S_CONSTANT [size = 16] `Bar`
 CHECK-NEXT:           type = 0x1002 (const int), value = 42
-CHECK-NEXT:      72 | S_CONSTANT [size = 16] `Foo`
+CHECK-NEXT:     100 | S_CONSTANT [size = 16] `Foo`
 CHECK-NEXT:           type = 0x1002 (const int), value = 41
-CHECK-NEXT:     128 | S_CONSTANT [size = 16] `Foo`
+CHECK-NEXT:     156 | S_CONSTANT [size = 16] `Foo`
 CHECK-NEXT:           type = 0x1002 (const int), value = 42
diff --git a/lld/test/COFF/pdb-global-gc.yaml b/lld/test/COFF/pdb-global-gc.yaml
index 5351ef545df87..55e885555ace3 100644
--- a/lld/test/COFF/pdb-global-gc.yaml
+++ b/lld/test/COFF/pdb-global-gc.yaml
@@ -16,7 +16,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 28] `__wc_mb_cur`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 28] `__wc_mb_cur`
 # CHECK-NEXT:            type = 0x0070 (char), addr = 0000:0000
 
 # CHECK:                                Symbols
diff --git a/lld/test/COFF/pdb-globals.test b/lld/test/COFF/pdb-globals.test
index a8fa2f615a735..e7d567cde6f1a 100644
--- a/lld/test/COFF/pdb-globals.test
+++ b/lld/test/COFF/pdb-globals.test
@@ -20,29 +20,29 @@ RUN: llvm-pdbutil dump -symbols -globals %t.pdb | FileCheck %s
 CHECK-LABEL:                        Global Symbols
 CHECK-NEXT: ============================================================
 CHECK-NEXT:   Records
-CHECK-NEXT:      444 | S_UDT [size = 20] `HelloPoint`
+CHECK-NEXT:      476 | S_UDT [size = 20] `HelloPoint`
 CHECK-NEXT:            original type = 0x1007
-CHECK-NEXT:      240 | S_LPROCREF [size = 24] `LocalFunc`
+CHECK-NEXT:      272 | S_LPROCREF [size = 24] `LocalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 424
-CHECK-NEXT:      192 | S_PROCREF [size = 28] `GlobalFunc`
+CHECK-NEXT:      224 | S_PROCREF [size = 28] `GlobalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 136
-CHECK-NEXT:      220 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:      252 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 224
-CHECK-NEXT:      340 | S_CONSTANT [size = 24] `ConstexprVar`
+CHECK-NEXT:      372 | S_CONSTANT [size = 24] `ConstexprVar`
 CHECK-NEXT:            type = 0x100B (const int), value = 18
-CHECK-NEXT:      264 | S_GDATA32 [size = 28] `__purecall`
+CHECK-NEXT:      296 | S_GDATA32 [size = 28] `__purecall`
 CHECK-NEXT:            type = 0x0403 (void*), addr = 0003:0004
-CHECK-NEXT:      292 | S_GDATA32 [size = 24] `GlobalVar`
+CHECK-NEXT:      324 | S_GDATA32 [size = 24] `GlobalVar`
 CHECK-NEXT:            type = 0x100C (const int*), addr = 0003:0000
-CHECK-NEXT:      316 | S_GTHREAD32 [size = 24] `GlobalTLS`
+CHECK-NEXT:      348 | S_GTHREAD32 [size = 24] `GlobalTLS`
 CHECK-NEXT:            type = 0x0074 (int), addr = 0004:0000
-CHECK-NEXT:      392 | S_LTHREAD32 [size = 24] `StaticTLS`
+CHECK-NEXT:      424 | S_LTHREAD32 [size = 24] `StaticTLS`
 CHECK-NEXT:            type = 0x0074 (int), addr = 0004:0004
-CHECK-NEXT:      416 | S_UDT [size = 28] `HelloPointTypedef`
+CHECK-NEXT:      448 | S_UDT [size = 28] `HelloPointTypedef`
 CHECK-NEXT:            original type = 0x1007
-CHECK-NEXT:      364 | S_LDATA32 [size = 28] `ConstantVar`
+CHECK-NEXT:      396 | S_LDATA32 [size = 28] `ConstantVar`
 CHECK-NEXT:            type = 0x100B (const int), addr = 0002:0000
-CHECK-NEXT:      464 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
+CHECK-NEXT:      496 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 572
 
 CHECK-LABEL:                           Symbols
diff --git a/lld/test/COFF/pdb-import-gc.yaml b/lld/test/COFF/pdb-import-gc.yaml
index 8f50a111411cc..5b0f6074fe014 100644
--- a/lld/test/COFF/pdb-import-gc.yaml
+++ b/lld/test/COFF/pdb-import-gc.yaml
@@ -14,7 +14,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 32] `__imp___wc_mb_cur`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 32] `__imp___wc_mb_cur`
 # CHECK-NEXT:            type = 0x0070 (char), addr = 0000:0000
 
 # CHECK:                                Symbols
diff --git a/lld/test/COFF/pdb-local-constants.test b/lld/test/COFF/pdb-local-constants.test
index 3a9538252ed80..8a4baad293838 100644
--- a/lld/test/COFF/pdb-local-constants.test
+++ b/lld/test/COFF/pdb-local-constants.test
@@ -13,7 +13,7 @@
 #
 
 CHECK:                       Global Symbols
-CHECK:           40 | S_CONSTANT [size = 20] `g_const`
+CHECK:           68 | S_CONSTANT [size = 20] `g_const`
 CHECK-NEXT:           type = 0x1002 (const int), value = 321
 
 CHECK:                          Symbols
diff --git a/lld/test/COFF/pdb-publics-import.test b/lld/test/COFF/pdb-publics-import.test
index 83f087b662186..08b8b66730889 100644
--- a/lld/test/COFF/pdb-publics-import.test
+++ b/lld/test/COFF/pdb-publics-import.test
@@ -54,11 +54,13 @@ CHECK-NEXT: pdb file ni: 1 `{{.*}}pdb-publics-import.test.tmp2.pdb`, src file ni
 CHECK:                             Public Symbols
 CHECK-NEXT: ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:      112 | S_PUB32 [size = 20] `main`
+CHECK-NEXT:       64 | S_PUB32 [size = 28] `__lld_buildid`
+CHECK-NEXT:            flags = none, addr = 0002:0028
+CHECK-NEXT:      140 | S_PUB32 [size = 20] `main`
 CHECK-NEXT:            flags = function, addr = 0001:0000
-CHECK-NEXT:       64 | S_PUB32 [size = 24] `exportfn1`
+CHECK-NEXT:       92 | S_PUB32 [size = 24] `exportfn1`
 CHECK-NEXT:            flags = function, addr = 0001:0016
-CHECK-NEXT:       88 | S_PUB32 [size = 24] `exportfn2`
+CHECK-NEXT:      116 | S_PUB32 [size = 24] `exportfn2`
 CHECK-NEXT:            flags = function, addr = 0001:0032
 CHECK-NEXT:       32 | S_PUB32 [size = 32] `__imp_exportfn2`
 CHECK-NEXT:            flags = none, addr = 0002:0136
diff --git a/lld/test/COFF/pdb-safeseh.yaml b/lld/test/COFF/pdb-safeseh.yaml
index cc7ddb19a49c6..6914c081cff9b 100644
--- a/lld/test/COFF/pdb-safeseh.yaml
+++ b/lld/test/COFF/pdb-safeseh.yaml
@@ -10,7 +10,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 40] `___safe_se_handler_table`
+# CHECK-NEXT:       52 | S_GDATA32 [size = 40] `___safe_se_handler_table`
 # CHECK-NEXT:            type = 0x0022 (unsigned long), addr = 0003:0000
 
 --- !COFF
diff --git a/lld/test/COFF/pdb-secrel-absolute.yaml b/lld/test/COFF/pdb-secrel-absolute.yaml
index 330106b3bbed0..a8f83ec0c11bd 100644
--- a/lld/test/COFF/pdb-secrel-absolute.yaml
+++ b/lld/test/COFF/pdb-secrel-absolute.yaml
@@ -10,7 +10,7 @@
 # CHECK:                             Global Symbols
 # CHECK-NEXT: ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       20 | S_GDATA32 [size = 36] `__guard_fids_table`
+# CHECK-NEXT:       48 | S_GDATA32 [size = 36] `__guard_fids_table`
 # CHECK-NEXT:            type = 0x0022 (unsigned long), addr = 0003:0000
 
 --- !COFF
diff --git a/lld/test/COFF/pdb-symbol-types.yaml b/lld/test/COFF/pdb-symbol-types.yaml
index d8741afd8cded..9e046b2da6119 100644
--- a/lld/test/COFF/pdb-symbol-types.yaml
+++ b/lld/test/COFF/pdb-symbol-types.yaml
@@ -16,13 +16,13 @@
 # CHECK-LABEL:                  Global Symbols
 # CHECK-NEXT:  ============================================================
 # CHECK-NEXT:   Records
-# CHECK-NEXT:       48 | S_PROCREF [size = 20] `main`
+# CHECK-NEXT:       76 | S_PROCREF [size = 20] `main`
 # CHECK-NEXT:            module = 1, sum name = 0, offset = 116
-# CHECK-NEXT:       96 | S_UDT [size = 16] `UDT_Foo`
+# CHECK-NEXT:      124 | S_UDT [size = 16] `UDT_Foo`
 # CHECK-NEXT:            original type = 0x1004
-# CHECK-NEXT:      112 | S_UDT [size = 12] `Foo`
+# CHECK-NEXT:      140 | S_UDT [size = 12] `Foo`
 # CHECK-NEXT:            original type = 0x1004
-# CHECK-NEXT:       68 | S_GDATA32 [size = 28] `global_foo`
+# CHECK-NEXT:       96 | S_GDATA32 [size = 28] `global_foo`
 # CHECK-NEXT:            type = 0x1004 (Foo), addr = 0003:0000
 
 # CHECK:                           Symbols
diff --git a/lld/test/COFF/pdb-type-server-guid-collision-valid.test b/lld/test/COFF/pdb-type-server-guid-collision-valid.test
index a3819eb278860..23cfa0e0588bb 100644
--- a/lld/test/COFF/pdb-type-server-guid-collision-valid.test
+++ b/lld/test/COFF/pdb-type-server-guid-collision-valid.test
@@ -13,5 +13,5 @@ RUN: llvm-pdbutil dump -globals collision.pdb | FileCheck %s -check-prefix DUMP
 DUMP-LABEL:                       Global Symbols
 DUMP:       ============================================================
 
-DUMP:     100 | S_GDATA32 [size = 24] `bar_gv`
+DUMP:     128 | S_GDATA32 [size = 24] `bar_gv`
 DUMP-NEXT:           type = 0x104E (Bar), addr = 0002:0004
diff --git a/lld/test/COFF/pdb-type-server-simple.test b/lld/test/COFF/pdb-type-server-simple.test
index e9757d187e2f1..e437eb27f26e8 100644
--- a/lld/test/COFF/pdb-type-server-simple.test
+++ b/lld/test/COFF/pdb-type-server-simple.test
@@ -66,11 +66,11 @@ CHECK:            {{.*}}: `b.c`
 CHECK-LABEL:                       Global Symbols
 CHECK:       ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:       36 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:       64 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 104
-CHECK-NEXT:       68 | S_PROCREF [size = 16] `g`
+CHECK-NEXT:       96 | S_PROCREF [size = 16] `g`
 CHECK-NEXT:            module = 2, sum name = 0, offset = 104
-CHECK-NEXT:       56 | S_UDT [size = 12] `Foo`
+CHECK-NEXT:       84 | S_UDT [size = 12] `Foo`
 CHECK-NEXT:            original type = 0x1006
 
 CHECK-LABEL:                           Symbols
@@ -112,7 +112,7 @@ SUMMARY-NEXT:              16 Merged IPI records
 SUMMARY-NEXT:               3 Output PDB strings
 SUMMARY-NEXT:               4 Global symbol records
 SUMMARY-NEXT:              14 Module symbol records
-SUMMARY-NEXT:               2 Public symbol records
+SUMMARY-NEXT:               3 Public symbol records
 
 SUMMARY:      Top 10 types responsible for the most TPI input:
 SUMMARY-NEXT:        index     total bytes   count     size
diff --git a/lld/test/COFF/pdb.test b/lld/test/COFF/pdb.test
index 5492fececccd5..6db5e79f95d10 100644
--- a/lld/test/COFF/pdb.test
+++ b/lld/test/COFF/pdb.test
@@ -184,23 +184,28 @@ RAW-NEXT:            0x1006: ` -I"C:\Program Files (x86)\Windows Kits\8.1\includ
 RAW:                            Public Symbols
 RAW-NEXT: ============================================================
 RAW-NEXT:   Publics Header
-RAW-NEXT:     sym hash = 556, thunk table addr = 0000:0000
+RAW-NEXT:     sym hash = 568, thunk table addr = 0000:0000
 RAW-NEXT:   GSI Header
-RAW-NEXT:     sig = 0xFFFFFFFF, hdr = 0xF12F091A, hr size = 16, num buckets = 524
+RAW-NEXT:     sig = 0xFFFFFFFF, hdr = 0xF12F091A, hr size = 24, num buckets = 528
 RAW-NEXT:   Records
-RAW-NEXT:       20 | S_PUB32 [size = 20] `main`
+RAW-NEXT:        0 | S_PUB32 [size = 28] `__lld_buildid`
+RAW-NEXT:            flags = none, addr = 0002:0028
+RAW-NEXT:       48 | S_PUB32 [size = 20] `main`
 RAW-NEXT:            flags = function, addr = 0001:0000
-RAW-NEXT:        0 | S_PUB32 [size = 20] `foo`
+RAW-NEXT:       28 | S_PUB32 [size = 20] `foo`
 RAW-NEXT:            flags = function, addr = 0001:0016
 RAW-NOT:             S_PUB32
 RAW-NEXT:   Hash Entries
-RAW-NEXT:     off = 21, refcnt = 1
 RAW-NEXT:     off = 1, refcnt = 1
+RAW-NEXT:     off = 49, refcnt = 1
+RAW-NEXT:     off = 29, refcnt = 1
 RAW-NEXT:   Hash Buckets
 RAW-NEXT:     0x00000000
 RAW-NEXT:     0x0000000c
+RAW-NEXT:     0x00000018
 RAW-NEXT:   Address Map
-RAW-NEXT:     off = 20
+RAW-NEXT:     off = 48
+RAW-NEXT:     off = 28
 RAW-NEXT:     off = 0
 RAW:                            Section Headers
 RAW-NEXT: ============================================================
diff --git a/lld/test/COFF/precomp-link.test b/lld/test/COFF/precomp-link.test
index 53d981c4e6228..5985f16d12ccb 100644
--- a/lld/test/COFF/precomp-link.test
+++ b/lld/test/COFF/precomp-link.test
@@ -68,7 +68,7 @@ SUMMARY-NEXT:             170 Merged IPI records
 SUMMARY-NEXT:               5 Output PDB strings
 SUMMARY-NEXT:             167 Global symbol records
 SUMMARY-NEXT:              20 Module symbol records
-SUMMARY-NEXT:               3 Public symbol records
+SUMMARY-NEXT:               4 Public symbol records
 
 // precomp.h
 #pragma once
diff --git a/lld/test/COFF/precomp-summary-fail.test b/lld/test/COFF/precomp-summary-fail.test
index 5ebba9a1d3c74..dfafae8973ec8 100644
--- a/lld/test/COFF/precomp-summary-fail.test
+++ b/lld/test/COFF/precomp-summary-fail.test
@@ -21,4 +21,4 @@ SUMMARY-NEXT:               2 Merged IPI records
 SUMMARY-NEXT:               1 Output PDB strings
 SUMMARY-NEXT:               0 Global symbol records
 SUMMARY-NEXT:               4 Module symbol records
-SUMMARY-NEXT:               0 Public symbol records
+SUMMARY-NEXT:               1 Public symbol records

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2023 •
edited
Loading 

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

@rnk
Copy link
Collaborator

rnk commented Dec 11, 2023

Can we handle this like _load_config_used, so the symbol only appears when referenced, kind of like with a static archive?
https://github.com/llvm/llvm-project/blob/main/lld/COFF/Driver.cpp#L2427

@ZequanWu ZequanWu requested review from rnk and amykhuang December 12, 2023 19:44
@ZequanWu
Copy link
Contributor Author

Can we handle this like _load_config_used, so the symbol only appears when referenced, kind of like with a static archive? https://github.com/llvm/llvm-project/blob/main/lld/COFF/Driver.cpp#L2427

Done.

rnk
rnk approved these changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think we can land this.

@petrhosek
Copy link
Member

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

@rnk
Copy link
Collaborator

rnk commented Dec 12, 2023

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

That's reasonable. Can we make the name even more descriptive to reduce the risk of symbol conflicts? There are many implementers in the implementer's namespace (__). One idea: __buildid_guid __build_guid (it is a GUID, isn't it?).

@mstorsjo
Copy link
Member

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

That's reasonable. Can we make the name even more descriptive to reduce the risk of symbol conflicts? There are many implementers in the implementer's namespace (__). One idea: __buildid_guid __build_guid (it is a GUID, isn't it?).

IIRC this doesn't point at a pure GUID, but at a whole debug directory which does contain a GUID and some timestamps and stubs for descriptions about the debug info.

@ZequanWu
Copy link
Contributor Author

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

That's reasonable. Can we make the name even more descriptive to reduce the risk of symbol conflicts? There are many implementers in the implementer's namespace (__). One idea: __buildid_guid __build_guid (it is a GUID, isn't it?).

IIRC this doesn't point at a pure GUID, but at a whole debug directory which does contain a GUID and some timestamps and stubs for descriptions about the debug info.

This points to the 24 bytes GUID which starts with RSDS followed by 16 bytes hash and 4 bytes age.

@ZequanWu
Copy link
Contributor Author

Renamed to __build_guid.

@mstorsjo
Copy link
Member

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

That's reasonable. Can we make the name even more descriptive to reduce the risk of symbol conflicts? There are many implementers in the implementer's namespace (__). One idea: __buildid_guid __build_guid (it is a GUID, isn't it?).

IIRC this doesn't point at a pure GUID, but at a whole debug directory which does contain a GUID and some timestamps and stubs for descriptions about the debug info.

This points to the 24 bytes GUID which starts with RSDS followed by 16 bytes hash and 4 bytes age.

Sorry, not to be nitpicky, but generally, GUID (or UUID) refers to a 128 bit ID, which in this case would be the 16 bytes hash. So if we call this __build_guid, someone might read only the following 16 bytes, which contains the RSDS header and only 12 bytes of actual hash - which isn't what probably was intended.

So either we have the symbol pointing at the raw hash and call it hash or GUID something like that, or we probably should make it clearer in the name that it actually is a debug directory structure.

@rnk
Copy link
Collaborator

rnk commented Dec 12, 2023

Sorry, not to be nitpicky, but generally, GUID (or UUID) refers to a 128 bit ID, which in this case would be the 16 bytes hash. So if we call this __build_guid, someone might read only the following 16 bytes, which contains the RSDS header and only 12 bytes of actual hash - which isn't what probably was intended.

So either we have the symbol pointing at the raw hash and call it hash or GUID something like that, or we probably should make it clearer in the name that it actually is a debug directory structure.

No worries, I share your concern, if it's not a real 16 byte GUID, we shouldn't call it one.

To your point, this change should document the expected struct.

@ZequanWu
Copy link
Contributor Author

Can we avoid lld in the name and call it just __buildid? I'm worried that calling it __lld_buildid might discourage other COFF linkers from adopting it.

That's reasonable. Can we make the name even more descriptive to reduce the risk of symbol conflicts? There are many implementers in the implementer's namespace (__). One idea: __buildid_guid __build_guid (it is a GUID, isn't it?).

IIRC this doesn't point at a pure GUID, but at a whole debug directory which does contain a GUID and some timestamps and stubs for descriptions about the debug info.

This points to the 24 bytes GUID which starts with RSDS followed by 16 bytes hash and 4 bytes age.

Sorry, not to be nitpicky, but generally, GUID (or UUID) refers to a 128 bit ID, which in this case would be the 16 bytes hash. So if we call this __build_guid, someone might read only the following 16 bytes, which contains the RSDS header and only 12 bytes of actual hash - which isn't what probably was intended.

So either we have the symbol pointing at the raw hash and call it hash or GUID something like that, or we probably should make it clearer in the name that it actually is a debug directory structure.

I added an offset field in DefinedSynthetic and let __build_guid points to 4 bytes after the CVDebugRecordChunk. So it will points to the 16 byte raw hash.

I tested on Windows, and this allows me to print the 16 byte __build_guid directly.

@ZequanWu ZequanWu changed the title [LLD][COFF] add __lld_buildid symbol. [LLD][COFF] add __build_guid symbol. Dec 14, 2023
rnk
rnk reviewed Dec 14, 2023
View reviewed changes
Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should go back to __buildid, I withdraw my suggestion for a more descriptive name. I'm just concerned someone will come along and tell us "this is not really a GUID", and then we'll look silly for using this name.

@ZequanWu
Copy link
Contributor Author

I think we should go back to __buildid, I withdraw my suggestion for a more descriptive name. I'm just concerned someone will come along and tell us "this is not really a GUID", and then we'll look silly for using this name.

Renamed to __buildid. This still points to the 16 bytes hash after "RSDS".

rnk
rnk approved these changes Dec 14, 2023
View reviewed changes
Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good and addresses everyone's concerns other than my descriptive names feedback tangent. Please confirm that the final git commit title is correct, since the PR title looks stale.

@ZequanWu ZequanWu changed the title [LLD][COFF] add __build_guid symbol. [LLD][COFF] add __buildid symbol. Dec 14, 2023
@ZequanWu ZequanWu merged commit 47b4bbf into llvm:main Dec 14, 2023
@ZequanWu ZequanWu deleted the coff-build-id-symbol branch December 14, 2023 22:43
@ZequanWu ZequanWu mentioned this pull request Dec 15, 2023
Merged
ZequanWu added a commit that referenced this pull request Dec 20, 2023
@ZequanWu
[Profile] Dump binary id to raw profiles on Windows. (#75618)
688fa35 
#74652 adds `__buildid` symbol which allows us to dump it at runtime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnk rnk rnk approved these changes

@mstorsjo mstorsjo Awaiting requested review from mstorsjo

@petrhosek petrhosek Awaiting requested review from petrhosek

@MaskRay MaskRay Awaiting requested review from MaskRay

@amykhuang amykhuang Awaiting requested review from amykhuang

Assignees
No one assigned
Labels
lld:COFF lld platform:windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ZequanWu @llvmbot @rnk @petrhosek @mstorsjo