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

[via llvm-commits]

Author: Sam Clegg
Date: 2025-01-28T16:06:01-08:00
New Revision: 617278e7b0c937fccbf7d67d14f053c3409bc33f

URL: https://github.com/llvm/llvm-project/commit/617278e7b0c937fccbf7d67d14f053c3409bc33f
DIFF: https://github.com/llvm/llvm-project/commit/617278e7b0c937fccbf7d67d14f053c3409bc33f.diff

LOG: [lld][WebAssembly] Fix for shared library symbols WRT replacing lazy symbols (#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

Added: 
    lld/test/wasm/shared-lazy.s

Modified: 
    lld/wasm/SymbolTable.cpp

Removed: 
    


################################################################################
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;
 }