From c28f4e3f047affece3ebb475bbc13de6096710c3 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 15 Sep 2022 11:12:32 -0400 Subject: [PATCH] Revert "[lld-macho] Add support for N_INDR symbols" This reverts commit 5b8da10b87f7009c06215449e4a9c61dab91697a. Breaks tests, see https://reviews.llvm.org/D133825 --- lld/MachO/Driver.cpp | 35 +------ lld/MachO/InputFiles.cpp | 27 ++---- lld/MachO/InputFiles.h | 4 +- lld/MachO/SymbolTable.cpp | 8 +- lld/MachO/SymbolTable.h | 3 +- lld/MachO/Symbols.h | 22 ----- lld/docs/MachO/ld64-vs-lld.rst | 9 -- lld/test/MachO/alias-symbols.s | 136 --------------------------- lld/test/MachO/{alias-option.s => aliases.s} | 0 9 files changed, 18 insertions(+), 226 deletions(-) delete mode 100644 lld/test/MachO/alias-symbols.s rename lld/test/MachO/{alias-option.s => aliases.s} (100%) diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 6bec108..533273e 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1200,40 +1200,13 @@ static void createAliases() { for (const auto &pair : config->aliasedSymbols) { if (const auto &sym = symtab->find(pair.first)) { if (const auto &defined = dyn_cast(sym)) { - symtab->aliasDefined(defined, pair.second, defined->getFile()); - } else { - error("TODO: support aliasing to symbols of kind " + - Twine(sym->kind())); + symtab->aliasDefined(defined, pair.second); + continue; } - } else { - warn("undefined base symbol '" + pair.first + "' for alias '" + - pair.second + "'\n"); } - } - for (const InputFile *file : inputFiles) { - if (auto *objFile = dyn_cast(file)) { - for (const AliasSymbol *alias : objFile->aliases) { - if (const auto &aliased = symtab->find(alias->getAliasedName())) { - if (const auto &defined = dyn_cast(aliased)) { - symtab->aliasDefined(defined, alias->getName(), alias->getFile(), - alias->privateExtern); - } else { - // Common, dylib, and undefined symbols are all valid alias - // referents (undefineds can become valid Defined symbols later on - // in the link.) - error("TODO: support aliasing to symbols of kind " + - Twine(aliased->kind())); - } - } else { - // This shouldn't happen since MC generates undefined symbols to - // represent the alias referents. Thus we fatal() instead of just - // warning here. - fatal("unable to find alias referent " + alias->getAliasedName() + - " for " + alias->getName()); - } - } - } + warn("undefined base symbol '" + pair.first + "' for alias '" + + pair.second + "'\n"); } } diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp index 588b87a..f68985f 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -872,8 +872,7 @@ static macho::Symbol *createAbsolute(const NList &sym, InputFile *file, template macho::Symbol *ObjFile::parseNonSectionSymbol(const NList &sym, - const char *strtab) { - StringRef name = StringRef(strtab + sym.n_strx); + StringRef name) { uint8_t type = sym.n_type & N_TYPE; bool isPrivateExtern = sym.n_type & N_PEXT || forceHidden; switch (type) { @@ -885,20 +884,9 @@ macho::Symbol *ObjFile::parseNonSectionSymbol(const NList &sym, isPrivateExtern); case N_ABS: return createAbsolute(sym, this, name, forceHidden); - case N_INDR: { - // Not much point in making local aliases -- relocs in the current file can - // just refer to the actual symbol itself. ld64 ignores these symbols too. - if (!(sym.n_type & N_EXT)) - return nullptr; - StringRef aliasedName = StringRef(strtab + sym.n_value); - // isPrivateExtern is the only symbol flag that has an impact on the final - // aliased symbol. - auto alias = make(this, name, aliasedName, isPrivateExtern); - aliases.push_back(alias); - return alias; - } case N_PBUD: - error("TODO: support symbols of type N_PBUD"); + case N_INDR: + error("TODO: support symbols of type " + std::to_string(type)); return nullptr; case N_SECT: llvm_unreachable( @@ -939,7 +927,7 @@ void ObjFile::parseSymbols(ArrayRef sectionHeaders, } else if (isUndef(sym)) { undefineds.push_back(i); } else { - symbols[i] = parseNonSectionSymbol(sym, strtab); + symbols[i] = parseNonSectionSymbol(sym, StringRef(strtab + sym.n_strx)); } } @@ -1038,8 +1026,11 @@ void ObjFile::parseSymbols(ArrayRef sectionHeaders, // symbol resolution behavior. In addition, a set of interconnected symbols // will all be resolved to the same file, instead of being resolved to // different files. - for (unsigned i : undefineds) - symbols[i] = parseNonSectionSymbol(nList[i], strtab); + for (unsigned i : undefineds) { + const NList &sym = nList[i]; + StringRef name = strtab + sym.n_strx; + symbols[i] = parseNonSectionSymbol(sym, name); + } } OpaqueFile::OpaqueFile(MemoryBufferRef mb, StringRef segName, diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h index 1b454f9..8917292 100644 --- a/lld/MachO/InputFiles.h +++ b/lld/MachO/InputFiles.h @@ -44,7 +44,6 @@ struct PlatformInfo; class ConcatInputSection; class Symbol; class Defined; -class AliasSymbol; struct Reloc; enum class RefState : uint8_t; @@ -177,7 +176,6 @@ public: std::vector callGraph; llvm::DenseMap fdes; std::vector optimizationHints; - std::vector aliases; private: llvm::once_flag initDwarf; @@ -188,7 +186,7 @@ private: ArrayRef nList, const char *strtab, bool subsectionsViaSymbols); template - Symbol *parseNonSectionSymbol(const NList &sym, const char *strtab); + Symbol *parseNonSectionSymbol(const NList &sym, StringRef name); template void parseRelocations(ArrayRef sectionHeaders, const SectionHeader &, Section &); diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp index cc0f1d5..de4af3c 100644 --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -115,11 +115,9 @@ Defined *SymbolTable::addDefined(StringRef name, InputFile *file, return defined; } -Defined *SymbolTable::aliasDefined(Defined *src, StringRef target, - InputFile *newFile, bool makePrivateExtern) { - bool isPrivateExtern = makePrivateExtern || src->privateExtern; - return addDefined(target, newFile, src->isec, src->value, src->size, - src->isWeakDef(), isPrivateExtern, src->thumb, +Defined *SymbolTable::aliasDefined(Defined *src, StringRef target) { + return addDefined(target, src->getFile(), src->isec, src->value, src->size, + src->isWeakDef(), src->privateExtern, src->thumb, src->referencedDynamically, src->noDeadStrip, src->weakDefCanBeHidden); } diff --git a/lld/MachO/SymbolTable.h b/lld/MachO/SymbolTable.h index d90245c..a393f9c 100644 --- a/lld/MachO/SymbolTable.h +++ b/lld/MachO/SymbolTable.h @@ -42,8 +42,7 @@ public: bool isReferencedDynamically, bool noDeadStrip, bool isWeakDefCanBeHidden); - Defined *aliasDefined(Defined *src, StringRef target, InputFile *newFile, - bool makePrivateExtern = false); + Defined *aliasDefined(Defined *src, StringRef target); Symbol *addUndefined(StringRef name, InputFile *, bool isWeakRef); diff --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h index 6773170..9d3b56a 100644 --- a/lld/MachO/Symbols.h +++ b/lld/MachO/Symbols.h @@ -39,7 +39,6 @@ public: DylibKind, LazyArchiveKind, LazyObjectKind, - AliasKind, }; virtual ~Symbol() {} @@ -322,26 +321,6 @@ public: static bool classof(const Symbol *s) { return s->kind() == LazyObjectKind; } }; -// Represents N_INDR symbols. Note that if we are given valid, linkable inputs, -// then all AliasSymbol instances will be converted into one of the other Symbol -// types after `createAliases()` runs. -class AliasSymbol final : public Symbol { -public: - AliasSymbol(InputFile *file, StringRef name, StringRef aliasedName, - bool isPrivateExtern) - : Symbol(AliasKind, name, file), privateExtern(isPrivateExtern), - aliasedName(aliasedName) {} - - StringRef getAliasedName() const { return aliasedName; } - - static bool classof(const Symbol *s) { return s->kind() == AliasKind; } - - const bool privateExtern; - -private: - StringRef aliasedName; -}; - union SymbolUnion { alignas(Defined) char a[sizeof(Defined)]; alignas(Undefined) char b[sizeof(Undefined)]; @@ -349,7 +328,6 @@ union SymbolUnion { alignas(DylibSymbol) char d[sizeof(DylibSymbol)]; alignas(LazyArchive) char e[sizeof(LazyArchive)]; alignas(LazyObject) char f[sizeof(LazyObject)]; - alignas(AliasSymbol) char g[sizeof(AliasSymbol)]; }; template diff --git a/lld/docs/MachO/ld64-vs-lld.rst b/lld/docs/MachO/ld64-vs-lld.rst index f571e6a..4f3ae69 100644 --- a/lld/docs/MachO/ld64-vs-lld.rst +++ b/lld/docs/MachO/ld64-vs-lld.rst @@ -37,12 +37,3 @@ archives. symbol" error. - LLD: Duplicate symbols, regardless of which archives they are from, will raise errors. - -Aliases -======= -ld64 treats all aliases as strong extern definitions. Having two aliases of the -same name, or an alias plus a regular extern symbol of the same name, both -result in duplicate symbol errors. LLD does not check for duplicate aliases; -instead we perform alias resolution first, and only then do we check for -duplicate symbols. In particular, we will not report a duplicate symbol error if -the aliased symbols turn out to be weak definitions, but ld64 will. diff --git a/lld/test/MachO/alias-symbols.s b/lld/test/MachO/alias-symbols.s deleted file mode 100644 index ef22c87..0000000 --- a/lld/test/MachO/alias-symbols.s +++ /dev/null @@ -1,136 +0,0 @@ -# REQUIRES: x86 -# RUN: rm -rf %t; split-file %s %t -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/aliases.s -o %t/aliases.o -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/definitions.s -o %t/definitions.o -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-extern-alias-to-weak.s -o %t/weak-extern-alias-to-weak.o -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-extern-alias-to-strong.s -o %t/weak-extern-alias-to-strong.o - -# RUN: %lld -lSystem %t/aliases.o %t/definitions.o -o %t/out -# RUN: llvm-objdump --macho --syms %t/out | FileCheck %s - -## local aliases should be dropped entirely. --implicit-check-not doesn't seem -## to work well with -DAG matches, so we check for _local_alias' absence in a -## separate step. -# RUN: llvm-objdump --macho --syms %t/out | FileCheck /dev/null --implicit-check-not _local_alias - -# CHECK-DAG: [[#%.16x,STRONG:]] g F __TEXT,__text _strong -# CHECK-DAG: [[#%.16x,WEAK_1:]] w F __TEXT,__text _weak_1 -# CHECK-DAG: [[#%.16x,PEXT:]] l F __TEXT,__text _pext -# CHECK-DAG: [[#%.16x,DEAD:]] g F __TEXT,__text _dead -# CHECK-DAG: [[#STRONG]] l F __TEXT,__text _pext_alias -# CHECK-DAG: [[#PEXT]] l F __TEXT,__text _alias_to_pext -# CHECK-DAG: [[#STRONG]] g F __TEXT,__text _extern_alias_to_strong -# CHECK-DAG: [[#WEAK_1]] w F __TEXT,__text _weak_extern_alias_to_weak -# CHECK-DAG: [[#DEAD]] g F __TEXT,__text _no_dead_strip_alias -# CHECK-DAG: [[#STRONG]] g F __TEXT,__text _weak_extern_alias_to_strong - -# RUN: %lld -lSystem -dead_strip %t/aliases.o %t/definitions.o -o %t/dead-stripped -# RUN: llvm-objdump --macho --syms %t/dead-stripped | FileCheck %s --check-prefix=STRIPPED - -# STRIPPED: SYMBOL TABLE: -# STRIPPED-NEXT: g F __TEXT,__text _main -# STRIPPED-NEXT: g F __TEXT,__text __mh_execute_header -# STRIPPED-NEXT: *UND* dyld_stub_binder -# STRIPPED-EMPTY: - -# RUN: not %lld -lSystem %t/aliases.o %t/definitions.o \ -# RUN: %t/weak-extern-alias-to-strong.o -o /dev/null 2>&1 - -## Verify that we preserve the file names of the aliases, rather than using the -## filename of the aliased symbols. -# DUP: error: duplicate symbol: _weak_extern_alias_to_weak -# DUP-NEXT: >>> defined in {{.*}}aliases.o -# DUP-NEXT: >>> defined in {{.*}}weak-extern-alias-to-weak.o - -## The following cases are actually all dup symbol errors under ld64. Alias -## symbols are treated like strong extern symbols by ld64 even if the symbol they alias -## is actually weak. LLD OTOH does not check for dup symbols until after -## resolving the aliases; this makes for a simpler implementation. -## The following test cases are meant to elucidate what LLD's behavior is, but -## we should feel free to change it in the future should it be helpful for the -## implementation. - -# RUN: %lld -lSystem %t/aliases.o %t/definitions.o \ -# RUN: %t/weak-extern-alias-to-weak.o -o %t/alias-clash-1 -# RUN: llvm-objdump --macho --syms %t/alias-clash-1 | FileCheck %s --check-prefix WEAK-1 - -# RUN: %lld -lSystem %t/weak-extern-alias-to-weak.o %t/aliases.o \ -# RUN: %t/definitions.o -o %t/alias-clash-2 -# RUN: llvm-objdump --macho --syms %t/alias-clash-2 | FileCheck %s --check-prefix WEAK-2 - -# RUN: %lld -lSystem %t/aliases.o %t/definitions.o \ -# RUN: -alias _weak_2 _weak_extern_alias_to_weak -o %t/opt-vs-symbol -# RUN: llvm-objdump --macho --syms %t/opt-vs-symbol | FileCheck %s --check-prefix WEAK-2 - -# RUN: %lld -lSystem -alias _weak_2 _weak_extern_alias_to_weak %t/aliases.o \ -# RUN: %t/definitions.o -o %t/opt-vs-symbol -# RUN: llvm-objdump --macho --syms %t/opt-vs-symbol | FileCheck %s --check-prefix WEAK-2 - -# WEAK-1-DAG: [[#%.16x,WEAK_1:]] w F __TEXT,__text _weak_1 -# WEAK-1-DAG: [[#WEAK_1]] w F __TEXT,__text _weak_extern_alias_to_weak - -# WEAK-2-DAG: [[#%.16x,WEAK_2:]] w F __TEXT,__text _weak_2 -# WEAK-2-DAG: [[#WEAK_2]] w F __TEXT,__text _weak_extern_alias_to_weak - -#--- aliases.s -.globl _extern_alias_to_strong, _weak_extern_alias_to_weak -.weak_definition _weak_extern_alias_to_weak - -## Private extern aliases result in local symbols in the output (i.e. it is as -## if the aliased symbol is also private extern.) -.private_extern _pext_alias - -## This test case demonstrates that it doesn't matter whether the alias itself -## is strong or weak. Rather, what matters is whether the aliased symbol is -## strong or weak. -.globl _weak_extern_alias_to_strong -.weak_definition _weak_extern_alias_to_strong - -## no_dead_strip doesn't retain the aliased symbol if it is dead -.globl _no_dead_strip_alias -.no_dead_strip _no_dead_strip_alias - -.globl _alias_to_pext -_alias_to_pext = _pext - -_extern_alias_to_strong = _strong -_weak_extern_alias_to_weak = _weak_1 -_weak_extern_alias_to_strong = _strong - -_pext_alias = _strong -_local_alias = _strong -_no_dead_strip_alias = _dead - -.subsections_via_symbols - -#--- weak-extern-alias-to-weak.s -.globl _weak_extern_alias_to_weak -.weak_definition _weak_extern_alias_to_weak -_weak_extern_alias_to_weak = _weak_2 - -#--- weak-extern-alias-to-strong.s -.globl _weak_extern_alias_to_strong -.weak_definition _weak_extern_alias_to_strong -_weak_extern_alias_to_strong = _strong - -#--- definitions.s -.globl _strong, _weak_1, _weak_2, _dead -.private_extern _pext -.weak_definition _weak_1 -.weak_definition _weak_2 - -_strong: - .space 1 -_weak_1: - .space 1 -_weak_2: - .space 1 -_dead: - .space 1 -_pext: - .space 1 - -.globl _main -_main: - -.subsections_via_symbols diff --git a/lld/test/MachO/alias-option.s b/lld/test/MachO/aliases.s similarity index 100% rename from lld/test/MachO/alias-option.s rename to lld/test/MachO/aliases.s -- 2.7.4