llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.06k stars 11.59k forks source link

[lld] Many "failed to compute relocation" warnings on duplicate symbol (coff/i386) #60623

Open orgads opened 1 year ago

orgads commented 1 year ago

Run the following in MinGW32 environment, and observe the warnings.

They appear for each file, on some cases it floods stderr...

$ echo 'int main() { return 0; }' | tee a.c > b.c
$ gcc -c a.c b.c
$ gcc -fuse-ld=lld a.o b.o
warning: failed to compute relocation: IMAGE_REL_I386_REL32, Invalid data was encountered while parsing the file
warning: failed to compute relocation: IMAGE_REL_I386_REL32, Invalid data was encountered while parsing the file
ld.lld: error: duplicate symbol: _main
>>> defined at a.o
>>> defined at b.o
collect2.exe: error: ld returned 1 exit status

This doesn't happen on 64-bit target.

CC @mstorsjo

mstorsjo commented 1 year ago

Reproduced, thanks! It's also reproducible with llvm-mingw on Ubuntu (22.04), by compiling the object files with GCC (with /usr/bin/i686-w64-mingw32-gcc -c a.c b.c) and linking with llvm-mingw (i686-w64-mingw32-clang a.o b.o). It doesn't happen with object files built by Clang.

(Side note, I had troubles making the cross-gcc work with -fuse-ld=lld.)

llvmbot commented 1 year ago

@llvm/issue-subscribers-lld-coff

orgads commented 1 year ago

(Side note, I had troubles making the cross-gcc work with -fuse-ld=lld.)

You need to create a symlink like:

ln -s /usr/bin/ld.lld /usr/bin/i686-w64-mingw32-ld.lld

I added this to mxe here: https://github.com/mxe/mxe/pull/2750

mstorsjo commented 1 year ago

(Side note, I had troubles making the cross-gcc work with -fuse-ld=lld.)

You need to create a symlink like:

ln -s /usr/bin/ld.lld /usr/bin/i686-w64-mingw32-ld.lld

I added this to mxe here: mxe/mxe#2750

Oh, I see, thanks for the clue!

mstorsjo commented 1 year ago

I've looked at the issue now and understand what's going on. CC @MaskRay

When LLD wants to print a warning/error, we try to resolve the source location that corresponds to the error. To do that, we try to parse the DWARF debug info in the object files. The DWARF is meant to be interpreted after linking, but we're reading it out of unlinked object files, so we have to do some manual interpretation of the relocations in the object files. This doesn't support all object file relocations (and in general, we can't figure out really anything that a linker would be able to either), but it supports the common ones that are found in DWARF typically. For COFF/i386, this is handled here: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/llvm/lib/Object/RelocationResolver.cpp#L575-L594

In the case of object files built with a recent GCC, for i386, it also includes .eh_frame (unwind info) sections where the PC addresses are encoded with IMAGE_REL_I386_REL32. The RelocationResolver referenced above doesn't support this relocation in this context - but it doesn't make any functional difference since we don't use anything from .eh_frame for the purposes in LLD.

(Context: LLVM produces one monolithic .eh_frame section for the whole object file, with absolute address references using IMAGE_REL_I386_DIR32, while GCC now splits up .eh_frame into smaller sections, with relative address references using IMAGE_REL_I386_REL32. That allows the linker to omit unwind info for cases with e.g. -ffunction-sections, for functions that are left out. I wonder if LLD/COFF handles this kind of .eh_frame from GCC correctly with --gc-sections involved. LLD/ELF has got custom code for splitting such a monolithic .eh_frame though.)

I think that this would be a suitable implementation for the relocation resolver:

diff --git a/llvm/lib/Object/RelocationResolver.cpp b/llvm/lib/Object/RelocationResolver.cpp
index 5a0edaf457b2..d62f8768f7e7 100644
--- a/llvm/lib/Object/RelocationResolver.cpp
+++ b/llvm/lib/Object/RelocationResolver.cpp
@@ -576,6 +576,7 @@ static bool supportsCOFFX86(uint64_t Type) {
   switch (Type) {
   case COFF::IMAGE_REL_I386_SECREL:
   case COFF::IMAGE_REL_I386_DIR32:
+  case COFF::IMAGE_REL_I386_REL32:
     return true;
   default:
     return false;
@@ -588,6 +589,8 @@ static uint64_t resolveCOFFX86(uint64_t Type, uint64_t Offset, uint64_t S,
   case COFF::IMAGE_REL_I386_SECREL:
   case COFF::IMAGE_REL_I386_DIR32:
     return (S + LocData) & 0xFFFFFFFF;
+  case COFF::IMAGE_REL_I386_REL32:
+    return (Offset + 4 + S + LocData) & 0xFFFFFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }

This does silence the warning from LLD - but it's a bit hard to go ahead with it, unless we'd have a more functional test of it, that actually relies on this returning the right thing.