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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: ikudrin, peter.smith.
Herald added subscribers: ormris, arphaman, steven_wu, hiraditya, arichardson, emaste.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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 we take this view, the archive index is essentially useless. To overcome
build system limitation not supporting --start-lib, a user may just build thin
archives without index (`ar rS --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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117284

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


Index: lld/test/ELF/lto/archive-no-index.ll
===================================================================
--- lld/test/ELF/lto/archive-no-index.ll
+++ 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 @@
 ; 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
Index: lld/test/ELF/archive-no-index.s
===================================================================
--- lld/test/ELF/archive-no-index.s
+++ lld/test/ELF/archive-no-index.s
@@ -5,9 +5,7 @@
 # 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
 
 .globl _start
 _start:
-
-# CHECK: error: {{.*}}.a: archive has no index; run ranlib to add one
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -236,20 +236,12 @@
     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));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117284.399911.patch
Type: text/x-patch
Size: 2905 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220114/51c6ada6/attachment.bin>