[lld] r367869 - [ELF] Consistently prioritize non-* wildcards overs "*" in version scripts
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 5 07:31:39 PDT 2019
Author: maskray
Date: Mon Aug 5 07:31:39 2019
New Revision: 367869
URL: http://llvm.org/viewvc/llvm-project?rev=367869&view=rev
Log:
[ELF] Consistently prioritize non-* wildcards overs "*" in version scripts
We prioritize non-* wildcards overs VER_NDX_LOCAL/VER_NDX_GLOBAL "*".
This patch generalizes the rule to "*" of other versions and thus fixes PR40176.
I don't feel strongly about this GNU linkers' behavior but the
generalization simplifies code.
Delete `config->defaultSymbolVersion` which was used to special case
VER_NDX_LOCAL/VER_NDX_GLOBAL "*".
In `SymbolTable::scanVersionScript`, custom versions are handled the same
way as VER_NDX_LOCAL/VER_NDX_GLOBAL. So merge
`config->versionScript{Locals,Globals}` into `config->versionDefinitions`.
Overall this seems to simplify the code.
In `SymbolTable::assign{Exact,Wildcard}Versions`,
`sym->verdefIndex == config->defaultSymbolVersion` is changed to
`verdefIndex == UINT32_C(-1)`.
This allows us to give duplicate assignment diagnostics for
`{ global: foo; };` `V1 { global: foo; };`
In test/linkerscript/version-script.s:
vs_index of an undefined symbol changes from 0 to 1. This doesn't matter (arguably 1 is better because the binding is STB_GLOBAL) because vs_index of an undefined symbol is ignored.
Reviewed By: ruiu
Differential Revision: https://reviews.llvm.org/D65716
Added:
lld/trunk/test/ELF/version-script-reassign-glob.s
Modified:
lld/trunk/ELF/Config.h
lld/trunk/ELF/Driver.cpp
lld/trunk/ELF/ScriptParser.cpp
lld/trunk/ELF/SymbolTable.cpp
lld/trunk/ELF/Symbols.cpp
lld/trunk/ELF/SyntheticSections.cpp
lld/trunk/ELF/Writer.cpp
lld/trunk/test/ELF/linkerscript/version-script.s
lld/trunk/test/ELF/version-script-reassign.s
Modified: lld/trunk/ELF/Config.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Config.h?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/Config.h (original)
+++ lld/trunk/ELF/Config.h Mon Aug 5 07:31:39 2019
@@ -71,8 +71,8 @@ struct SymbolVersion {
// can be found in version script if it is used for link.
struct VersionDefinition {
llvm::StringRef name;
- uint16_t id = 0;
- std::vector<SymbolVersion> globals;
+ uint16_t id;
+ std::vector<SymbolVersion> patterns;
};
// This struct contains the global configuration for the linker.
@@ -117,8 +117,6 @@ struct Configuration {
std::vector<llvm::StringRef> symbolOrderingFile;
std::vector<llvm::StringRef> undefined;
std::vector<SymbolVersion> dynamicList;
- std::vector<SymbolVersion> versionScriptGlobals;
- std::vector<SymbolVersion> versionScriptLocals;
std::vector<uint8_t> buildIdVector;
llvm::MapVector<std::pair<const InputSectionBase *, const InputSectionBase *>,
uint64_t>
@@ -224,7 +222,6 @@ struct Configuration {
ARMVFPArgKind armVFPArgs = ARMVFPArgKind::Default;
BuildIdKind buildId = BuildIdKind::None;
ELFKind ekind = ELFNoneKind;
- uint16_t defaultSymbolVersion = llvm::ELF::VER_NDX_GLOBAL;
uint16_t emachine = llvm::ELF::EM_NONE;
llvm::Optional<uint64_t> imageBase;
uint64_t commonPageSize;
@@ -310,6 +307,12 @@ struct Configuration {
// The only instance of Configuration struct.
extern Configuration *config;
+// The first two elements of versionDefinitions represent VER_NDX_LOCAL and
+// VER_NDX_GLOBAL. This helper returns other elements.
+static inline ArrayRef<VersionDefinition> namedVersionDefs() {
+ return llvm::makeArrayRef(config->versionDefinitions).slice(2);
+}
+
static inline void errorOrWarn(const Twine &msg) {
if (!config->noinhibitExec)
error(msg);
Modified: lld/trunk/ELF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/Driver.cpp (original)
+++ lld/trunk/ELF/Driver.cpp Mon Aug 5 07:31:39 2019
@@ -1011,14 +1011,20 @@ static void readConfigs(opt::InputArgLis
}
}
+ assert(config->versionDefinitions.empty());
+ config->versionDefinitions.push_back({"local", (uint16_t)VER_NDX_LOCAL, {}});
+ config->versionDefinitions.push_back(
+ {"global", (uint16_t)VER_NDX_GLOBAL, {}});
+
// If --retain-symbol-file is used, we'll keep only the symbols listed in
// the file and discard all others.
if (auto *arg = args.getLastArg(OPT_retain_symbols_file)) {
- config->defaultSymbolVersion = VER_NDX_LOCAL;
+ config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(
+ {"*", /*isExternCpp=*/false, /*hasWildcard=*/true});
if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
for (StringRef s : args::getLines(*buffer))
- config->versionScriptGlobals.push_back(
- {s, /*IsExternCpp*/ false, /*HasWildcard*/ false});
+ config->versionDefinitions[VER_NDX_GLOBAL].patterns.push_back(
+ {s, /*isExternCpp=*/false, /*hasWildcard=*/false});
}
bool hasExportDynamic =
Modified: lld/trunk/ELF/ScriptParser.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/ScriptParser.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/ScriptParser.cpp (original)
+++ lld/trunk/ELF/ScriptParser.cpp Mon Aug 5 07:31:39 2019
@@ -1344,16 +1344,10 @@ void ScriptParser::readAnonymousDeclarat
std::vector<SymbolVersion> locals;
std::vector<SymbolVersion> globals;
std::tie(locals, globals) = readSymbols();
-
- for (SymbolVersion v : locals) {
- if (v.name == "*")
- config->defaultSymbolVersion = VER_NDX_LOCAL;
- else
- config->versionScriptLocals.push_back(v);
- }
-
- for (SymbolVersion v : globals)
- config->versionScriptGlobals.push_back(v);
+ for (const SymbolVersion &pat : locals)
+ config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(pat);
+ for (const SymbolVersion &pat : globals)
+ config->versionDefinitions[VER_NDX_GLOBAL].patterns.push_back(pat);
expect(";");
}
@@ -1365,22 +1359,14 @@ void ScriptParser::readVersionDeclaratio
std::vector<SymbolVersion> locals;
std::vector<SymbolVersion> globals;
std::tie(locals, globals) = readSymbols();
-
- for (SymbolVersion v : locals) {
- if (v.name == "*")
- config->defaultSymbolVersion = VER_NDX_LOCAL;
- else
- config->versionScriptLocals.push_back(v);
- }
+ for (const SymbolVersion &pat : locals)
+ config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(pat);
// Create a new version definition and add that to the global symbols.
VersionDefinition ver;
ver.name = verStr;
- ver.globals = globals;
-
- // User-defined version number starts from 2 because 0 and 1 are
- // reserved for VER_NDX_LOCAL and VER_NDX_GLOBAL, respectively.
- ver.id = config->versionDefinitions.size() + 2;
+ ver.patterns = globals;
+ ver.id = config->versionDefinitions.size();
config->versionDefinitions.push_back(ver);
// Each version may have a parent version. For example, "Ver2"
Modified: lld/trunk/ELF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/SymbolTable.cpp (original)
+++ lld/trunk/ELF/SymbolTable.cpp Mon Aug 5 07:31:39 2019
@@ -73,7 +73,7 @@ Symbol *SymbolTable::insert(StringRef na
sym->setName(name);
sym->symbolKind = Symbol::PlaceholderKind;
- sym->versionId = config->defaultSymbolVersion;
+ sym->versionId = VER_NDX_GLOBAL;
sym->visibility = STV_DEFAULT;
sym->isUsedInRegularObj = false;
sym->exportDynamic = false;
@@ -192,7 +192,7 @@ void SymbolTable::assignExactVersion(Sym
return "VER_NDX_LOCAL";
if (ver == VER_NDX_GLOBAL)
return "VER_NDX_GLOBAL";
- return ("version '" + config->versionDefinitions[ver - 2].name + "'").str();
+ return ("version '" + config->versionDefinitions[ver].name + "'").str();
};
// Assign the version.
@@ -203,8 +203,12 @@ void SymbolTable::assignExactVersion(Sym
if (sym->getName().contains('@'))
continue;
- if (sym->versionId == config->defaultSymbolVersion)
+ // If the version has not been assigned, verdefIndex is -1. Use an arbitrary
+ // number (0) to indicate the version has been assigned.
+ if (sym->verdefIndex == UINT32_C(-1)) {
+ sym->verdefIndex = 0;
sym->versionId = versionId;
+ }
if (sym->versionId == versionId)
continue;
@@ -214,15 +218,14 @@ void SymbolTable::assignExactVersion(Sym
}
void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId) {
- if (!ver.hasWildcard)
- return;
-
// Exact matching takes precendence over fuzzy matching,
// so we set a version to a symbol only if no version has been assigned
// to the symbol. This behavior is compatible with GNU.
- for (Symbol *b : findAllByVersion(ver))
- if (b->versionId == config->defaultSymbolVersion)
- b->versionId = versionId;
+ for (Symbol *sym : findAllByVersion(ver))
+ if (sym->verdefIndex == UINT32_C(-1)) {
+ sym->verdefIndex = 0;
+ sym->versionId = versionId;
+ }
}
// This function processes version scripts by updating the versionId
@@ -233,26 +236,24 @@ void SymbolTable::assignWildcardVersion(
void SymbolTable::scanVersionScript() {
// First, we assign versions to exact matching symbols,
// i.e. version definitions not containing any glob meta-characters.
- for (SymbolVersion &ver : config->versionScriptGlobals)
- assignExactVersion(ver, VER_NDX_GLOBAL, "global");
- for (SymbolVersion &ver : config->versionScriptLocals)
- assignExactVersion(ver, VER_NDX_LOCAL, "local");
for (VersionDefinition &v : config->versionDefinitions)
- for (SymbolVersion &ver : v.globals)
- assignExactVersion(ver, v.id, v.name);
-
- // Next, we assign versions to fuzzy matching symbols,
- // i.e. version definitions containing glob meta-characters.
- for (SymbolVersion &ver : config->versionScriptGlobals)
- assignWildcardVersion(ver, VER_NDX_GLOBAL);
- for (SymbolVersion &ver : config->versionScriptLocals)
- assignWildcardVersion(ver, VER_NDX_LOCAL);
+ for (SymbolVersion &pat : v.patterns)
+ assignExactVersion(pat, v.id, v.name);
- // Note that because the last match takes precedence over previous matches,
- // we iterate over the definitions in the reverse order.
+ // Next, assign versions to wildcards that are not "*". Note that because the
+ // last match takes precedence over previous matches, we iterate over the
+ // definitions in the reverse order.
for (VersionDefinition &v : llvm::reverse(config->versionDefinitions))
- for (SymbolVersion &ver : v.globals)
- assignWildcardVersion(ver, v.id);
+ for (SymbolVersion &pat : v.patterns)
+ if (pat.hasWildcard && pat.name != "*")
+ assignWildcardVersion(pat, v.id);
+
+ // Then, assign versions to "*". In GNU linkers they have lower priority than
+ // other wildcards.
+ for (VersionDefinition &v : config->versionDefinitions)
+ for (SymbolVersion &pat : v.patterns)
+ if (pat.hasWildcard && pat.name == "*")
+ assignWildcardVersion(pat, v.id);
// Symbol themselves might know their versions because symbols
// can contain versions in the form of <name>@<version>.
@@ -262,7 +263,7 @@ void SymbolTable::scanVersionScript() {
// isPreemptible is false at this point. To correctly compute the binding of a
// Defined (which is used by includeInDynsym()), we need to know if it is
- // VER_NDX_LOCAL or not. If defaultSymbolVersion is VER_NDX_LOCAL, we should
- // compute symbol versions before handling --dynamic-list.
+ // VER_NDX_LOCAL or not. Compute symbol versions before handling
+ // --dynamic-list.
handleDynamicList();
}
Modified: lld/trunk/ELF/Symbols.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/Symbols.cpp (original)
+++ lld/trunk/ELF/Symbols.cpp Mon Aug 5 07:31:39 2019
@@ -227,7 +227,7 @@ void Symbol::parseSymbolVersion() {
if (isDefault)
verstr = verstr.substr(1);
- for (VersionDefinition &ver : config->versionDefinitions) {
+ for (const VersionDefinition &ver : namedVersionDefs()) {
if (ver.name != verstr)
continue;
Modified: lld/trunk/ELF/SyntheticSections.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/SyntheticSections.cpp (original)
+++ lld/trunk/ELF/SyntheticSections.cpp Mon Aug 5 07:31:39 2019
@@ -1162,10 +1162,12 @@ void StringTableSection::writeTo(uint8_t
}
}
-// Returns the number of version definition entries. Because the first entry
-// is for the version definition itself, it is the number of versioned symbols
-// plus one. Note that we don't support multiple versions yet.
-static unsigned getVerDefNum() { return config->versionDefinitions.size() + 1; }
+// Returns the number of entries in .gnu.version_d: the number of
+// non-VER_NDX_LOCAL-non-VER_NDX_GLOBAL definitions, plus 1.
+// Note that we don't support vd_cnt > 1 yet.
+static unsigned getVerDefNum() {
+ return namedVersionDefs().size() + 1;
+}
template <class ELFT>
DynamicSection<ELFT>::DynamicSection()
@@ -2771,7 +2773,7 @@ StringRef VersionDefinitionSection::getF
void VersionDefinitionSection::finalizeContents() {
fileDefNameOff = getPartition().dynStrTab->addString(getFileDefName());
- for (VersionDefinition &v : config->versionDefinitions)
+ for (const VersionDefinition &v : namedVersionDefs())
verDefNameOffs.push_back(getPartition().dynStrTab->addString(v.name));
if (OutputSection *sec = getPartition().dynStrTab->getParent())
@@ -2805,7 +2807,7 @@ void VersionDefinitionSection::writeTo(u
writeOne(buf, 1, getFileDefName(), fileDefNameOff);
auto nameOffIt = verDefNameOffs.begin();
- for (VersionDefinition &v : config->versionDefinitions) {
+ for (const VersionDefinition &v : namedVersionDefs()) {
buf += EntrySize;
writeOne(buf, v.id, v.name, *nameOffIt++);
}
Modified: lld/trunk/ELF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/ELF/Writer.cpp (original)
+++ lld/trunk/ELF/Writer.cpp Mon Aug 5 07:31:39 2019
@@ -396,7 +396,7 @@ template <class ELFT> static void create
part.verSym = make<VersionTableSection>();
add(part.verSym);
- if (!config->versionDefinitions.empty()) {
+ if (!namedVersionDefs().empty()) {
part.verDef = make<VersionDefinitionSection>();
add(part.verDef);
}
Modified: lld/trunk/test/ELF/linkerscript/version-script.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/version-script.s?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/test/ELF/linkerscript/version-script.s (original)
+++ lld/trunk/test/ELF/linkerscript/version-script.s Mon Aug 5 07:31:39 2019
@@ -17,7 +17,7 @@
# CHECK-NEXT: Name:
# CHECK-NEXT: }
# CHECK-NEXT: Symbol {
-# CHECK-NEXT: Version: 0
+# CHECK-NEXT: Version: 1
# CHECK-NEXT: Name: und
# CHECK-NEXT: }
# CHECK-NEXT: Symbol {
Added: lld/trunk/test/ELF/version-script-reassign-glob.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/version-script-reassign-glob.s?rev=367869&view=auto
==============================================================================
--- lld/trunk/test/ELF/version-script-reassign-glob.s (added)
+++ lld/trunk/test/ELF/version-script-reassign-glob.s Mon Aug 5 07:31:39 2019
@@ -0,0 +1,19 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+
+# RUN: echo 'foo { foo*; }; bar { *; };' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so --fatal-warnings
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=FOO %s
+
+# RUN: echo 'foo { foo*; }; bar { f*; };' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so --fatal-warnings
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=BAR %s
+
+## If both a non-* glob and a * match, non-* wins.
+## This is GNU linkers' behavior. We don't feel strongly this should be supported.
+# FOO: GLOBAL DEFAULT 7 foo@@foo
+
+# BAR: GLOBAL DEFAULT 7 foo@@bar
+
+.globl foo
+foo:
Modified: lld/trunk/test/ELF/version-script-reassign.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/version-script-reassign.s?rev=367869&r1=367868&r2=367869&view=diff
==============================================================================
--- lld/trunk/test/ELF/version-script-reassign.s (original)
+++ lld/trunk/test/ELF/version-script-reassign.s Mon Aug 5 07:31:39 2019
@@ -1,18 +1,22 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: echo '{ global: foo; };' > %tg.ver
# RUN: echo '{ local: foo; };' > %tl.ver
-# RUN: echo '{ global: foo; local: *; };' > %tg.ver
+# RUN: echo '{ global: foo; local: *; };' > %tgl.ver
# RUN: echo 'V1 { global: foo; };' > %t1.ver
# RUN: echo 'V2 { global: foo; };' > %t2.ver
# RUN: echo 'V2 { global: notexist; local: f*; };' > %t2w.ver
-## Note, ld.bfd errors on the two cases.
+## Note, ld.bfd errors on these cases.
# RUN: ld.lld -shared %t.o --version-script %tl.ver --version-script %t1.ver \
# RUN: -o %t.so 2>&1 | FileCheck --check-prefix=LOCAL %s
# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=LOCAL-SYM %s
# RUN: ld.lld -shared %t.o --version-script %tg.ver --version-script %t1.ver \
# RUN: -o %t.so 2>&1 | FileCheck --check-prefix=GLOBAL %s
# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=GLOBAL-SYM %s
+# RUN: ld.lld -shared %t.o --version-script %tgl.ver --version-script %t1.ver \
+# RUN: -o %t.so 2>&1 | FileCheck --check-prefix=GLOBAL %s
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=GLOBAL-SYM %s
## Note, ld.bfd silently accepts this case.
# RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2.ver \
More information about the llvm-commits
mailing list