[ELF] Error on archive with missing index

[sbc100]

Summary

This matches the wasm lld and GNU ld behavior.

The ELF linker has special handling for bitcode archives but if that
doesn't kick in we probably want to error out rather than silently
ignore the library.

[ruiu]

I think this is a backward-incompatible change. Previously, you could create an archive containing both bitcode files and ELF files, and this patch disallow that combination, which seems a bit too risky change to me.

[sbc100]

This change only effects archives without an index. If you previously created an archive with both bitcode *and* ELF files *and* you didn't create and index, the previous behaviour was to completely ignore the archive. This seems bad to me. The new behaviour is to give an error. We could downgrade it to a warning but both GNU ld and wasm-ld give a hard error in this case.

Are you saying that ignoring these hybrid archives is a behavior that you want to keep?

[sbc100]

The previous behaviour was also to ignore archives that contain only ELF files but no index.

[ruiu]

In D63781#1559504, @sbc100 wrote:

The previous behaviour was also to ignore archives that contain only ELF files but no index.

Is that true? If an archive has no index, each individual member is handled as if they were surrounded by --start-lib/--end-lib, so they wouldn't be ignored, I think.

[sbc100]

I'm pretty certain yes. The old code only loads archives one object at a time if all objects are bitcode. I didn't change that part.

[sbc100]

The comment also says exactly that does't it?

[sbc100]

Can you take a closer look? I'm pretty sure this change is correct. At the very least an improvement, and not a regression.

[MaskRay]

lld/test/ELF/archive-no-index.s

8

/dev/null if you don't need the output file?

[ruiu]

LGTM

Yeah, looks like I misunderstood the existing code. Thank you for pointing that out.

[sbc100]

lld/test/ELF/archive-no-index.s

8

Done. I assume that magically works on windows too somehow.

[aykevl]

Just a note after the fact: this change broke linking in TinyGo. It used to manually build an archive (without index) with object files from compiler-rt and that worked just fine in LLVM 8 - it did read symbols from those archives even without a symbol table. In LLVM 9, ld.lld refuses to link those libraries. We'll work around it by manually building a symbol table for lld.

For details: https://github.com/tinygo-org/tinygo/issues/595

[sbc100]

I'm pretty sure this change went from silently ignoring the archive to erroring out. So I think your use case would have been broken even before this change. See the return statement on line 227. Perhaps there are no needed symbols in your archive? In that case maybe this would be a regression but that seems unlikely.

lld/ELF/Driver.cpp

case file_magic::archive: {
     // is attempting LTO and using a default ar command that doesn't
     // understand the LLVM bitcode file. It is a pretty common error, so
     // we'll handle it as if it had a symbol table.
     if (!File->isEmpty() && !File->hasSymbolTable()) {
       // Check if all members are bitcode files. If not, ignore, which is the
       // default action without the LTO hack described above.
       for (const std::pair<MemoryBufferRef, uint64_t> &P :
            getArchiveMembers(MBRef))
-        if (identify_magic(P.first.getBuffer()) != file_magic::bitcode)
+        if (identify_magic(P.first.getBuffer()) != file_magic::bitcode) {
+          error(Path + ": archive has no index; run ranlib to add one");
           return;
+        }
 
       for (const std::pair<MemoryBufferRef, uint64_t> &P :
            getArchiveMembers(MBRef))
         Files.push_back(make<LazyObjFile>(P.first, Path, P.second));
       return;
     }
 
     // Handle the regular case.

lld/test/ELF/archive-no-index.s

+# REQUIRES: x86
+# Tests error on archive file without a symbol table
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t.o %s
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t.archive.o %S/Inputs/archive.s
+# RUN: rm -f %t.a
+# RUN: llvm-ar crS %t.a %t.archive.o
+
+# RUN: not ld.lld -o out.wasm %t.o %t.a 2>&1 | FileCheck %s
+
+.globl _start
+_start:
+
+# CHECK: error: {{.*}}.a: archive has no index; run ranlib to add one

lld/test/ELF/lto/archive-no-index.ll

define i32 @main() {
   call void @f()
   ret i32 0
 }
 
 ; RUN: echo 'f:' | llvm-mc -triple=x86_64-pc-linux -filetype=obj - -o %t3.o
 ; RUN: rm -f %t3.a
 ; RUN: llvm-ar crS %t3.a %t3.o
 ; RUN: not ld.lld -o /dev/null -emain %t1.o %t3.a 2>&1 | FileCheck -check-prefix=ERR1 %s
-; ERR1: error: undefined symbol: f
+; ERR1: error: {{.*}}.a: archive has no index; run ranlib to add one
 
 ; RUN: rm -f %t4.a
 ; RUN: llvm-ar cr %t4.a
 ; RUN: not ld.lld -o /dev/null -emain %t1.o %t4.a 2>&1 | FileCheck -check-prefix=ERR2 %s
 ; ERR2: error: undefined symbol: f