-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
release/18.x: [LLD] [COFF] Don't add pseudo relocs for dangling references (#88487) #88759
Conversation
@cjacek What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-lld-coff @llvm/pr-subscribers-lld Author: None (llvmbot) ChangesBackport 9c970d5 Requested by: @mstorsjo Full diff: https://github.com/llvm/llvm-project/pull/88759.diff 2 Files Affected:
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 39f4575031be54..e2074932bc466e 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -652,6 +652,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
|
) 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. (cherry picked from commit 9c970d5)
Hi @mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. |
Backport 9c970d5
Requested by: @mstorsjo