[LLD] [COFF] Error out if the runtime pseudo relocation function is missing

[mstorsjo]

When then linker creates runtime pseudo relocations, it places them in a list with the assumption that the runtime will fix these relocations later, when the image gets loaded. If the relevant runtime function doesn't seem to be present in the linked image, error out.

Normally when linking the mingw-w64 runtime libraries, this function always is available. However, if linking without including the mingw-w64 CRT startup files, and the image needs runtime pseudo relocations, make it clear that this won't work as expected at runtime.

With ld.bfd, this situation is a hard error too; ld.bfd adds an undefined reference to this symbol if runtime pseudo relocations are needed.

A later alternative would be to actually try to pull in the symbol (if seen in a static library, but not included yet). This would allow decoupling the function from the main CRT startup code (making it optional, only running if the linker actually produced runtime pseudo relocations). Doing that would require restructuring the code (gathering pseudo relocations earlier, in order to be able to continue linking in more object files if the initial set did require pseudo relocations) though.

Also, ld.bfd doesn't currently successfully pull in more object files to satisfy the dependency on _pei386_runtime_relocator, so with that in mind, there's not much extra value in making LLD do it currently either.

This fixes one issue brought up in #84424.

[llvmbot]

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

@llvm/pr-subscribers-lld-coff

Author: Martin Storsjö (mstorsjo)

Changes

When then linker creates runtime pseudo relocations, it places them in a list with the assumption that the runtime will fix these relocations later, when the image gets loaded. If the relevant runtime function doesn't seem to be present in the linked image, print a warning.

Normally when linking the mingw-w64 runtime libraries, this function always is available. However, if linking without including the mingw-w64 CRT startup files, and the image needs runtime pseudo relocations, try to make the user aware of the situation.

This just prints a warning to alert the user about this situation. Alternatively, we could also make this situation a hard error.

With ld.bfd, this situation is a hard error; ld.bfd adds an undefined reference to this symbol if runtime pseudo relocations are needed.

Yet another alternative would be to actually try to pull in the symbol (if seen in a static library, but not included yet). This would allow decoupling the function from the main CRT startup code (making it optional, only running if the linker actually produced runtime pseudo relocations). Doing that would require restructuring the code (gathering pseudo relocations earlier, in order to be able to continue linking in more object files if the initial set did require pseudo relocations) though.

Also, ld.bfd doesn't currently successfully pull in more object files to satisfy the dependency on _pei386_runtime_relocator, so with that in mind, there's not much extra value in making LLD do it currently either.

This fixes one issue brought up in
#84424.

---

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

2 Files Affected:

• (modified) lld/COFF/Writer.cpp (+9-1)
• (added) lld/test/COFF/autoimport-warn-func.s (+36)

diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 9c20bbb83d86d1..f2194616b93a63 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -2072,8 +2072,16 @@ void Writer::createRuntimePseudoRelocs() {
     return;
   }
 
-  if (!rels.empty())
+  if (!rels.empty()) {
     log("Writing " + Twine(rels.size()) + " runtime pseudo relocations");
+    const char *symbolName = "_pei386_runtime_relocator";
+    Symbol *relocator = ctx.symtab.findUnderscore(symbolName);
+    if (!relocator)
+      warn("output image has pseudo relocations, but function " +
+           Twine(symbolName) +
+           " missing; the relocations might not get fixed at runtime");
+  }
+
   PseudoRelocTableChunk *table = make<PseudoRelocTableChunk>(rels);
   rdataSec->addChunk(table);
   EmptyChunk *endOfList = make<EmptyChunk>();
diff --git a/lld/test/COFF/autoimport-warn-func.s b/lld/test/COFF/autoimport-warn-func.s
new file mode 100644
index 00000000000000..94e1238376b710
--- /dev/null
+++ b/lld/test/COFF/autoimport-warn-func.s
@@ -0,0 +1,36 @@
+# REQUIRES: x86
+# RUN: split-file %s %t.dir
+
+# RUN: llvm-dlltool -m i386:x86-64 -d %t.dir/lib.def -D lib.dll -l %t.dir/lib.lib
+
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/main.s -filetype=obj -o %t.dir/main.obj
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/func.s -filetype=obj -o %t.dir/func.obj
+# RUN: lld-link -lldmingw -out:%t.dir/main.exe -entry:main %t.dir/main.obj %t.dir/lib.lib 2>&1 | FileCheck %s --check-prefix=WARN
+
+# RUN: lld-link -lldmingw -out:%t.dir/main.exe -entry:main %t.dir/main.obj %t.dir/func.obj %t.dir/lib.lib 2>&1 | FileCheck %s --check-prefix=NOWARN --allow-empty
+
+# WARN: warning: output image has pseudo relocations, but function _pei386_runtime_relocator missing; the relocations might not get fixed at runtime
+
+# NOWARN-NOT: warning
+
+#--- main.s
+    .global main
+    .text
+main:
+    ret
+
+    .data
+    .long 1
+    .quad variable
+    .long 2
+
+#--- func.s
+    .global _pei386_runtime_relocator
+    .text
+_pei386_runtime_relocator:
+    ret
+
+#--- lib.def
+EXPORTS
+variable DATA
+

[mstorsjo]

CC @mati865 @jeremyd2019

What do others think - should we make this a hard error, or just a friendly warning? (This issue was brought up in #84424 (comment).)

Ideally I'd like to make the linker try to pull in this function once we realize we need runtime pseudo relocs; I have a WIP patch for that since a long time, but I feel that it's a bit messy, and as long as ld.bfd isn't fixed to work in the same way, we can't really make use of it.

[Keno]

Alternatively, we could also make this situation a hard error.

I think a hard error is probably better since non-application of the pseudo relocation ends up exposing the wrong symbol address. I think the situation would be different if the pseudo relocations where still being applied if the library was ultimately opened by an application that is linked against the mingw runtime, but that's not the case - the only question is whether the startup files are linked into the .dll, which we can detect here.

[jeremyd2019]

Alternatively, we could also make this situation a hard error.

I think a hard error is probably better ...

I agree. Since I have been dealing with it lately, I am a little concerned if this will be an issue for rust. I imagine not, because it does usually work with ld.bfd, and this is a hard error there.

[mstorsjo]

Thanks for the input! I changed this to be a hard error now - adjusting both commit message and the tone of the output message slightly.

[github-actions]

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

[mati865]

Normally it's nice to have a warning for one release to make migration easier but in this case looks like the outputs are always more or less broken, so the hard error makes sense.

[cjacek]

LGTM