From c6cd62352cc15110c7a7389721560046a6635cde Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 13 Aug 2019 09:12:52 +0000 Subject: [PATCH] [ELF] Simplify handling of exportDynamic and isPreemptible In Writer::includeInDynSym(), exportDynamic is used by a Defined with protected or default visibility, to record whether it is required to be exported into .dynsym. It is set when any of the following conditions hold: 1) There is an interposable symbol from a DSO (Undefined or SharedSymbol with default visibility) 2) If -shared or --export-dynamic is specified, any symbol in an object file/bitcode sets this property, unless suppressed by canBeOmittedFromSymbolTable(). 3) --dynamic-list when producing an executable 4) protected symbol from a DSO preempted by copy relocation/canonical PLT when --ignore-{data,function}-address-equality is specified 5) ifunc is exported when -z ifunc-noplt is specified Bullet points 4) and 5) are irrelevant in this patch. Bullet 3) does not play well with 1) and 2). When -shared is specified, exportDynamic of most symbols is true. This makes it incapable to record --dynamic-list marked symbols. We thus have obscure: if (!config->shared) b->exportDynamic = true; else if (b->includeInDynsym()) b->isPreemptible = true; This patch adds another bit `Symbol::inDynamicList` to record 3). We can thus simplify handleDynamicList() by unifying the DSO and executable cases. It also allows us to simplify isPreemptible - now the field is only used in finalizeSections() and later stages. Reviewed By: peter.smith Differential Revision: https://reviews.llvm.org/D66091 llvm-svn: 368659 --- lld/ELF/SymbolTable.cpp | 9 +++------ lld/ELF/Symbols.cpp | 2 +- lld/ELF/Symbols.h | 27 +++++++++++++++++++-------- lld/ELF/Writer.cpp | 14 ++++++-------- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index 143620e..e978e0b 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -77,6 +77,7 @@ Symbol *SymbolTable::insert(StringRef name) { sym->visibility = STV_DEFAULT; sym->isUsedInRegularObj = false; sym->exportDynamic = false; + sym->inDynamicList = false; sym->canInline = true; sym->scriptDefined = false; sym->partition = 1; @@ -162,12 +163,8 @@ void SymbolTable::handleDynamicList() { else syms = findByVersion(ver); - for (Symbol *b : syms) { - if (!config->shared) - b->exportDynamic = true; - else if (b->includeInDynsym()) - b->isPreemptible = true; - } + for (Symbol *sym : syms) + sym->inDynamicList = true; } } diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 84bc158..3845da7 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -295,7 +295,7 @@ bool Symbol::includeInDynsym() const { if (isUndefWeak() && config->pie && sharedFiles.empty()) return false; - return isUndefined() || isShared() || exportDynamic; + return isUndefined() || isShared() || exportDynamic || inDynamicList; } // Print out a log message for --trace-symbol. diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h index 92f9751..5012710 100644 --- a/lld/ELF/Symbols.h +++ b/lld/ELF/Symbols.h @@ -116,12 +116,21 @@ public: // are unreferenced except by other bitcode objects. unsigned isUsedInRegularObj : 1; - // If this flag is true and the symbol has protected or default visibility, it - // will appear in .dynsym. This flag is set by interposable DSO symbols in - // executables, by most symbols in DSOs and executables built with - // --export-dynamic, and by dynamic lists. + // Used by a Defined symbol with protected or default visibility, to record + // whether it is required to be exported into .dynsym. This is set when any of + // the following conditions hold: + // + // - If there is an interposable symbol from a DSO. + // - If -shared or --export-dynamic is specified, any symbol in an object + // file/bitcode sets this property, unless suppressed by LTO + // canBeOmittedFromSymbolTable(). unsigned exportDynamic : 1; + // True if the symbol is in the --dynamic-list file. A Defined symbol with + // protected or default visibility with this property is required to be + // exported into .dynsym. + unsigned inDynamicList : 1; + // False if LTO shouldn't inline whatever this symbol points to. If a symbol // is overwritten after LTO, LTO shouldn't inline the symbol because it // doesn't know the final contents of the symbol. @@ -233,10 +242,11 @@ protected: : file(file), nameData(name.data), nameSize(name.size), binding(binding), type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3), isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind), - exportDynamic(isExportDynamic(k, visibility)), canInline(false), - referenced(false), traced(false), needsPltAddr(false), isInIplt(false), - gotInIgot(false), isPreemptible(false), used(!config->gcSections), - needsTocRestore(false), scriptDefined(false) {} + exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false), + canInline(false), referenced(false), traced(false), needsPltAddr(false), + isInIplt(false), gotInIgot(false), isPreemptible(false), + used(!config->gcSections), needsTocRestore(false), + scriptDefined(false) {} public: // True the symbol should point to its PLT entry. @@ -531,6 +541,7 @@ void Symbol::replace(const Symbol &newSym) { visibility = old.visibility; isUsedInRegularObj = old.isUsedInRegularObj; exportDynamic = old.exportDynamic; + inDynamicList = old.inDynamicList; canInline = old.canInline; referenced = old.referenced; traced = old.traced; diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index a359b93..9c7044a 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -1653,13 +1653,13 @@ static bool computeIsPreemptible(const Symbol &b) { if (!b.isDefined()) return true; - // If we have a dynamic list it specifies which local symbols are preemptible. - if (config->hasDynamicList) - return false; - if (!config->shared) return false; + // If the dynamic list is present, it specifies preemptable symbols in a DSO. + if (config->hasDynamicList) + return b.inDynamicList; + // -Bsymbolic means that definitions are not preempted. if (config->bsymbolic || (config->bsymbolicFunctions && b.isFunc())) return false; @@ -1728,10 +1728,8 @@ template void Writer::finalizeSections() { for (Partition &part : partitions) finalizeSynthetic(part.ehFrame); - symtab->forEachSymbol([](Symbol *s) { - if (!s->isPreemptible) - s->isPreemptible = computeIsPreemptible(*s); - }); + symtab->forEachSymbol( + [](Symbol *s) { s->isPreemptible = computeIsPreemptible(*s); }); // Scan relocations. This must be done after every symbol is declared so that // we can correctly decide if a dynamic relocation is needed. -- 2.7.4