Fix version script priority

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 #1158

common/common.h

@@ -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];
   };
@@ -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;
 };

common/multi-glob.cc

@@ -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
@@ -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;
         }
 
@@ -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;
 }
@@ -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());
 
@@ -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;
 }
 
@@ -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());

elf/passes.cc

@@ -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;
@@ -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);
 

test/elf/version-script22.sh

@@ -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