Skip to content

[LLD][COFF] Allow symbols with empty chunks to have no associated output section #149523

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Jul 18, 2025

If a chunk is empty and there are no other non-empty chunks in the same section, removeEmptySections() will remove the entire section. In this case, use a section index of 0, as the MSVC linker does, instead of asserting.

…put section

If a chunk is empty and there are no other non-empty chunks in the same section,
removeEmptySections() will remove the entire section. In this case, use a section
index of 0, as the MSVC linker does, instead of asserting.
@cjacek
Copy link
Contributor Author

cjacek commented Jul 18, 2025

This issue was reported to cause problems when building Wine:
mstorsjo/llvm-mingw@ae03577#commitcomment-162336778

In that case, it involved the recently introduced __data_start__ symbol, but the problem isn't specific to that symbol. The test included in this MR demonstrates that the issue can occur more generally.

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-platform-windows

Author: Jacek Caban (cjacek)

Changes

If a chunk is empty and there are no other non-empty chunks in the same section, removeEmptySections() will remove the entire section. In this case, use a section index of 0, as the MSVC linker does, instead of asserting.


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

2 Files Affected:

  • (modified) lld/COFF/PDB.cpp (+6-3)
  • (added) lld/test/COFF/pdb-empty-sec.s (+19)
diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp
index a54ea403ba2ec..94eeae2797971 100644
--- a/lld/COFF/PDB.cpp
+++ b/lld/COFF/PDB.cpp
@@ -1135,9 +1135,12 @@ static pdb::BulkPublic createPublic(COFFLinkerContext &ctx, Defined *def) {
   pub.setFlags(flags);
 
   OutputSection *os = ctx.getOutputSection(def->getChunk());
-  assert(os && "all publics should be in final image");
-  pub.Offset = def->getRVA() - os->getRVA();
-  pub.Segment = os->sectionIndex;
+  assert((os || !def->getChunk()->getSize()) &&
+         "all publics should be in final image");
+  if (os) {
+    pub.Offset = def->getRVA() - os->getRVA();
+    pub.Segment = os->sectionIndex;
+  }
   return pub;
 }
 
diff --git a/lld/test/COFF/pdb-empty-sec.s b/lld/test/COFF/pdb-empty-sec.s
new file mode 100644
index 0000000000000..0d61447b76651
--- /dev/null
+++ b/lld/test/COFF/pdb-empty-sec.s
@@ -0,0 +1,19 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %s -o %t.obj
+// RUN: lld-link -dll -noentry -debug %t.obj -out:%t.dll
+// RUN: llvm-pdbutil dump -publics %t.pdb | FileCheck %s
+
+// CHECK:       Records
+// CHECK-NEXT:       0 | S_PUB32 [size = 20] `func`
+// CHECK-NEXT:           flags = none, addr = 0001:0000
+// CHECK-NEXT:      20 | S_PUB32 [size = 20] `sym`
+// CHECK-NEXT:           flags = none, addr = 0000:0000
+
+        .globl sym
+        .data
+sym:
+        .text
+        .globl func
+func:
+        ret

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

If a chunk is empty and there are no other non-empty chunks in the same section, removeEmptySections() will remove the entire section. In this case, use a section index of 0, as the MSVC linker does, instead of asserting.


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

2 Files Affected:

  • (modified) lld/COFF/PDB.cpp (+6-3)
  • (added) lld/test/COFF/pdb-empty-sec.s (+19)
diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp
index a54ea403ba2ec..94eeae2797971 100644
--- a/lld/COFF/PDB.cpp
+++ b/lld/COFF/PDB.cpp
@@ -1135,9 +1135,12 @@ static pdb::BulkPublic createPublic(COFFLinkerContext &ctx, Defined *def) {
   pub.setFlags(flags);
 
   OutputSection *os = ctx.getOutputSection(def->getChunk());
-  assert(os && "all publics should be in final image");
-  pub.Offset = def->getRVA() - os->getRVA();
-  pub.Segment = os->sectionIndex;
+  assert((os || !def->getChunk()->getSize()) &&
+         "all publics should be in final image");
+  if (os) {
+    pub.Offset = def->getRVA() - os->getRVA();
+    pub.Segment = os->sectionIndex;
+  }
   return pub;
 }
 
diff --git a/lld/test/COFF/pdb-empty-sec.s b/lld/test/COFF/pdb-empty-sec.s
new file mode 100644
index 0000000000000..0d61447b76651
--- /dev/null
+++ b/lld/test/COFF/pdb-empty-sec.s
@@ -0,0 +1,19 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %s -o %t.obj
+// RUN: lld-link -dll -noentry -debug %t.obj -out:%t.dll
+// RUN: llvm-pdbutil dump -publics %t.pdb | FileCheck %s
+
+// CHECK:       Records
+// CHECK-NEXT:       0 | S_PUB32 [size = 20] `func`
+// CHECK-NEXT:           flags = none, addr = 0001:0000
+// CHECK-NEXT:      20 | S_PUB32 [size = 20] `sym`
+// CHECK-NEXT:           flags = none, addr = 0000:0000
+
+        .globl sym
+        .data
+sym:
+        .text
+        .globl func
+func:
+        ret

cjacek referenced this pull request in mstorsjo/llvm-mingw Jul 18, 2025
Relevant changes include:
- Lots of work for supporting ARM64EC and ARM64X
- LLD now produces more compact string tables in executables (only
  relevant when including debug symbols)
- llvm-rc now supports multiplication/division expressions, like GNU
  windres
- Improved support for armv7 and aarch64 in libunwind (for Itanium
  ABI "forced unwinding", which I'm not aware of being used in
  practice anywhere)
- Made the libunwind, libcxxabi, libcxx and compiler-rt tests pass on
  armv7 and aarch64 mingw
- Made the llvm/clang tests pass on aarch64 mingw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants