From 9bf1c6dabf3c9a25d74a451ef35ff2d06c156433 Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Mon, 25 Jul 2022 21:11:19 +0200 Subject: [PATCH] Revert "[lld-macho] Implement -load_hidden" This reverts commit 4c79e1a3f4eb790f40239833ae237e828ce07386. Broke this bot: https://lab.llvm.org/buildbot/#builders/57/builds/20319 --- lld/MachO/Driver.cpp | 10 ++---- lld/MachO/DriverUtils.cpp | 1 - lld/MachO/InputFiles.cpp | 57 +++++++++++++++-------------------- lld/MachO/InputFiles.h | 13 ++------ lld/MachO/Options.td | 4 --- lld/test/MachO/load-hidden.s | 72 -------------------------------------------- lld/test/MachO/reroot-path.s | 4 --- 7 files changed, 30 insertions(+), 131 deletions(-) delete mode 100644 lld/test/MachO/load-hidden.s diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index c644543..454708f 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -266,8 +266,7 @@ static DenseMap loadedArchives; static InputFile *addFile(StringRef path, LoadType loadType, bool isLazy = false, bool isExplicit = true, - bool isBundleLoader = false, - bool isForceHidden = false) { + bool isBundleLoader = false) { Optional 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(std::move(archive), isForceHidden); + file = make(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 d8e474d..b52d5e8 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 0a5a37d..e3bf553 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -768,7 +768,7 @@ void ObjFile::parseRelocations(ArrayRef sectionHeaders, template 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 different 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 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 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 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 sectionHeaders, // 4. If we have a literal section (e.g. __cstring and __literal4). if (!subsectionsViaSymbols || symbolOffset == 0 || sym.n_desc & N_ALT_ENTRY || !isa(isec)) { - symbols[symIndex] = createDefined(sym, name, isec, symbolOffset, - symbolSize, forceHidden); + symbols[symIndex] = + createDefined(sym, name, isec, symbolOffset, symbolSize); continue; } auto *concatIsec = cast(isec); @@ -1004,8 +998,8 @@ void ObjFile::parseSymbols(ArrayRef 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 &&f, bool forceHidden) - : InputFile(ArchiveKind, f->getMemoryBufferRef()), file(std::move(f)), - forceHidden(forceHidden) {} +ArchiveFile::ArchiveFile(std::unique_ptr &&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 -loadArchiveMember(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName, - uint64_t offsetInArchive, bool forceHidden) { +static Expected 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(mb, modTime, archiveName, /*lazy=*/false, forceHidden); + return make(mb, modTime, archiveName); case file_magic::bitcode: - return make(mb, archiveName, offsetInArchive, /*lazy=*/false, - forceHidden); + return make(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 file = loadArchiveMember( - *mb, toTimeT(*modTime), getName(), c.getChildOffset(), forceHidden); + Expected 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 3359161..5deb052 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 getDataInCode() const; template void parse(); @@ -171,7 +171,6 @@ public: std::unique_ptr dwarfCache; Section *addrSigSection = nullptr; const uint32_t modTime; - bool forceHidden; std::vector debugSections; std::vector callGraph; llvm::DenseMap fdes; @@ -260,8 +259,7 @@ private: // .a file class ArchiveFile final : public InputFile { public: - explicit ArchiveFile(std::unique_ptr &&file, - bool forceHidden); + explicit ArchiveFile(std::unique_ptr &&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 @@ private: // Keep track of children fetched from the archive by tracking // which address offsets have been fetched already. llvm::DenseSet 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 @@ public: private: void parseLazy(); - - bool forceHidden; }; extern llvm::SetVector inputFiles; diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td index 38b79f8..b3d74a8 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; -def load_hidden : Separate<["-"], "load_hidden">, - MetaVarName<"">, - HelpText<"Load all symbols from static library with hidden visibility">, - Group; 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 003409c..0000000 --- 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 43ff21d..b99d991 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" -- 2.7.4