[lld][WebAssembly] Add GOT.mem for WASM_MEMORY_ADDR_REL_ of Shared Data
#104920
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lld-wasm @llvm/pr-subscribers-lld Author: Luc Blaeser (luc-blaeser) ChangesFix Full diff: https://github.com/llvm/llvm-project/pull/104920.diff 3 Files Affected:
diff --git a/lld/test/wasm/shared-relative-data.s b/lld/test/wasm/shared-relative-data.s
new file mode 100644
index 00000000000000..2bb31c57e9a560
--- /dev/null
+++ b/lld/test/wasm/shared-relative-data.s
@@ -0,0 +1,126 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld --experimental-pic -shared -o %t.wasm %t.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+# RUN: llvm-objdump --disassemble-symbols=test_relative_local_data_access,test_relative_shared_data_access --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __memory_base, i32, immutable
+
+.section .data.local_data,"",@
+local_data:
+ .int32 1
+ .size local_data, 4
+
+.section .data.external_data,"",@
+external_data:
+ .int32 2
+ .size external_data, 4
+
+.section .text,"",@
+test_relative_local_data_access:
+ .functype test_relative_local_data_access () -> ()
+ global.get __memory_base
+ i32.const local_data@MBREL
+ i32.add
+ i32.load 0
+ drop
+ end_function
+
+# Test relative address relocation in code section for shared data
+
+test_relative_shared_data_access:
+ .functype test_relative_shared_data_access () -> ()
+ global.get __memory_base
+ i32.const external_data@MBREL
+ i32.add
+ i32.load 0
+ drop
+ end_function
+
+_start:
+ .functype _start () -> ()
+ call test_relative_local_data_access
+ call test_relative_shared_data_access
+ end_function
+
+.globl external_data
+.globl _start
+
+# Verify that `GOT.mem` has been added for `external_data`.
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 8
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 0
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+# CHECK-NEXT: Signatures:
+# CHECK-NEXT: - Index: 0
+# CHECK-NEXT: ParamTypes: []
+# CHECK-NEXT: ReturnTypes: []
+# CHECK-NEXT: - Type: IMPORT
+# CHECK-NEXT: Imports:
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: memory
+# CHECK-NEXT: Kind: MEMORY
+# CHECK-NEXT: Memory:
+# CHECK-NEXT: Minimum: 0x1
+# 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: external_data
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0 ]
+# CHECK-NEXT: - Type: GLOBAL
+# CHECK-NEXT: Globals:
+# CHECK-NEXT: - Index: 3
+# CHECK-NEXT: Type: I32
+# CHECK-NEXT: Mutable: false
+# CHECK-NEXT: InitExpr:
+# CHECK-NEXT: Opcode: I32_CONST
+# CHECK-NEXT: Value: 4
+# CHECK-NEXT: - Type: EXPORT
+# CHECK-NEXT: Exports:
+# CHECK-NEXT: - Name: __wasm_call_ctors
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: - Name: __wasm_apply_data_relocs
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: - Name: external_data
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: Index: 3
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 4
+
+# DIS: <test_relative_local_data_access>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 0
+# DIS-NEXT: i32.const 0
+# DIS-NEXT: i32.add
+# DIS-NEXT: i32.load 0
+# DIS-NEXT: drop
+# DIS-NEXT: end
+
+# DIS: <test_relative_shared_data_access>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 0
+# DIS-NEXT: i32.const 4
+# DIS-NEXT: i32.add
+# DIS-NEXT: i32.load 0
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/test/wasm/shared-relative-data64.s b/lld/test/wasm/shared-relative-data64.s
new file mode 100644
index 00000000000000..4d9795eb41531f
--- /dev/null
+++ b/lld/test/wasm/shared-relative-data64.s
@@ -0,0 +1,127 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm64-unknown-unknown -o %t.o %s
+# RUN: wasm-ld -mwasm64 --experimental-pic -shared -o %t.wasm %t.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+# RUN: llvm-objdump --disassemble-symbols=test_relative_local_data_access,test_relative_shared_data_access --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __memory_base, i64, immutable
+
+.section .data.local_data,"",@
+local_data:
+ .int64 1
+ .size local_data, 8
+
+.section .data.external_data,"",@
+external_data:
+ .int64 2
+ .size external_data, 8
+
+.section .text,"",@
+test_relative_local_data_access:
+ .functype test_relative_local_data_access () -> ()
+ global.get __memory_base
+ i64.const local_data@MBREL
+ i64.add
+ i64.load 0
+ drop
+ end_function
+
+# Test relative address relocation in code section for shared data
+
+test_relative_shared_data_access:
+ .functype test_relative_shared_data_access () -> ()
+ global.get __memory_base
+ i64.const external_data@MBREL
+ i64.add
+ i64.load 0
+ drop
+ end_function
+
+_start:
+ .functype _start () -> ()
+ call test_relative_local_data_access
+ call test_relative_shared_data_access
+ end_function
+
+.globl external_data
+.globl _start
+
+# Verify that `GOT.mem` has been added for `external_data`.
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 16
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 0
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+# CHECK-NEXT: Signatures:
+# CHECK-NEXT: - Index: 0
+# CHECK-NEXT: ParamTypes: []
+# CHECK-NEXT: ReturnTypes: []
+# CHECK-NEXT: - Type: IMPORT
+# CHECK-NEXT: Imports:
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: memory
+# CHECK-NEXT: Kind: MEMORY
+# CHECK-NEXT: Memory:
+# CHECK-NEXT: Flags: [ IS_64 ]
+# CHECK-NEXT: Minimum: 0x1
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __memory_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I64
+# CHECK-NEXT: GlobalMutable: false
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __table_base
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I64
+# CHECK-NEXT: GlobalMutable: false
+# CHECK-NEXT: - Module: GOT.mem
+# CHECK-NEXT: Field: external_data
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I64
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0 ]
+# CHECK-NEXT: - Type: GLOBAL
+# CHECK-NEXT: Globals:
+# CHECK-NEXT: - Index: 3
+# CHECK-NEXT: Type: I64
+# CHECK-NEXT: Mutable: false
+# CHECK-NEXT: InitExpr:
+# CHECK-NEXT: Opcode: I64_CONST
+# CHECK-NEXT: Value: 8
+# CHECK-NEXT: - Type: EXPORT
+# CHECK-NEXT: Exports:
+# CHECK-NEXT: - Name: __wasm_call_ctors
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: - Name: __wasm_apply_data_relocs
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: - Name: external_data
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: Index: 3
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 4
+
+# DIS: <test_relative_local_data_access>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 0
+# DIS-NEXT: i64.const 0
+# DIS-NEXT: i64.add
+# DIS-NEXT: i64.load 0
+# DIS-NEXT: drop
+# DIS-NEXT: end
+
+# DIS: <test_relative_shared_data_access>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 0
+# DIS-NEXT: i64.const 8
+# DIS-NEXT: i64.add
+# DIS-NEXT: i64.load 0
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index 6f33a4f28a9d09..b4b1b07cfd2106 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -115,6 +115,11 @@ void scanRelocations(InputChunk *chunk) {
if (!isa<GlobalSymbol>(sym))
addGOTEntry(sym);
break;
+ case R_WASM_MEMORY_ADDR_REL_SLEB:
+ case R_WASM_MEMORY_ADDR_REL_SLEB64:
+ if (requiresGOTAccess(sym))
+ addGOTEntry(sym);
+ break;
case R_WASM_MEMORY_ADDR_TLS_SLEB:
case R_WASM_MEMORY_ADDR_TLS_SLEB64:
if (!sym->isDefined()) {
|
|
The way this stuff is designed to work is that a given symbol is either accessed via the GOT or via base register. If you are accessing something via If you are accessing something via a GOT entry then each symbol gets its own global (no offset needed). |
Thank you for the explanation. I was primarily wondering about the case an importing module would need to perform pointer comparison or arithmetics on that exported data. But maybe this would need explicit GOT entry from the LLVM codegen? I am happy to close this PR if not needed. |
|
Perhaps you could explain in a little more detail? Basically, at codegen time that code generator makes and either or decsion when it need to calculate the address of a symbol:
(1) comes at some cost since it requires O(N) wasm globals whereas (2) only requires a single global, but requires the additional addition of a constant. (1) is the most conservative option since it will work with any symbol. (2) only works for symbols that are local to the current DSO (and cannot be interposed / replaced by another DSO). The most commen example I can think of here are |
|
Thanks for following up. I was thinking of a case where a pointer to static data of DSO would be exported. But then the codegen should not use approach (2) if I understand correctly. |
Fix
wasm-ldto handleWASM_MEMORY_ADDR_REL_relocations referring to shared data. TheGOT.mementry was missing for the corresponding data.