[lld][WebAssembly] Fix relocation of Wasm table index with GOT access #104043
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be 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 If you have received no comments on your PR for a week, you can request a review 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 Author: Luc Blaeser (luc-blaeser) ChangesFix Full diff: https://github.com/llvm/llvm-project/pull/104043.diff 4 Files Affected:
diff --git a/lld/test/wasm/call-indirect-external.s b/lld/test/wasm/call-indirect-external.s
new file mode 100644
index 00000000000000..e226b891442485
--- /dev/null
+++ b/lld/test/wasm/call-indirect-external.s
@@ -0,0 +1,27 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+# ERRUND: error: {{.*}}.o: relocation R_WASM_TABLE_INDEX_REL_SLEB cannot be used against an undefined symbol `external_func`
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --unresolved-symbols=report-all 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --warn-unresolved-symbols 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --unresolved-symbols=ignore-all 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+.globaltype __table_base, i32, immutable
+
+.functype external_func () -> ()
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ global.get __table_base
+ i32.const external_func@TBREL
+ i32.add
+ call_indirect () -> ()
+ end_function
diff --git a/lld/test/wasm/shared-call-indirect.s b/lld/test/wasm/shared-call-indirect.s
new file mode 100644
index 00000000000000..d805670b8513c7
--- /dev/null
+++ b/lld/test/wasm/shared-call-indirect.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_indirect_local_func_call,test_indirect_shared_func_call --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __table_base, i32, immutable
+
+test_indirect_local_func_call:
+ .functype test_indirect_local_func_call () -> ()
+ global.get __table_base
+ i32.const local_func@TBREL
+ i32.add
+ call_indirect () -> ()
+ end_function
+
+# Test table index relocation in code section for shared functions
+
+test_indirect_shared_func_call:
+ .functype test_indirect_shared_func_call () -> ()
+ i64.const 12345
+ global.get __table_base
+ i32.const shared_func@TBREL
+ i32.add
+ call_indirect (i64) -> (i32)
+ drop
+ end_function
+
+local_func:
+ .functype local_func () -> ()
+ end_function
+
+.globl shared_func
+shared_func:
+ .functype shared_func (i64) -> (i32)
+ local.get 0
+ i32.wrap_i64
+ end_function
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ call test_indirect_local_func_call
+ call test_indirect_shared_func_call
+ end_function
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 0
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 2
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+
+# 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: __indirect_function_table
+# CHECK-NEXT: Kind: TABLE
+# CHECK-NEXT: Table:
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: ElemType: FUNCREF
+# CHECK-NEXT: Limits:
+# CHECK-NEXT: Minimum: 0x2
+# 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.func
+# CHECK-NEXT: Field: shared_func
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0, 1, 0 ]
+# 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: shared_func
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 5
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 6
+# CHECK-NEXT: - Type: ELEM
+# CHECK-NEXT: Segments:
+# CHECK-NEXT: - Offset:
+# CHECK-NEXT: Opcode: GLOBAL_GET
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: Functions: [ 3, 5 ]
+
+
+# DIS: <test_indirect_local_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i32.const 0
+# DIS-NEXT: i32.add
+# DIS-NEXT: call_indirect 0
+# DIS-NEXT: end
+
+# DIS: <test_indirect_shared_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: i64.const 12345
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i32.const 1
+# DIS-NEXT: i32.add
+# DIS-NEXT: call_indirect 1
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/test/wasm/shared-call-indirect64.s b/lld/test/wasm/shared-call-indirect64.s
new file mode 100644
index 00000000000000..a4c27a8ddf1970
--- /dev/null
+++ b/lld/test/wasm/shared-call-indirect64.s
@@ -0,0 +1,131 @@
+# 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_indirect_local_func_call,test_indirect_shared_func_call --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __table_base, i64, immutable
+
+test_indirect_local_func_call:
+ .functype test_indirect_local_func_call () -> ()
+ global.get __table_base
+ i64.const local_func@TBREL
+ i64.add
+ i32.wrap_i64 # Remove when table64 is supported
+ call_indirect () -> ()
+ end_function
+
+# Test table index relocation in code section for shared functions
+
+test_indirect_shared_func_call:
+ .functype test_indirect_shared_func_call () -> ()
+ i32.const 12345
+ global.get __table_base
+ i64.const shared_func@TBREL
+ i64.add
+ i32.wrap_i64 # Remove when table64 is supported
+ call_indirect (i32) -> (i32)
+ drop
+ end_function
+
+local_func:
+ .functype local_func () -> ()
+ end_function
+
+.globl shared_func
+shared_func:
+ .functype shared_func (i32) -> (i32)
+ local.get 0
+ end_function
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ call test_indirect_local_func_call
+ call test_indirect_shared_func_call
+ end_function
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 0
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 2
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+
+# CHECK: - 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: 0x0
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __indirect_function_table
+# CHECK-NEXT: Kind: TABLE
+# CHECK-NEXT: Table:
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: ElemType: FUNCREF
+# CHECK-NEXT: Limits:
+# CHECK-NEXT: Flags: [ IS_64 ]
+# CHECK-NEXT: Minimum: 0x2
+# 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.func
+# CHECK-NEXT: Field: shared_func
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I64
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0, 1, 0 ]
+# 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: shared_func
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 5
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 6
+# CHECK-NEXT: - Type: ELEM
+# CHECK-NEXT: Segments:
+# CHECK-NEXT: - Offset:
+# CHECK-NEXT: Opcode: GLOBAL_GET
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: Functions: [ 3, 5 ]
+
+
+# DIS: <test_indirect_local_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i64.const 0
+# DIS-NEXT: i64.add
+# DIS-NEXT: i32.wrap_i64
+# DIS-NEXT: call_indirect 0
+# DIS-NEXT: end
+
+# DIS: <test_indirect_shared_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: i32.const 12345
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i64.const 1
+# DIS-NEXT: i64.add
+# DIS-NEXT: i32.wrap_i64
+# DIS-NEXT: call_indirect 1
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index 6f33a4f28a9d09..bb9c53954d589d 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -102,13 +102,27 @@ void scanRelocations(InputChunk *chunk) {
switch (reloc.Type) {
case R_WASM_TABLE_INDEX_I32:
case R_WASM_TABLE_INDEX_I64:
+ // These relocations target the data section and are handled
+ // by `generateRelocationCode`. GOT accesses are handled below.
+ if (requiresGOTAccess(sym))
+ break;
+ out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ break;
case R_WASM_TABLE_INDEX_SLEB:
case R_WASM_TABLE_INDEX_SLEB64:
case R_WASM_TABLE_INDEX_REL_SLEB:
case R_WASM_TABLE_INDEX_REL_SLEB64:
- if (requiresGOTAccess(sym))
- break;
- out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ // These relocations target the code section and can only be resolved
+ // at linking time.
+ if (requiresGOTAccess(sym)) {
+ addGOTEntry(sym);
+ }
+ // The indirect call needs a table element that can only be added
+ // for defined functions.
+ if (sym->isDefined()) {
+ out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ }
+ // Unresolved symbols are handled below.
break;
case R_WASM_GLOBAL_INDEX_LEB:
case R_WASM_GLOBAL_INDEX_I32:
@@ -160,6 +174,15 @@ void scanRelocations(InputChunk *chunk) {
" cannot be used against symbol `" + toString(*sym) +
"`; recompile with -fPIC");
break;
+ case R_WASM_TABLE_INDEX_REL_SLEB:
+ case R_WASM_TABLE_INDEX_REL_SLEB64:
+ if (sym->isUndefined()) {
+ error(toString(file) + ": relocation " +
+ relocTypeToString(reloc.Type) +
+ " cannot be used against an undefined symbol `" +
+ toString(*sym) + "`");
+ }
+ break;
case R_WASM_TABLE_INDEX_I32:
case R_WASM_TABLE_INDEX_I64:
case R_WASM_MEMORY_ADDR_I32:
|
|
@llvm/pr-subscribers-lld-wasm Author: Luc Blaeser (luc-blaeser) ChangesFix Full diff: https://github.com/llvm/llvm-project/pull/104043.diff 4 Files Affected:
diff --git a/lld/test/wasm/call-indirect-external.s b/lld/test/wasm/call-indirect-external.s
new file mode 100644
index 00000000000000..e226b891442485
--- /dev/null
+++ b/lld/test/wasm/call-indirect-external.s
@@ -0,0 +1,27 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+# ERRUND: error: {{.*}}.o: relocation R_WASM_TABLE_INDEX_REL_SLEB cannot be used against an undefined symbol `external_func`
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --unresolved-symbols=report-all 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --warn-unresolved-symbols 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+# RUN: not wasm-ld --experimental-pic -shared %t.o -o /dev/null --unresolved-symbols=ignore-all 2>&1 | \
+# RUN: FileCheck -check-prefix=ERRUND %s
+
+.globaltype __table_base, i32, immutable
+
+.functype external_func () -> ()
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ global.get __table_base
+ i32.const external_func@TBREL
+ i32.add
+ call_indirect () -> ()
+ end_function
diff --git a/lld/test/wasm/shared-call-indirect.s b/lld/test/wasm/shared-call-indirect.s
new file mode 100644
index 00000000000000..d805670b8513c7
--- /dev/null
+++ b/lld/test/wasm/shared-call-indirect.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_indirect_local_func_call,test_indirect_shared_func_call --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __table_base, i32, immutable
+
+test_indirect_local_func_call:
+ .functype test_indirect_local_func_call () -> ()
+ global.get __table_base
+ i32.const local_func@TBREL
+ i32.add
+ call_indirect () -> ()
+ end_function
+
+# Test table index relocation in code section for shared functions
+
+test_indirect_shared_func_call:
+ .functype test_indirect_shared_func_call () -> ()
+ i64.const 12345
+ global.get __table_base
+ i32.const shared_func@TBREL
+ i32.add
+ call_indirect (i64) -> (i32)
+ drop
+ end_function
+
+local_func:
+ .functype local_func () -> ()
+ end_function
+
+.globl shared_func
+shared_func:
+ .functype shared_func (i64) -> (i32)
+ local.get 0
+ i32.wrap_i64
+ end_function
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ call test_indirect_local_func_call
+ call test_indirect_shared_func_call
+ end_function
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 0
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 2
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+
+# 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: __indirect_function_table
+# CHECK-NEXT: Kind: TABLE
+# CHECK-NEXT: Table:
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: ElemType: FUNCREF
+# CHECK-NEXT: Limits:
+# CHECK-NEXT: Minimum: 0x2
+# 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.func
+# CHECK-NEXT: Field: shared_func
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I32
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0, 1, 0 ]
+# 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: shared_func
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 5
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 6
+# CHECK-NEXT: - Type: ELEM
+# CHECK-NEXT: Segments:
+# CHECK-NEXT: - Offset:
+# CHECK-NEXT: Opcode: GLOBAL_GET
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: Functions: [ 3, 5 ]
+
+
+# DIS: <test_indirect_local_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i32.const 0
+# DIS-NEXT: i32.add
+# DIS-NEXT: call_indirect 0
+# DIS-NEXT: end
+
+# DIS: <test_indirect_shared_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: i64.const 12345
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i32.const 1
+# DIS-NEXT: i32.add
+# DIS-NEXT: call_indirect 1
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/test/wasm/shared-call-indirect64.s b/lld/test/wasm/shared-call-indirect64.s
new file mode 100644
index 00000000000000..a4c27a8ddf1970
--- /dev/null
+++ b/lld/test/wasm/shared-call-indirect64.s
@@ -0,0 +1,131 @@
+# 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_indirect_local_func_call,test_indirect_shared_func_call --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS
+
+.globaltype __table_base, i64, immutable
+
+test_indirect_local_func_call:
+ .functype test_indirect_local_func_call () -> ()
+ global.get __table_base
+ i64.const local_func@TBREL
+ i64.add
+ i32.wrap_i64 # Remove when table64 is supported
+ call_indirect () -> ()
+ end_function
+
+# Test table index relocation in code section for shared functions
+
+test_indirect_shared_func_call:
+ .functype test_indirect_shared_func_call () -> ()
+ i32.const 12345
+ global.get __table_base
+ i64.const shared_func@TBREL
+ i64.add
+ i32.wrap_i64 # Remove when table64 is supported
+ call_indirect (i32) -> (i32)
+ drop
+ end_function
+
+local_func:
+ .functype local_func () -> ()
+ end_function
+
+.globl shared_func
+shared_func:
+ .functype shared_func (i32) -> (i32)
+ local.get 0
+ end_function
+
+.globl _start
+_start:
+ .functype _start () -> ()
+ call test_indirect_local_func_call
+ call test_indirect_shared_func_call
+ end_function
+
+# CHECK: Sections:
+# CHECK-NEXT: - Type: CUSTOM
+# CHECK-NEXT: Name: dylink.0
+# CHECK-NEXT: MemorySize: 0
+# CHECK-NEXT: MemoryAlignment: 0
+# CHECK-NEXT: TableSize: 2
+# CHECK-NEXT: TableAlignment: 0
+# CHECK-NEXT: Needed: []
+# CHECK-NEXT: - Type: TYPE
+
+# CHECK: - 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: 0x0
+# CHECK-NEXT: - Module: env
+# CHECK-NEXT: Field: __indirect_function_table
+# CHECK-NEXT: Kind: TABLE
+# CHECK-NEXT: Table:
+# CHECK-NEXT: Index: 0
+# CHECK-NEXT: ElemType: FUNCREF
+# CHECK-NEXT: Limits:
+# CHECK-NEXT: Flags: [ IS_64 ]
+# CHECK-NEXT: Minimum: 0x2
+# 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.func
+# CHECK-NEXT: Field: shared_func
+# CHECK-NEXT: Kind: GLOBAL
+# CHECK-NEXT: GlobalType: I64
+# CHECK-NEXT: GlobalMutable: true
+# CHECK-NEXT: - Type: FUNCTION
+# CHECK-NEXT: FunctionTypes: [ 0, 0, 0, 0, 0, 1, 0 ]
+# 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: shared_func
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 5
+# CHECK-NEXT: - Name: _start
+# CHECK-NEXT: Kind: FUNCTION
+# CHECK-NEXT: Index: 6
+# CHECK-NEXT: - Type: ELEM
+# CHECK-NEXT: Segments:
+# CHECK-NEXT: - Offset:
+# CHECK-NEXT: Opcode: GLOBAL_GET
+# CHECK-NEXT: Index: 1
+# CHECK-NEXT: Functions: [ 3, 5 ]
+
+
+# DIS: <test_indirect_local_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i64.const 0
+# DIS-NEXT: i64.add
+# DIS-NEXT: i32.wrap_i64
+# DIS-NEXT: call_indirect 0
+# DIS-NEXT: end
+
+# DIS: <test_indirect_shared_func_call>:
+# DIS-EMPTY:
+# DIS-NEXT: i32.const 12345
+# DIS-NEXT: global.get 1
+# DIS-NEXT: i64.const 1
+# DIS-NEXT: i64.add
+# DIS-NEXT: i32.wrap_i64
+# DIS-NEXT: call_indirect 1
+# DIS-NEXT: drop
+# DIS-NEXT: end
diff --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index 6f33a4f28a9d09..bb9c53954d589d 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -102,13 +102,27 @@ void scanRelocations(InputChunk *chunk) {
switch (reloc.Type) {
case R_WASM_TABLE_INDEX_I32:
case R_WASM_TABLE_INDEX_I64:
+ // These relocations target the data section and are handled
+ // by `generateRelocationCode`. GOT accesses are handled below.
+ if (requiresGOTAccess(sym))
+ break;
+ out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ break;
case R_WASM_TABLE_INDEX_SLEB:
case R_WASM_TABLE_INDEX_SLEB64:
case R_WASM_TABLE_INDEX_REL_SLEB:
case R_WASM_TABLE_INDEX_REL_SLEB64:
- if (requiresGOTAccess(sym))
- break;
- out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ // These relocations target the code section and can only be resolved
+ // at linking time.
+ if (requiresGOTAccess(sym)) {
+ addGOTEntry(sym);
+ }
+ // The indirect call needs a table element that can only be added
+ // for defined functions.
+ if (sym->isDefined()) {
+ out.elemSec->addEntry(cast<FunctionSymbol>(sym));
+ }
+ // Unresolved symbols are handled below.
break;
case R_WASM_GLOBAL_INDEX_LEB:
case R_WASM_GLOBAL_INDEX_I32:
@@ -160,6 +174,15 @@ void scanRelocations(InputChunk *chunk) {
" cannot be used against symbol `" + toString(*sym) +
"`; recompile with -fPIC");
break;
+ case R_WASM_TABLE_INDEX_REL_SLEB:
+ case R_WASM_TABLE_INDEX_REL_SLEB64:
+ if (sym->isUndefined()) {
+ error(toString(file) + ": relocation " +
+ relocTypeToString(reloc.Type) +
+ " cannot be used against an undefined symbol `" +
+ toString(*sym) + "`");
+ }
+ break;
case R_WASM_TABLE_INDEX_I32:
case R_WASM_TABLE_INDEX_I64:
case R_WASM_MEMORY_ADDR_I32:
|
sbc100
reviewed
Aug 19, 2024
| @@ -0,0 +1,27 @@ | |||
| # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s | |||
There was a problem hiding this comment.
I wonder if we can give this test a better name? Perhaps bad-pic-relocations.s?
Could we split this test and the corresponding change into a separate PR?
Also, presumably the same error should occur for R_WASM_MEMORY_ADDR_REL_ relocations against undefined symbols?
BTW, how were you able to reproduce this error in the wild? e.g. did it require writing assembly?
There was a problem hiding this comment.
Thank you. I agree. This probably better addressed separately of this.
I have seen that bad pic relocations are only reported as link errors if UnresolvedPolicy::ReportError is enabled. Maybe undefined PIC relocations in code should always be reported as errors (even for the unresolved policy ImportDynamic, Warn, and Ignore), as long as the linker does not handle them. (If I understand correctly, they could be supported by rewriting the Wasm constant by using a GOT global.) I prepared #104926 that would make it more robust by raising errors in such unsupported cases.
The error message would indeed also apply to R_WASM_MEMORY_ADDR_REL_ relocation types. If I am not mistaken, similar to this fix, a GOT.mem would be needed for those relocations if referring to an exported data symbol. I also opened #104920 to fix this.
BTW: I noticed the issue when building the runtime system for our PL called Motoko with latest Rust (nigthly 2024-07-28) and wasm64.
There was a problem hiding this comment.
I'm curious how your project was able to generate MBREL or TBREL relocations types against undefined symbols. I don't think LLVM should ever do that.. I would expect it would require hand-written assembly. If you are seeing those in LLVM-generated code then that could point to an LLVM codegen bug.
There was a problem hiding this comment.
Actually, in my case, the symbols for those relocations were all defined, just not handled by the Wasm linker. The undefined case would only happen if a subset of o-files would be linked (with a permissive UnresolvedPolicy).
There was a problem hiding this comment.
Were the MBREL/TBREL relocations generated by LLVM? IIRC those should only be generated for symbols local to a given object file... but perhaps its actually DSO-local which is the deciding factor here.
Are you able to repoduce this issue with C code? If not then perhaps with llvm bitcode? Could you share an example that reproduces the issue?
There was a problem hiding this comment.
I am trying a small repro... It certainly involves DSO-local.
I keep you updated.
There was a problem hiding this comment.
I prepared https://github.com/luc-blaeser/wasm-ld-repro with some description (LLVM IR obtained from Rust).
There was a problem hiding this comment.
This is a smaller repro (however this one is hand-written LLVM IR): https://github.com/luc-blaeser/wasm-ld-repro/tree/luc%2Fmini-repro
There was a problem hiding this comment.
So it looks like this test was landed as part of #104926. Are there other parts of this PR that would still be good to land or should it closed?
|
@sbc100, thank you very much for the review. |
luc-blaeser
commented
Aug 21, 2024
Comment on lines
116
to
+117
| if (requiresGOTAccess(sym)) | ||
| break; | ||
| out.elemSec->addEntry(cast<FunctionSymbol>(sym)); | ||
| addGOTEntry(sym); |
There was a problem hiding this comment.
Maybe adding a GOT entry is not even needed for those PIC relocations (cf. discussion of #104920)?
This was referenced Aug 22, 2024
|
Ping |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix
wasm-ldto handle the relocation of table indices for shared functions in the code segment. The table element was not created for the corresponding table index. Moreover,GOT.funcwas missing for the corresponding function.