Skip to content

Commit

Permalink
Fix version script priority
Browse files Browse the repository at this point in the history
Previously, if two or more VERSION clauses match to the same symbol,
the first one took precedence. This was incompatible with GNU ld, which
gives the last one the highesth priority.

This change inverted the priority so that the last one will take
precedence other the others.

Fixes rui314#1158
  • Loading branch information
rui314 committed Dec 23, 2023
1 parent 35516a6 commit e1e16bf
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 23 deletions.
8 changes: 4 additions & 4 deletions common/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -829,13 +829,13 @@ class Glob {

class MultiGlob {
public:
bool add(std::string_view pat, u32 val);
bool add(std::string_view pat, i64 val);
bool empty() const { return strings.empty(); }
std::optional<u32> find(std::string_view str);
std::optional<i64> find(std::string_view str);

private:
struct TrieNode {
u32 value = -1;
i64 value = -1;
TrieNode *suffix_link = nullptr;
std::unique_ptr<TrieNode> children[256];
};
Expand All @@ -846,7 +846,7 @@ class MultiGlob {

std::vector<std::string> strings;
std::unique_ptr<TrieNode> root;
std::vector<std::pair<Glob, u32>> globs;
std::vector<std::pair<Glob, i64>> globs;
std::once_flag once;
bool is_compiled = false;
};
Expand Down
18 changes: 9 additions & 9 deletions common/multi-glob.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@

namespace mold {

std::optional<u32> MultiGlob::find(std::string_view str) {
std::optional<i64> MultiGlob::find(std::string_view str) {
std::call_once(once, [&] { compile(); });
u32 val = UINT32_MAX;
i64 val = -1;

if (root) {
// Match against simple glob patterns
Expand All @@ -36,7 +36,7 @@ std::optional<u32> MultiGlob::find(std::string_view str) {
for (;;) {
if (node->children[c]) {
node = node->children[c].get();
val = std::min(val, node->value);
val = std::max(val, node->value);
return;
}

Expand All @@ -53,11 +53,11 @@ std::optional<u32> MultiGlob::find(std::string_view str) {
}

// Match against complex glob patterns
for (std::pair<Glob, u32> &glob : globs)
for (std::pair<Glob, i64> &glob : globs)
if (glob.first.match(str))
val = std::min(val, glob.second);
val = std::max(val, glob.second);

if (val == UINT32_MAX)
if (val == -1)
return {};
return val;
}
Expand All @@ -82,7 +82,7 @@ static std::string handle_stars(std::string_view pat) {
return "\0"s + str + "\0"s;
}

bool MultiGlob::add(std::string_view pat, u32 val) {
bool MultiGlob::add(std::string_view pat, i64 val) {
assert(!is_compiled);
assert(!pat.empty());

Expand All @@ -108,7 +108,7 @@ bool MultiGlob::add(std::string_view pat, u32 val) {
node = node->children[c].get();
}

node->value = std::min(node->value, val);
node->value = std::max(node->value, val);
return true;
}

Expand Down Expand Up @@ -157,7 +157,7 @@ void MultiGlob::fix_values() {
for (std::unique_ptr<TrieNode> &child : node->children) {
if (!child)
continue;
child->value = std::min(child->value, child->suffix_link->value);
child->value = std::max(child->value, child->suffix_link->value);
queue.push(child.get());
}
} while (!queue.empty());
Expand Down
34 changes: 24 additions & 10 deletions elf/passes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1656,8 +1656,22 @@ void apply_version_script(Context<E> &ctx) {
MultiGlob matcher;
MultiGlob cpp_matcher;

for (i64 i = 0; i < ctx.version_patterns.size(); i++) {
VersionPattern &v = ctx.version_patterns[i];
// The "local:" label has a special meaning in the version script.
// It can appear in any VERSION clause, and it hides matched symbols
// unless other non-local patterns match to them. In other words,
// "local:" has lower precedence than other version definitions.
//
// If two or more non-local patterns match to the same symbol, the
// last one takes precedence.
std::vector<VersionPattern> patterns = ctx.version_patterns;

std::stable_partition(patterns.begin(), patterns.end(),
[](const VersionPattern &pat) {
return pat.ver_idx == VER_NDX_LOCAL;
});

for (i64 i = 0; i < patterns.size(); i++) {
VersionPattern &v = patterns[i];
if (v.is_cpp) {
if (!cpp_matcher.add(v.pattern, i))
Fatal(ctx) << "invalid version pattern: " << v.pattern;
Expand All @@ -1674,30 +1688,30 @@ void apply_version_script(Context<E> &ctx) {
continue;

std::string_view name = sym->name();
i64 match = INT64_MAX;
i64 match = -1;

if (std::optional<u32> idx = matcher.find(name))
match = std::min<i64>(match, *idx);
if (std::optional<i64> idx = matcher.find(name))
match = std::max(match, *idx);

// Match non-mangled symbols against the C++ pattern as well.
// Weird, but required to match other linkers' behavior.
if (!cpp_matcher.empty()) {
if (std::optional<std::string_view> s = demangle_cpp(name))
name = *s;
if (std::optional<u32> idx = cpp_matcher.find(name))
match = std::min<i64>(match, *idx);
if (std::optional<i64> idx = cpp_matcher.find(name))
match = std::max(match, *idx);
}

if (match != INT64_MAX)
sym->ver_idx = ctx.version_patterns[match].ver_idx;
if (match != -1)
sym->ver_idx = patterns[match].ver_idx;
}
});
}

// Next, assign versions to symbols specified by exact name.
// In other words, exact matches have higher precedence over
// wildcard or `extern "C++"` patterns.
for (VersionPattern &v : ctx.version_patterns) {
for (VersionPattern &v : patterns) {
if (!v.is_cpp && v.pattern.find_first_of("*?[") == v.pattern.npos) {
Symbol<E> *sym = get_symbol(ctx, v.pattern);

Expand Down
15 changes: 15 additions & 0 deletions test/elf/version-script22.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash
. $(dirname $0)/common.inc

cat <<'EOF' > $t/a.ver
VER1 { foo*; };
VER2 { foo*bar*; };
EOF

cat <<EOF | $CC -fPIC -c -o $t/b.o -xc -
void foo_bar() {}
EOF

$CC -B. -shared -Wl,--version-script=$t/a.ver -o $t/c.so $t/b.o
readelf -W --dyn-syms $t/c.so > $t/log
grep -Fq 'foo_bar@@VER2' $t/log

0 comments on commit e1e16bf

Please sign in to comment.