[lld][WebAssembly] Allow linker-synthetic symbols to be undefine when… #153537
Conversation
|
@llvm/pr-subscribers-lld-wasm @llvm/pr-subscribers-lld Author: YAMAMOTO Takashi (yamt) Changes… building shared libraries Fixes: #103592 Full diff: https://github.com/llvm/llvm-project/pull/153537.diff 2 Files Affected:
diff --git a/lld/test/wasm/shared-synthetic-symbols.s b/lld/test/wasm/shared-synthetic-symbols.s
new file mode 100644
index 0000000000000..0c9c68aabc239
--- /dev/null
+++ b/lld/test/wasm/shared-synthetic-symbols.s
@@ -0,0 +1,75 @@
+## Check that synthetic data-layout symbols such as __heap_base and __heap_end
+## can be referenced from shared libraries and pie executables without
+## generating undefined symbols.
+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld --experimental-pic -pie --import-memory -o %t.wasm %t.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+# RUN: wasm-ld --experimental-pic -shared -o %t.so %t.o
+# RUN: obj2yaml %t.so | FileCheck %s
+
+.globl _start
+
+_start:
+ .functype _start () -> ()
+ i32.const __heap_base@GOT
+ drop
+ i32.const __heap_end@GOT
+ drop
+ i32.const __stack_low@GOT
+ drop
+ i32.const __stack_high@GOT
+ drop
+ i32.const __global_base@GOT
+ drop
+ i32.const __data_end@GOT
+ drop
+ end_function
+
+# CHECK: - Type: IMPORT
+# CHECK-NEXT: Imports:
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: memory
+# CHECK-NEXT: Kind: MEMORY
+# CHECK-NEXT: Memory:
+# CHECK-NEXT: Minimum: 0x0
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __memory_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: false
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __table_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: false
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __heap_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __heap_end
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __stack_low
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __stack_high
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __global_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: __data_end
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 1c5d21c06f5af..3b76e91aaab3b 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -985,23 +985,35 @@ static void createOptionalSymbols() {
ctx.sym.dsoHandle = symtab->addOptionalDataSymbol("__dso_handle");
- if (!ctx.arg.shared)
- ctx.sym.dataEnd = symtab->addOptionalDataSymbol("__data_end");
-
+ auto addDataLayoutSymbol = [&](StringRef s) -> DefinedData * {
+ // Data layout symbols are either defined by the lld, or (in the case
+ // of PIC code) defined by the dynamic linker / embedder.
+ if (ctx.isPic) {
+ ctx.arg.allowUndefinedSymbols.insert(s);
+ return nullptr;
+ } else {
+ return symtab->addOptionalDataSymbol(s);
+ }
+ };
+
+ ctx.sym.dataEnd = addDataLayoutSymbol("__data_end");
+ ctx.sym.stackLow = addDataLayoutSymbol("__stack_low");
+ ctx.sym.stackHigh = addDataLayoutSymbol("__stack_high");
+ ctx.sym.globalBase = addDataLayoutSymbol("__global_base");
+ ctx.sym.heapBase = addDataLayoutSymbol("__heap_base");
+ ctx.sym.heapEnd = addDataLayoutSymbol("__heap_end");
+
+ // for pic,
+ // - __memory_base and __table_base are handled in createSyntheticSymbols.
+ // - how __wasm_first_page_end should be is not clear yet.
if (!ctx.isPic) {
- ctx.sym.stackLow = symtab->addOptionalDataSymbol("__stack_low");
- ctx.sym.stackHigh = symtab->addOptionalDataSymbol("__stack_high");
- ctx.sym.globalBase = symtab->addOptionalDataSymbol("__global_base");
- ctx.sym.heapBase = symtab->addOptionalDataSymbol("__heap_base");
- ctx.sym.heapEnd = symtab->addOptionalDataSymbol("__heap_end");
ctx.sym.definedMemoryBase = symtab->addOptionalDataSymbol("__memory_base");
ctx.sym.definedTableBase = symtab->addOptionalDataSymbol("__table_base");
+ ctx.sym.firstPageEnd = symtab->addOptionalDataSymbol("__wasm_first_page_end");
+ if (ctx.sym.firstPageEnd)
+ ctx.sym.firstPageEnd->setVA(ctx.arg.pageSize);
}
- ctx.sym.firstPageEnd = symtab->addOptionalDataSymbol("__wasm_first_page_end");
- if (ctx.sym.firstPageEnd)
- ctx.sym.firstPageEnd->setVA(ctx.arg.pageSize);
-
// For non-shared memory programs we still need to define __tls_base since we
// allow object files built with TLS to be linked into single threaded
// programs, and such object files can contain references to this symbol.
|
|
original PR: #128223 |
yamt
force-pushed
the
shared_libary_data_layout_symbols2
branch
from
6f0e70d to
7101db1
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
lld/wasm/Driver.cpp
Outdated
There was a problem hiding this comment.
i suspect __data_end and __global_base don't make much sense for pic. what do you think?
There was a problem hiding this comment.
Agreed, but what should they resolve to? Should they point to the start and end of the data region of the current DSO, or should they point to the start and end of the main module's data (which I guess is what this change would do?
lld/wasm/Driver.cpp
Outdated
There was a problem hiding this comment.
i'm not sure how __wasm_first_page_end should work with dynamic-linking.
this PR is leaning towards the conservative side for now. ie. do not provide the symbol.
right now i can think of two ways:
a. make the runtime linker provide the global.
b. declare dynamic-linking and custom-page-sizes are not compatible.
both of them have obvious downsides.
There was a problem hiding this comment.
With dynamic linking the generated Wasm modules are still importing a memory, correct? That imported memory has a type that must be provided to the import declaration, and that type includes the page size, so I would expect that the __wasm_first_page_end symbol should largely remain the same (that is, it should still be a constant, but based on the memory import's type rather than the memory definition's type).
There was a problem hiding this comment.
Indeed, it should be absolute/constant address, regardless of dynamic linking.
There was a problem hiding this comment.
ok. it makes sense.
wasi-sdk may need to ship another set of shared libraries with page_size=1, right?
There was a problem hiding this comment.
wasi-sdk may need to ship another set of shared libraries with page_size=1, right?
it seems working with:
WebAssembly/wasi-libc#616
#153763
yamt
force-pushed
the
shared_libary_data_layout_symbols2
branch
from
7101db1 to
ff1719f
Compare
… building shared libraries Fixes: llvm#103592 Co-authored-by: YAMAMOTO Takashi <[email protected]>
yamt
force-pushed
the
shared_libary_data_layout_symbols2
branch
from
ff1719f to
51ced5c
Compare
|
@sbc100 can this land? |
4 participants
… building shared libraries
Fixes: #103592