[lld] d838bf2 - [ELF] Allow non-bitcode archive with an empty index

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2022-01-19T10:01:53-08:00
New Revision: d838bf2adc9cba1ef56e446fc2070dda233c1d05

URL: https://github.com/llvm/llvm-project/commit/d838bf2adc9cba1ef56e446fc2070dda233c1d05
DIFF: https://github.com/llvm/llvm-project/commit/d838bf2adc9cba1ef56e446fc2070dda233c1d05.diff

LOG: [ELF] Allow non-bitcode archive with an empty index

When an archive with an empty index contains only bitcode files, it is
handled as a group of lazy (--start-lib) object files. If there is a
non-bitcode file, there will be a diagnostic a la GNU ld.

For some programs, the archive member extraction ratio is high (e.g. for chrome,
79% archive members are extracted according to --print-archive-stats=). Because
symbol interning is cached for ObjFile::parseLazy but not for ArchiveFile,
parsing an archive as a group of --start-lib object files may be faster.

If the linker speculatively creates section representations for archive members,
the archive index will not be used.

If we take the above view, the archive index is essentially useless. If a user
wants a fast build without using --start-lib, they may just build thin archives
without index (`ar rcS --thin`).

Therefore, I suggest that we no longer treat the code as a hack, instead as a
supported feature. I believe we will do this anyway if we add parallel symbol
interning (parallel symbol interning for lazy object files is simpler than that
for archives).

Ecosystem issues:

* parseLazy actually has nearly the same behavior as ArchiveFile::parse, but the symbol order may be different.
* users may get addicted to the behavior and build archives not working with GNU ld and gold. I think it is easy to rebuild archives to be compatible.

Reviewed By: ikudrin

Differential Revision: https://reviews.llvm.org/D117284

Added: 
    

Modified: 
    lld/ELF/Driver.cpp
    lld/test/ELF/archive-no-index.s
    lld/test/ELF/lto/archive-no-index.ll

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 5e7b74e2e4c5..abec642bed1b 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -235,23 +235,22 @@ void LinkerDriver::addFile(StringRef path, bool withLOption) {
     std::unique_ptr<Archive> file =
         CHECK(Archive::create(mbref), path + ": failed to parse archive");
 
-    // If an archive file has no symbol table, it is likely that a user
-    // 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 an archive file has no symbol table, it may be intentional (used as a
+    // group of lazy object files where the symbol table is not useful), or the
+    // user is attempting LTO and using a default ar command that doesn't
+    // understand the LLVM bitcode file. Treat the archive as a group of lazy
+    // object files.
     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) {
-          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(createLazyFile(p.first, path, p.second));
+           getArchiveMembers(mbref)) {
+        auto magic = identify_magic(p.first.getBuffer());
+        if (magic == file_magic::bitcode ||
+            magic == file_magic::elf_relocatable)
+          files.push_back(createLazyFile(p.first, path, p.second));
+        else
+          error(path + ": archive member '" + p.first.getBufferIdentifier() +
+                "' is neither ET_REL nor LLVM bitcode");
+      }
       return;
     }
 

diff  --git a/lld/test/ELF/archive-no-index.s b/lld/test/ELF/archive-no-index.s
index e59274cfa43a..259051773b1b 100644
--- a/lld/test/ELF/archive-no-index.s
+++ b/lld/test/ELF/archive-no-index.s
@@ -5,9 +5,13 @@
 # RUN: rm -f %t.a
 # RUN: llvm-ar crS %t.a %t.archive.o
 
-# RUN: not ld.lld -o /dev/null %t.o %t.a 2>&1 | FileCheck %s
+# RUN: ld.lld %t.o %t.a -o /dev/null 2>&1 | count 0
+
+# RUN: ld.lld -shared %t.archive.o -o %t.so
+# RUN: llvm-ar crS %t.a %t.so
+# RUN: not ld.lld %t.o %t.a --noinhibit-exec -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: {{.*}}.a: archive member '{{.*}}.so' is neither ET_REL nor LLVM bitcode
 
 .globl _start
 _start:
-
-# CHECK: error: {{.*}}.a: archive has no index; run ranlib to add one

diff  --git a/lld/test/ELF/lto/archive-no-index.ll b/lld/test/ELF/lto/archive-no-index.ll
index 79052db38cce..0ede81fec23a 100644
--- a/lld/test/ELF/lto/archive-no-index.ll
+++ b/lld/test/ELF/lto/archive-no-index.ll
@@ -1,6 +1,5 @@
 ; REQUIRES: x86
-; Tests that we accept an archive file without symbol table
-; if all the member files are bitcode files.
+; Tests that we accept an archive file without symbol table.
 
 ; RUN: llvm-as -o %t1.o %s
 ; RUN: llvm-as -o %t2.o %S/Inputs/archive.ll
@@ -25,8 +24,7 @@ define i32 @main() {
 ; 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: {{.*}}.a: archive has no index; run ranlib to add one
+; RUN: not ld.lld -o %t -emain %t1.o %t3.a 2>&1 | FileCheck --check-prefix=ERR2 %s
 
 ; RUN: rm -f %t4.a
 ; RUN: llvm-ar cr %t4.a