[lld] [lld][WebAssembly] Fix for shared library symbols WRT replacing lazy symbols (PR #124619)

[Sam Clegg via llvm-commits]

https://github.com/sbc100 created https://github.com/llvm/llvm-project/pull/124619

The rule here, which I'm copying from the ELF linker, is that shared library symbols should take presence, unless the symbol has already be extracted from the archive.  e.g:

```
$ wasm-ld foo.a foo.so ref.o  // .so wins
$ wasm-ld foo.a ref.o foo.so  // .a wins
```

In the first case the shared library takes precedence because the lazy symbol is replaced by the .so symbol before it is extracted from the archive.  In the second example the ref.o file causes the archive to be exracted before the .so file is processed, so in that case the archive file wins.

Fixes: https://github.com/emscripten-core/emscripten/issues/23501

>From 8ccf1abf1caeed3d30167b0d223a86b54ce5a045 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc at chromium.org>
Date: Mon, 27 Jan 2025 10:08:39 -0800
Subject: [PATCH] [lld][WebAssembly] Fix for shared library symbols WRT
 replacing lazy symbols

The rule here, which I'm copying from the ELF linker, is that shared
library symbols should take presence, unless the symbol has already be
extracted from the archive.  e.g:

wasm-ld foo.a foo.so ref.o

wasm-ld foo.a ref.o foo.so

In the first case the shared library takes precedence because the lazy
symbol is replaced by the .so symbol before it is extracted from the
archive.  In the second example the ref.o file causes the archive to be
exracted before the .so file is processed, so in that case the archive
file wins.

Fixes: https://github.com/emscripten-core/emscripten/issues/23501
---
 lld/test/wasm/shared-lazy.s | 79 +++++++++++++++++++++++++++++++++++++
 lld/wasm/SymbolTable.cpp    | 12 +++++-
 2 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 lld/test/wasm/shared-lazy.s

diff --git a/lld/test/wasm/shared-lazy.s b/lld/test/wasm/shared-lazy.s
new file mode 100644
index 00000000000000..f1044547203a2a
--- /dev/null
+++ b/lld/test/wasm/shared-lazy.s
@@ -0,0 +1,79 @@
+##  Based on lld/test/ELF/shared-lazy.s
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten a.s -o a.o
+# RUN: wasm-ld a.o --experimental-pic -shared -o a.so
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten b.s -o b.o
+# RUN: wasm-ld b.o --experimental-pic -shared -o b.so
+# RUN: llvm-ar rc a.a a.o
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten ref.s -o ref.o
+# RUN: wasm-ld a.a b.so ref.o --experimental-pic -shared -o 1.so
+# RUN: obj2yaml 1.so | FileCheck %s
+# RUN: wasm-ld a.so a.a ref.o --experimental-pic -shared -o 1.so
+# RUN: obj2yaml 1.so | FileCheck %s
+
+## The definitions from a.so are used and we don't extract a member from the
+## archive.
+
+# CHECK:   - Type:            IMPORT
+# CHECK:            - Module:          GOT.mem
+# CHECK-NEXT:         Field:           x1
+# CHECK-NEXT:         Kind:            GLOBAL
+# CHECK-NEXT:         GlobalType:      I32
+# CHECK-NEXT:         GlobalMutable:   true
+# CHECK-NEXT:       - Module:          GOT.mem
+# CHECK-NEXT:         Field:           x2
+# CHECK-NEXT:         Kind:            GLOBAL
+# CHECK-NEXT:         GlobalType:      I32
+# CHECK-NEXT:         GlobalMutable:   true
+
+## The extracted x1 is defined as STB_GLOBAL.
+# RUN: wasm-ld ref.o a.a b.so -o 2.so --experimental-pic -shared
+# RUN: obj2yaml 2.so | FileCheck %s --check-prefix=CHECK2
+# RUN: wasm-ld a.a ref.o b.so -o 2.so --experimental-pic -shared
+# RUN: obj2yaml 2.so | FileCheck %s --check-prefix=CHECK2
+
+# CHECK2:       - Type:            EXPORT
+# CHECK2-NEXT:    Exports:
+# CHECK2-NEXT:      - Name:            __wasm_call_ctors
+# CHECK2-NEXT:        Kind:            FUNCTION
+# CHECK2-NEXT:        Index:
+# CHECK2-NEXT:      - Name:            x1
+# CHECK2-NEXT:        Kind:            GLOBAL
+# CHECK2-NEXT:        Index:
+# CHECK2-NEXT:      - Name:            x2
+# CHECK2-NEXT:        Kind:            GLOBAL
+
+#--- a.s
+.section .data.x1,"",@
+.global x1
+x1:
+  .byte 0
+.size x1, 1
+
+.section .data.x2,"",@
+.weak x2
+x2:
+  .byte 0
+.size x2, 1
+#--- b.s
+.section .data.x1,"",@
+.globl x1
+x1:
+  .byte 0
+.size x1, 1
+
+.section .data.x2,"",@
+.globl x2
+x2:
+  .byte 0
+.size x2, 1
+#--- ref.s
+.globl x1
+.globl x2
+.globl d
+.section .data.d,"",@
+d:
+  .int x1
+  .int x2
+.size d, 8
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 6309cf35fe5523..7e8b4aa632a321 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -363,7 +363,7 @@ Symbol *SymbolTable::addSharedFunction(StringRef name, uint32_t flags,
     replaceSymbol<SharedFunctionSymbol>(sym, name, flags, file, sig);
   };
 
-  if (wasInserted) {
+  if (wasInserted || s->isLazy()) {
     replaceSym(s);
     return s;
   }
@@ -408,10 +408,18 @@ Symbol *SymbolTable::addSharedData(StringRef name, uint32_t flags,
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
 
-  if (wasInserted || s->isUndefined()) {
+  if (wasInserted || s->isLazy()) {
     replaceSymbol<SharedData>(s, name, flags, file);
+    return s;
+  }
+
+  // Shared symbols should never replace locally-defined ones
+  if (s->isDefined()) {
+    return s;
   }
 
+  checkDataType(s, file);
+  replaceSymbol<SharedData>(s, name, flags, file);
   return s;
 }