[lld] c9b241e - Revert "[lld-macho] Avoid force-loading the same archive twice"

[Nico Weber via llvm-commits]

Author: Nico Weber
Date: 2021-06-18T20:25:27-04:00
New Revision: c9b241efd68c5a0f1f67e9250960ade454f3bc11

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

LOG: Revert "[lld-macho] Avoid force-loading the same archive twice"

This reverts commit 24706cd73cd150543753a2e169c68a2c68da46a1.
Test seems to fail flakily. See comments on https://reviews.llvm.org/D104353
for a hypothesis for why.

Added: 
    

Modified: 
    lld/MachO/Driver.cpp
    lld/test/MachO/archive.s
    lld/test/MachO/force-load.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 726ca3e00fb1..c81160c51cf7 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -235,8 +235,6 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
   return v;
 }
 
-static DenseMap<StringRef, ArchiveFile *> loadedArchives;
-
 static InputFile *addFile(StringRef path, bool forceLoadArchive,
                           bool isExplicit = true,
                           bool isBundleLoader = false) {
@@ -249,13 +247,6 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
   file_magic magic = identify_magic(mbref.getBuffer());
   switch (magic) {
   case file_magic::archive: {
-    // Avoid loading archives twice. If the archives are being force-loaded,
-    // loading them twice would create duplicate symbol errors. In the
-    // non-force-loading case, this is just a minor performance optimization.
-    ArchiveFile *&cachedFile = loadedArchives[path];
-    if (cachedFile)
-      return cachedFile;
-
     std::unique_ptr<object::Archive> file = CHECK(
         object::Archive::create(mbref), path + ": failed to parse archive");
 
@@ -295,7 +286,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
       }
     }
 
-    newFile = cachedFile = make<ArchiveFile>(std::move(file));
+    newFile = make<ArchiveFile>(std::move(file));
     break;
   }
   case file_magic::macho_object:

diff  --git a/lld/test/MachO/archive.s b/lld/test/MachO/archive.s
index 3a946c0ab935..2ac2d302b88d 100644
--- a/lld/test/MachO/archive.s
+++ b/lld/test/MachO/archive.s
@@ -32,11 +32,6 @@
 # ALL-LOAD: T _main
 # ALL-LOAD: T _unused
 
-## Multiple archives defining the same symbols aren't an issue, due to lazy
-## loading
-# RUN: cp %t/test.a %t/test2.a
-# RUN: %lld %t/test.a %t/test2.a %t/main.o -o /dev/null
-
 #--- 2.s
 .globl _boo
 _boo:

diff  --git a/lld/test/MachO/force-load.s b/lld/test/MachO/force-load.s
index 79f5e9328ef0..1cedae3173bc 100644
--- a/lld/test/MachO/force-load.s
+++ b/lld/test/MachO/force-load.s
@@ -1,48 +1,23 @@
 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/archive-foo.s -o %t/archive-foo.o
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/archive-baz.s -o %t/archive-baz.o
 # RUN: llvm-ar rcs %t/foo.a %t/archive-foo.o
-# RUN: llvm-ar rcs %t/baz.a %t/archive-baz.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
 
-# RUN: %lld -lSystem -force_load %t/foo.a %t/foo.o %t/test.o -o %t/test-force-load-first
+# RUN: %lld -force_load %t/foo.a %t/foo.o %t/test.o -o %t/test-force-load-first
 # FORCE-LOAD-FIRST:  __TEXT,archive _foo
 # RUN: llvm-objdump --syms %t/test-force-load-first | FileCheck %s --check-prefix=FORCE-LOAD-FIRST
 
-# RUN: %lld %t/foo.o -lSystem -force_load %t/foo.a %t/test.o -o %t/test-force-load-second
+# RUN: %lld %t/foo.o -force_load %t/foo.a %t/test.o -o %t/test-force-load-second
 # RUN: llvm-objdump --syms %t/test-force-load-second | FileCheck %s --check-prefix=FORCE-LOAD-SECOND
 # FORCE-LOAD-SECOND: __TEXT,obj _foo
 
-## Force-loading the same path twice is fine
-# RUN: %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load %t/foo.a %t/test.o -o /dev/null
-
-## Note that we do not call realpath() before dedup'ing the force-load
-## arguments, so this is an error.
-# RUN: cd %t; not %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load foo.a \
-# RUN:   %t/test.o -o /dev/null 2>&1
-
-# DUP: error: duplicate symbol: _bar
-
-## Force-loading two 
diff erent paths w/o conflicting symbols is fine
-# RUN: %lld -lSystem -force_load %t/foo.a -force_load %t/baz.a %t/test.o -o %t/test-two-force-loads
-# RUN: llvm-objdump --syms %t/test-two-force-loads | FileCheck %s --check-prefix=TWICE
-# TWICE-DAG: __TEXT,archive _foo
-# TWICE-DAG: __TEXT,archive _bar
-# TWICE-DAG: __TEXT,archive _baz
-
 #--- archive-foo.s
 .section __TEXT,archive
-.globl _foo, _bar
+.globl _foo
 .weak_definition _foo
 _foo:
-_bar:
-
-#--- archive-baz.s
-.section __TEXT,archive
-.globl _baz
-_baz:
 
 #--- foo.s
 .section __TEXT,obj