[LLD] [COFF] Don't add pseudo relocs for dangling references

[mstorsjo]

When doing GC, we normally won't have dangling references, because such a reference would keep the other section alive, keeping it from being eliminated.

However, references within DWARF sections are ignored for the purposes of GC (because otherwise, they would essentially keep everything alive, defeating the point of the GC), see c579a5b for more context.

Therefore, dangling relocations against discarded symbols are ignored within DWARF sections (see maybeReportRelocationToDiscarded in Chunks.cpp). Consequently, we also shouldn't create any pseudo relocations for these cases, as we run into a null pointer dereference when trying to generate the pseudo relocation info for it.

This fixes the downstream bug
mstorsjo/llvm-mingw#418, fixing crashes on combinations with -ffunction-sections, -fdata-sections, -Wl,--gc-sections and debug info.

[llvmbot]

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

@llvm/pr-subscribers-lld-coff

Author: Martin Storsjö (mstorsjo)

Changes

When doing GC, we normally won't have dangling references, because such a reference would keep the other section alive, keeping it from being eliminated.

However, references within DWARF sections are ignored for the purposes of GC (because otherwise, they would essentially keep everything alive, defeating the point of the GC), see c579a5b for more context.

Therefore, dangling relocations against discarded symbols are ignored within DWARF sections (see maybeReportRelocationToDiscarded in Chunks.cpp). Consequently, we also shouldn't create any pseudo relocations for these cases, as we run into a null pointer dereference when trying to generate the pseudo relocation info for it.

This fixes the downstream bug
mstorsjo/llvm-mingw#418, fixing crashes on combinations with -ffunction-sections, -fdata-sections, -Wl,--gc-sections and debug info.

---

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

2 Files Affected:

• (modified) lld/COFF/Chunks.cpp (+7)
• (added) lld/test/COFF/autoimport-gc.s (+41)

diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 004d71097387a2..0cae52785a0002 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -648,6 +648,13 @@ void SectionChunk::getRuntimePseudoRelocs(
         dyn_cast_or_null<Defined>(file->getSymbol(rel.SymbolTableIndex));
     if (!target || !target->isRuntimePseudoReloc)
       continue;
+    // If the target doesn't have a chunk allocated, it may be a
+    // DefinedImportData symbol which ended up unnecessary after GC.
+    // Normally we wouldn't eliminate section chunks that are referenced, but
+    // references within DWARF sections don't count for keeping section chunks
+    // alive. Thus such dangling references in DWARF sections are expected.
+    if (!target->getChunk())
+      continue;
     int sizeInBits =
         getRuntimePseudoRelocSize(rel.Type, file->ctx.config.machine);
     if (sizeInBits == 0) {
diff --git a/lld/test/COFF/autoimport-gc.s b/lld/test/COFF/autoimport-gc.s
new file mode 100644
index 00000000000000..fef6c02eba82f9
--- /dev/null
+++ b/lld/test/COFF/autoimport-gc.s
@@ -0,0 +1,41 @@
+# REQUIRES: x86
+# RUN: split-file %s %t.dir
+
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/lib.s -filetype=obj -o %t.dir/lib.obj
+# RUN: lld-link -out:%t.dir/lib.dll -dll -entry:DllMainCRTStartup %t.dir/lib.obj -lldmingw -implib:%t.dir/lib.lib
+
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/main.s -filetype=obj -o %t.dir/main.obj
+# RUN: lld-link -lldmingw -out:%t.dir/main.exe -entry:main %t.dir/main.obj %t.dir/lib.lib -opt:ref -debug:dwarf
+
+#--- main.s
+    .global main
+    .section .text$main,"xr",one_only,main
+main:
+    ret
+
+    .global other
+    .section .text$other,"xr",one_only,other
+other:
+    movq .refptr.variable(%rip), %rax
+    movl (%rax), %eax
+    ret
+
+    .section .rdata$.refptr.variable,"dr",discard,.refptr.variable
+    .global .refptr.variable
+.refptr.variable:
+    .quad   variable
+
+    .section .debug_info
+    .long 1
+    .quad variable
+    .long 2
+
+#--- lib.s
+    .global variable
+    .global DllMainCRTStartup
+    .text
+DllMainCRTStartup:
+    ret
+    .data
+variable:
+    .long 42

[cjacek]

LGTM

For the record, we also discussed possibility of entirely skipping pseudo relocs for DWARF sections: DWARF sections shouldn't need runtime pseudo relocs in general (the section should be usable by interpreting the file by tools like debuggers from the disk, etc.) and they are usually not mapped (so pseudo relocs have no effect anyway). This would be a more invasive change and @mstorsjo plans to backport it, so a less invasive change seems fine for now.

[mstorsjo]

/cherry-pick 9c970d5

[llvmbot]

/pull-request #88759