[lld] 9bf1c6d - Revert "[lld-macho] Implement -load_hidden"

Daniel Bertalan via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 12:14:01 PDT 2022


Author: Daniel Bertalan
Date: 2022-07-25T21:11:19+02:00
New Revision: 9bf1c6dabf3c9a25d74a451ef35ff2d06c156433

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

LOG: Revert "[lld-macho] Implement -load_hidden"

This reverts commit 4c79e1a3f4eb790f40239833ae237e828ce07386.

Broke this bot: https://lab.llvm.org/buildbot/#builders/57/builds/20319

Added: 
    

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/DriverUtils.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputFiles.h
    lld/MachO/Options.td
    lld/test/MachO/reroot-path.s

Removed: 
    lld/test/MachO/load-hidden.s


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index c6445438dfeb3..454708fad4ef2 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -266,8 +266,7 @@ static DenseMap<StringRef, ArchiveFileInfo> loadedArchives;
 
 static InputFile *addFile(StringRef path, LoadType loadType,
                           bool isLazy = false, bool isExplicit = true,
-                          bool isBundleLoader = false,
-                          bool isForceHidden = false) {
+                          bool isBundleLoader = false) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return nullptr;
@@ -294,7 +293,7 @@ static InputFile *addFile(StringRef path, LoadType loadType,
 
       if (!archive->isEmpty() && !archive->hasSymbolTable())
         error(path + ": archive has no index; run ranlib to add one");
-      file = make<ArchiveFile>(std::move(archive), isForceHidden);
+      file = make<ArchiveFile>(std::move(archive));
     } else {
       file = entry->second.file;
       // Command-line loads take precedence. If file is previously loaded via
@@ -1036,11 +1035,6 @@ static void createFiles(const InputArgList &args) {
     case OPT_force_load:
       addFile(rerootPath(arg->getValue()), LoadType::CommandLineForce);
       break;
-    case OPT_load_hidden:
-      addFile(rerootPath(arg->getValue()), LoadType::CommandLine,
-              /*isLazy=*/false, /*isExplicit=*/true, /*isBundleLoader=*/false,
-              /*isForceHidden=*/true);
-      break;
     case OPT_l:
     case OPT_needed_l:
     case OPT_reexport_l:

diff  --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index d8e474d15cfd3..b52d5e851c62d 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -150,7 +150,6 @@ std::string macho::createResponseFile(const InputArgList &args) {
       break;
     case OPT_force_load:
     case OPT_weak_library:
-    case OPT_load_hidden:
       os << arg->getSpelling() << " "
          << quote(rewriteInputPath(arg->getValue())) << "\n";
       break;

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 0a5a37d9f78a9..e3bf553e53347 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -768,7 +768,7 @@ void ObjFile::parseRelocations(ArrayRef<SectionHeader> sectionHeaders,
 template <class NList>
 static macho::Symbol *createDefined(const NList &sym, StringRef name,
                                     InputSection *isec, uint64_t value,
-                                    uint64_t size, bool forceHidden) {
+                                    uint64_t size) {
   // Symbol scope is determined by sym.n_type & (N_EXT | N_PEXT):
   // N_EXT: Global symbols. These go in the symbol table during the link,
   //        and also in the export table of the output so that the dynamic
@@ -787,10 +787,7 @@ static macho::Symbol *createDefined(const NList &sym, StringRef name,
       (sym.n_desc & (N_WEAK_DEF | N_WEAK_REF)) == (N_WEAK_DEF | N_WEAK_REF);
 
   if (sym.n_type & N_EXT) {
-    // -load_hidden makes us treat global symbols as linkage unit scoped.
-    // Duplicates are reported but the symbol does not go in the export trie.
-    bool isPrivateExtern = sym.n_type & N_PEXT || forceHidden;
-
+    bool isPrivateExtern = sym.n_type & N_PEXT;
     // lld's behavior for merging symbols is slightly 
diff erent from ld64:
     // ld64 picks the winning symbol based on several criteria (see
     // pickBetweenRegularAtoms() in ld64's SymbolTable.cpp), while lld
@@ -847,12 +844,11 @@ static macho::Symbol *createDefined(const NList &sym, StringRef name,
 // InputSection. They cannot be weak.
 template <class NList>
 static macho::Symbol *createAbsolute(const NList &sym, InputFile *file,
-                                     StringRef name, bool forceHidden) {
+                                     StringRef name) {
   if (sym.n_type & N_EXT) {
-    bool isPrivateExtern = sym.n_type & N_PEXT || forceHidden;
     return symtab->addDefined(
         name, file, nullptr, sym.n_value, /*size=*/0,
-        /*isWeakDef=*/false, isPrivateExtern, sym.n_desc & N_ARM_THUMB_DEF,
+        /*isWeakDef=*/false, sym.n_type & N_PEXT, sym.n_desc & N_ARM_THUMB_DEF,
         /*isReferencedDynamically=*/false, sym.n_desc & N_NO_DEAD_STRIP,
         /*isWeakDefCanBeHidden=*/false);
   }
@@ -868,16 +864,15 @@ template <class NList>
 macho::Symbol *ObjFile::parseNonSectionSymbol(const NList &sym,
                                               StringRef name) {
   uint8_t type = sym.n_type & N_TYPE;
-  bool isPrivateExtern = sym.n_type & N_PEXT || forceHidden;
   switch (type) {
   case N_UNDF:
     return sym.n_value == 0
                ? symtab->addUndefined(name, this, sym.n_desc & N_WEAK_REF)
                : symtab->addCommon(name, this, sym.n_value,
                                    1 << GET_COMM_ALIGN(sym.n_desc),
-                                   isPrivateExtern);
+                                   sym.n_type & N_PEXT);
   case N_ABS:
-    return createAbsolute(sym, this, name, forceHidden);
+    return createAbsolute(sym, this, name);
   case N_PBUD:
   case N_INDR:
     error("TODO: support symbols of type " + std::to_string(type));
@@ -949,8 +944,7 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
                 " at misaligned offset");
           continue;
         }
-        symbols[symIndex] =
-            createDefined(sym, name, isec, 0, isec->getSize(), forceHidden);
+        symbols[symIndex] = createDefined(sym, name, isec, 0, isec->getSize());
       }
       continue;
     }
@@ -985,8 +979,8 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
       //   4. If we have a literal section (e.g. __cstring and __literal4).
       if (!subsectionsViaSymbols || symbolOffset == 0 ||
           sym.n_desc & N_ALT_ENTRY || !isa<ConcatInputSection>(isec)) {
-        symbols[symIndex] = createDefined(sym, name, isec, symbolOffset,
-                                          symbolSize, forceHidden);
+        symbols[symIndex] =
+            createDefined(sym, name, isec, symbolOffset, symbolSize);
         continue;
       }
       auto *concatIsec = cast<ConcatInputSection>(isec);
@@ -1004,8 +998,8 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
 
       // By construction, the symbol will be at offset zero in the new
       // subsection.
-      symbols[symIndex] = createDefined(sym, name, nextIsec, /*value=*/0,
-                                        symbolSize, forceHidden);
+      symbols[symIndex] =
+          createDefined(sym, name, nextIsec, /*value=*/0, symbolSize);
       // TODO: ld64 appears to preserve the original alignment as well as each
       // subsection's offset from the last aligned address. We should consider
       // emulating that behavior.
@@ -1042,8 +1036,8 @@ OpaqueFile::OpaqueFile(MemoryBufferRef mb, StringRef segName,
 }
 
 ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName,
-                 bool lazy, bool forceHidden)
-    : InputFile(ObjKind, mb, lazy), modTime(modTime), forceHidden(forceHidden) {
+                 bool lazy)
+    : InputFile(ObjKind, mb, lazy), modTime(modTime) {
   this->archiveName = std::string(archiveName);
   if (lazy) {
     if (target->wordSize == 8)
@@ -2067,27 +2061,26 @@ void DylibFile::checkAppExtensionSafety(bool dylibIsAppExtensionSafe) const {
     warn("using '-application_extension' with unsafe dylib: " + toString(this));
 }
 
-ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f, bool forceHidden)
-    : InputFile(ArchiveKind, f->getMemoryBufferRef()), file(std::move(f)),
-      forceHidden(forceHidden) {}
+ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f)
+    : InputFile(ArchiveKind, f->getMemoryBufferRef()), file(std::move(f)) {}
 
 void ArchiveFile::addLazySymbols() {
   for (const object::Archive::Symbol &sym : file->symbols())
     symtab->addLazyArchive(sym.getName(), this, sym);
 }
 
-static Expected<InputFile *>
-loadArchiveMember(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName,
-                  uint64_t offsetInArchive, bool forceHidden) {
+static Expected<InputFile *> loadArchiveMember(MemoryBufferRef mb,
+                                               uint32_t modTime,
+                                               StringRef archiveName,
+                                               uint64_t offsetInArchive) {
   if (config->zeroModTime)
     modTime = 0;
 
   switch (identify_magic(mb.getBuffer())) {
   case file_magic::macho_object:
-    return make<ObjFile>(mb, modTime, archiveName, /*lazy=*/false, forceHidden);
+    return make<ObjFile>(mb, modTime, archiveName);
   case file_magic::bitcode:
-    return make<BitcodeFile>(mb, archiveName, offsetInArchive, /*lazy=*/false,
-                             forceHidden);
+    return make<BitcodeFile>(mb, archiveName, offsetInArchive);
   default:
     return createStringError(inconvertibleErrorCode(),
                              mb.getBufferIdentifier() +
@@ -2111,8 +2104,8 @@ Error ArchiveFile::fetch(const object::Archive::Child &c, StringRef reason) {
   if (!modTime)
     return modTime.takeError();
 
-  Expected<InputFile *> file = loadArchiveMember(
-      *mb, toTimeT(*modTime), getName(), c.getChildOffset(), forceHidden);
+  Expected<InputFile *> file =
+      loadArchiveMember(*mb, toTimeT(*modTime), getName(), c.getChildOffset());
 
   if (!file)
     return file.takeError();
@@ -2175,8 +2168,8 @@ static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
 }
 
 BitcodeFile::BitcodeFile(MemoryBufferRef mb, StringRef archiveName,
-                         uint64_t offsetInArchive, bool lazy, bool forceHidden)
-    : InputFile(BitcodeKind, mb, lazy), forceHidden(forceHidden) {
+                         uint64_t offsetInArchive, bool lazy)
+    : InputFile(BitcodeKind, mb, lazy) {
   this->archiveName = std::string(archiveName);
   std::string path = mb.getBufferIdentifier().str();
   // ThinLTO assumes that all MemoryBufferRefs given to it have a unique

diff  --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 3359161d83341..5deb05272a6b1 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -156,7 +156,7 @@ struct FDE {
 class ObjFile final : public InputFile {
 public:
   ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName,
-          bool lazy = false, bool forceHidden = false);
+          bool lazy = false);
   ArrayRef<llvm::MachO::data_in_code_entry> getDataInCode() const;
   template <class LP> void parse();
 
@@ -171,7 +171,6 @@ class ObjFile final : public InputFile {
   std::unique_ptr<lld::DWARFCache> dwarfCache;
   Section *addrSigSection = nullptr;
   const uint32_t modTime;
-  bool forceHidden;
   std::vector<ConcatInputSection *> debugSections;
   std::vector<CallGraphEntry> callGraph;
   llvm::DenseMap<ConcatInputSection *, FDE> fdes;
@@ -260,8 +259,7 @@ class DylibFile final : public InputFile {
 // .a file
 class ArchiveFile final : public InputFile {
 public:
-  explicit ArchiveFile(std::unique_ptr<llvm::object::Archive> &&file,
-                       bool forceHidden);
+  explicit ArchiveFile(std::unique_ptr<llvm::object::Archive> &&file);
   void addLazySymbols();
   void fetch(const llvm::object::Archive::Symbol &);
   // LLD normally doesn't use Error for error-handling, but the underlying
@@ -275,15 +273,12 @@ class ArchiveFile final : public InputFile {
   // Keep track of children fetched from the archive by tracking
   // which address offsets have been fetched already.
   llvm::DenseSet<uint64_t> seen;
-  // Load all symbols with hidden visibility (-load_hidden).
-  bool forceHidden;
 };
 
 class BitcodeFile final : public InputFile {
 public:
   explicit BitcodeFile(MemoryBufferRef mb, StringRef archiveName,
-                       uint64_t offsetInArchive, bool lazy = false,
-                       bool forceHidden = false);
+                       uint64_t offsetInArchive, bool lazy = false);
   static bool classof(const InputFile *f) { return f->kind() == BitcodeKind; }
   void parse();
 
@@ -291,8 +286,6 @@ class BitcodeFile final : public InputFile {
 
 private:
   void parseLazy();
-
-  bool forceHidden;
 };
 
 extern llvm::SetVector<InputFile *> inputFiles;

diff  --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 38b79f820f805..b3d74a83f5822 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -240,10 +240,6 @@ def force_load : Separate<["-"], "force_load">,
 def force_load_swift_libs : Flag<["-"], "force_load_swift_libs">,
     HelpText<"Apply -force_load to libraries listed in LC_LINKER_OPTIONS whose names start with 'swift'">,
     Group<grp_libs>;
-def load_hidden : Separate<["-"], "load_hidden">,
-    MetaVarName<"<path>">,
-    HelpText<"Load all symbols from static library with hidden visibility">,
-    Group<grp_libs>;
 
 def grp_content : OptionGroup<"content">, HelpText<"ADDITIONAL CONTENT">;
 

diff  --git a/lld/test/MachO/load-hidden.s b/lld/test/MachO/load-hidden.s
deleted file mode 100644
index 003409c278fa4..0000000000000
--- a/lld/test/MachO/load-hidden.s
+++ /dev/null
@@ -1,72 +0,0 @@
-# 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/obj.s -o %t/obj.o
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/obj-bar.s -o %t/obj-bar.o
-
-# RUN: %lld -dylib -lSystem %t/obj.o -load_hidden %t/foo.a -o %t/test.dylib
-# RUN: llvm-nm %t/test.dylib | FileCheck %s
-# CHECK-DAG: t _foo
-# CHECK-DAG: d _bar
-
-## If an archive has already been loaded without -load_hidden earlier in the command line,
-## -load_hidden does not have an effect.
-# RUN: %lld -dylib -lSystem %t/obj.o %t/foo.a -load_hidden %t/foo.a -o %t/test-regular-then-hidden.dylib
-# RUN: llvm-nm %t/test-regular-then-hidden.dylib | FileCheck %s --check-prefix=REGULAR-THEN-HIDDEN
-# REGULAR-THEN-HIDDEN-DAG: T _foo
-# REGULAR-THEN-HIDDEN-DAG: D _bar
-
-## If -load_hidden comes first, the symbols will have hidden visibility.
-# RUN: %lld -dylib -lSystem %t/obj.o -load_hidden %t/foo.a %t/foo.a -o %t/test-hidden-then-regular.dylib
-# RUN: llvm-nm %t/test-hidden-then-regular.dylib | FileCheck %s --check-prefix=HIDDEN-THEN-REGULAR
-# HIDDEN-THEN-REGULAR-DAG: t _foo
-# HIDDEN-THEN-REGULAR-DAG: d _bar
-
-## If both -load_hidden and -force_load are specified, the earlier one will have an effect.
-# RUN: %lld -dylib -lSystem %t/obj.o %t/foo.a -force_load %t/baz.a -load_hidden %t/baz.a -o %t/test-force-then-hidden.dylib
-# RUN: llvm-nm %t/test-force-then-hidden.dylib | FileCheck %s --check-prefix=FORCE-THEN-HIDDEN
-# FORCE-THEN-HIDDEN: T _baz
-# RUN: %lld -dylib -lSystem %t/obj.o %t/foo.a -load_hidden %t/baz.a -force_load %t/baz.a -o %t/test-hidden-then-force.dylib
-# RUN: llvm-nm %t/test-hidden-then-force.dylib | FileCheck %s --check-prefix=HIDDEN-THEN-FORCE
-# HIDDEN-THEN-FORCE-NOT: _baz
-
-## -load_hidden does not cause the library to be loaded eagerly.
-# RUN: %lld -dylib -lSystem %t/obj.o -load_hidden %t/foo.a -load_hidden %t/baz.a -o %t/test-lazy.dylib
-# RUN: llvm-nm %t/test-lazy.dylib | FileCheck %s --check-prefix=LAZY
-# LAZY-NOT: _baz
-
-## Specifying the same library twice is fine.
-# RUN: %lld -dylib -lSystem %t/obj.o -load_hidden %t/foo.a -load_hidden %t/foo.a -o %t/test-twice.dylib
-# RUN: llvm-nm %t/test-twice.dylib | FileCheck %s --check-prefix=TWICE
-# TWICE-DAG: t _foo
-# TWICE-DAG: d _bar
-
-## -load_hidden causes the symbols to have "private external" visibility, so duplicate definitions
-## are not allowed.
-# RUN: not %lld -dylib -lSystem %t/obj.o %t/obj-bar.o -load_hidden %t/foo.a 2>&1 | FileCheck %s --check-prefix=DUP
-# DUP: error: duplicate symbol: _bar
-
-#--- archive-foo.s
-.globl _foo
-_foo:
-
-.data
-.globl _bar
-_bar:
-
-#--- archive-baz.s
-.globl _baz
-_baz:
-
-#--- obj-bar.s
-.data
-.globl _bar
-_bar:
-
-#--- obj.s
-.globl _test
-_test:
-    call _foo

diff  --git a/lld/test/MachO/reroot-path.s b/lld/test/MachO/reroot-path.s
index 43ff21d1b915f..b99d99176f991 100644
--- a/lld/test/MachO/reroot-path.s
+++ b/lld/test/MachO/reroot-path.s
@@ -39,10 +39,6 @@
 # RUN: tar xf %t/repro4.tar -C %t
 # RUN: cd %t/repro4; %no-arg-lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
 
-# RUN: %lld --reproduce %t/repro7.tar -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: tar xf %t/repro7.tar -C %t
-# RUN: cd %t/repro7; %no-arg-lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
-
 # RUN: echo "%t/libfoo.dylib" > %t/filelist
 # RUN: echo "%t/libbar.dylib" >> %t/filelist
 # RUN: %lld --reproduce %t/repro5.tar -lSystem -syslibroot %t -filelist %t/filelist %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"


        


More information about the llvm-commits mailing list