[lld] 457f38a - [lld][WebAssembly] Revert moving of data relocations to start function
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 9 17:49:58 PDT 2022
Author: Sam Clegg
Date: 2022-06-09T17:49:35-07:00
New Revision: 457f38a7b0ef7d845832738c0a8add469b31bbcc
URL: https://github.com/llvm/llvm-project/commit/457f38a7b0ef7d845832738c0a8add469b31bbcc
DIFF: https://github.com/llvm/llvm-project/commit/457f38a7b0ef7d845832738c0a8add469b31bbcc.diff
LOG: [lld][WebAssembly] Revert moving of data relocations to start function
Back in https://reviews.llvm.org/D117412 we moved the application of
data reloctions to the wasm start function.
However, because the dynamic linker doesn't know the final addresses
at module instantiation time, this proved to be too early and the
relocations could be applied with the wrong values.
Fixes: https://github.com/emscripten-core/emscripten/issues/17150
Differential Revision: https://reviews.llvm.org/D127333
Added:
Modified:
lld/test/wasm/data-segments.ll
lld/test/wasm/pie.ll
lld/test/wasm/shared-weak-symbols.s
lld/wasm/Writer.cpp
Removed:
################################################################################
diff --git a/lld/test/wasm/data-segments.ll b/lld/test/wasm/data-segments.ll
index bfba82ee29cf2..11fa97fd98e82 100644
--- a/lld/test/wasm/data-segments.ll
+++ b/lld/test/wasm/data-segments.ll
@@ -20,22 +20,22 @@
; atomics, bulk memory, shared memory => passive segments
; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem.o -o %t.atomics.bulk-mem.wasm
; RUN: obj2yaml %t.atomics.bulk-mem.wasm | FileCheck %s --check-prefix PASSIVE
-; RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.atomics.bulk-mem.wasm | FileCheck %s --check-prefixes DIS,NOPIC-DIS -DPTR=i32
+; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.atomics.bulk-mem.wasm | FileCheck %s --check-prefixes DIS,NOPIC-DIS -DPTR=i32
; atomics, bulk memory, shared memory, wasm64 => passive segments
; RUN: wasm-ld -mwasm64 -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem64.o -o %t.atomics.bulk-mem64.wasm
; RUN: obj2yaml %t.atomics.bulk-mem64.wasm | FileCheck %s --check-prefix PASSIVE
-; RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.atomics.bulk-mem64.wasm | FileCheck %s --check-prefixes DIS,NOPIC-DIS -DPTR=i64
+; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.atomics.bulk-mem64.wasm | FileCheck %s --check-prefixes DIS,NOPIC-DIS -DPTR=i64
; Also test in combination with PIC/pie
; RUN: wasm-ld --experimental-pic -pie -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem.pic.o -o %t.pic.wasm
; RUN: obj2yaml %t.pic.wasm | FileCheck %s --check-prefixes PASSIVE-PIC,PASSIVE32-PIC
-; RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.pic.wasm | FileCheck %s --check-prefixes DIS,PIC-DIS -DPTR=i32
+; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.pic.wasm | FileCheck %s --check-prefixes DIS,PIC-DIS -DPTR=i32
; Also test in combination with PIC/pie + wasm64
; RUN: wasm-ld -mwasm64 --experimental-pic -pie -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem.pic-mem64.o -o %t.pic-mem64.wasm
; RUN: obj2yaml %t.pic-mem64.wasm | FileCheck %s --check-prefixes PASSIVE-PIC,PASSIVE64-PIC
-; RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.pic-mem64.wasm | FileCheck %s --check-prefixes DIS,PIC-DIS -DPTR=i64
+; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.pic-mem64.wasm | FileCheck %s --check-prefixes DIS,PIC-DIS -DPTR=i64
@a = hidden global [6 x i8] c"hello\00", align 1
@b = hidden global [8 x i8] c"goodbye\00", align 1
@@ -120,7 +120,7 @@
; PASSIVE-PIC-NEXT: Functions:
; PASSIVE-PIC-NEXT: - Index: 0
; PASSIVE-PIC-NEXT: Locals: []
-; PASSIVE-PIC-NEXT: Body: 0B
+; PASSIVE-PIC-NEXT: Body: 10030B
; PASSIVE-PIC-NEXT: - Index: 1
; PASSIVE-PIC-NEXT: Locals: []
; PASSIVE-PIC-NEXT: Body: {{.*}}
@@ -156,6 +156,14 @@
; PASSIVE-PIC-NEXT: - Index: 3
; PASSIVE-PIC-NEXT: Name: __wasm_apply_data_relocs
+; In PIC mode __wasm_call_ctors contains a call to __wasm_apply_data_relocs
+; In non-PIC mode __wasm_call_ctors is an emtpy function since there are
+; no data relocations.
+; DIS-LABEL: <__wasm_call_ctors>:
+; DIS-EMPTY:
+; PIC-DIS-NEXT: call 3
+; DIS-NEXT: end
+
; DIS-LABEL: <__wasm_init_memory>:
; PIC-DIS: .local [[PTR]]
@@ -216,8 +224,6 @@
; DIS-NEXT: i32.const 10000
; DIS-NEXT: memory.fill 0
-; PIC-DIS-NEXT: call 3
-
; NOPIC-DIS-NEXT: [[PTR]].const 11064
; PIC-DIS-NEXT: local.get 0
diff --git a/lld/test/wasm/pie.ll b/lld/test/wasm/pie.ll
index 71c7d1fb5ce53..c781611569944 100644
--- a/lld/test/wasm/pie.ll
+++ b/lld/test/wasm/pie.ll
@@ -1,7 +1,7 @@
; RUN: llc -relocation-model=pic -mattr=+mutable-globals -filetype=obj %s -o %t.o
; RUN: wasm-ld --no-gc-sections --experimental-pic -pie -o %t.wasm %t.o
; RUN: obj2yaml %t.wasm | FileCheck %s
-; RUN: llvm-objdump --disassemble-symbols=__wasm_start --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DISASSEM
+; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DISASSEM
target triple = "wasm32-unknown-emscripten"
@@ -70,7 +70,7 @@ declare void @external_func()
; CHECK-NEXT: GlobalMutable: false
; CHECK: - Type: START
-; CHECK-NEXT: StartFunction: 4
+; CHECK-NEXT: StartFunction: 3
; CHECK: - Type: CUSTOM
; CHECK-NEXT: Name: name
@@ -84,11 +84,15 @@ declare void @external_func()
; CHECK-NEXT: - Index: 3
; CHECK-NEXT: Name: __wasm_apply_global_relocs
; CHECK-NEXT: - Index: 4
-; CHECK-NEXT: Name: __wasm_start
-
-; DISASSEM: <__wasm_start>:
+; CHECK-NEXT: Name: foo
+; CHECK-NEXT: - Index: 5
+; CHECK-NEXT: Name: get_data_address
+; CHECK-NEXT: - Index: 6
+; CHECK-NEXT: Name: _start
+; CHECK-NEXT: GlobalNames:
+
+; DISASSEM: <__wasm_call_ctors>:
; DISASSEM-EMPTY:
-; DISASSEM-NEXT: call 3
; DISASSEM-NEXT: call 2
; DISASSEM-NEXT: end
@@ -126,8 +130,7 @@ declare void @external_func()
; (global.get[0x23] 0x1 i32.const[0x41] 0x0C i32.add[0x6A] end[0x0b])
; EXTENDED-CONST-NEXT: Body: 2301410C6A0B
-; EXTENDED-CONST: - Type: START
-; EXTENDED-CONST-NEXT: StartFunction: 2
+; EXTENDED-CONST-NOT: - Type: START
; EXTENDED-CONST: FunctionNames:
; EXTENDED-CONST-NEXT: - Index: 0
diff --git a/lld/test/wasm/shared-weak-symbols.s b/lld/test/wasm/shared-weak-symbols.s
index f22aaa5431539..c74d599602949 100644
--- a/lld/test/wasm/shared-weak-symbols.s
+++ b/lld/test/wasm/shared-weak-symbols.s
@@ -68,7 +68,7 @@ call_weak:
# CHECK-NEXT: - Name: call_weak
# CHECK-NEXT: Kind: FUNCTION
# CHECK-NEXT: Index: 5
-# CHECK-NEXT: - Type: START
+# CHECK-NEXT: - Type: CODE
# CHECK: - Type: CUSTOM
# CHECK-NEXT: Name: name
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 4da324351df28..c9307d73ecaf5 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -1037,16 +1037,10 @@ void Writer::createSyntheticInitFunctions() {
WasmSym::applyGlobalRelocs->markLive();
}
- int startCount = 0;
- if (WasmSym::applyGlobalRelocs)
- startCount++;
- if (WasmSym::WasmSym::initMemory || WasmSym::applyDataRelocs)
- startCount++;
-
// If there is only one start function we can just use that function
// itself as the Wasm start function, otherwise we need to synthesize
// a new function to call them in sequence.
- if (startCount > 1) {
+ if (WasmSym::applyGlobalRelocs && WasmSym::initMemory) {
WasmSym::startFunction = symtab->addSyntheticFunction(
"__wasm_start", WASM_SYMBOL_VISIBILITY_HIDDEN,
make<SyntheticFunction>(nullSignature, "__wasm_start"));
@@ -1224,14 +1218,6 @@ void Writer::createInitMemoryFunction() {
}
}
- // Memory init is now complete. Apply data relocation if there
- // are any.
- if (WasmSym::applyDataRelocs) {
- writeU8(os, WASM_OPCODE_CALL, "CALL");
- writeUleb128(os, WasmSym::applyDataRelocs->getFunctionIndex(),
- "function index");
- }
-
if (config->sharedMemory) {
// Set flag to 2 to mark end of initialization
writeGetFlagAddress();
@@ -1289,27 +1275,18 @@ void Writer::createInitMemoryFunction() {
void Writer::createStartFunction() {
// If the start function exists when we have more than one function to call.
- if (WasmSym::startFunction) {
+ if (WasmSym::initMemory && WasmSym::applyGlobalRelocs) {
+ assert(WasmSym::startFunction);
std::string bodyContent;
{
raw_string_ostream os(bodyContent);
writeUleb128(os, 0, "num locals");
- if (WasmSym::applyGlobalRelocs) {
- writeU8(os, WASM_OPCODE_CALL, "CALL");
- writeUleb128(os, WasmSym::applyGlobalRelocs->getFunctionIndex(),
- "function index");
- }
- if (WasmSym::initMemory) {
- writeU8(os, WASM_OPCODE_CALL, "CALL");
- writeUleb128(os, WasmSym::initMemory->getFunctionIndex(),
- "function index");
- } else if (WasmSym::applyDataRelocs) {
- // When initMemory is present it calls applyDataRelocs. If not,
- // we must call it directly.
- writeU8(os, WASM_OPCODE_CALL, "CALL");
- writeUleb128(os, WasmSym::applyDataRelocs->getFunctionIndex(),
- "function index");
- }
+ writeU8(os, WASM_OPCODE_CALL, "CALL");
+ writeUleb128(os, WasmSym::applyGlobalRelocs->getFunctionIndex(),
+ "function index");
+ writeU8(os, WASM_OPCODE_CALL, "CALL");
+ writeUleb128(os, WasmSym::initMemory->getFunctionIndex(),
+ "function index");
writeU8(os, WASM_OPCODE_END, "END");
}
createFunction(WasmSym::startFunction, bodyContent);
@@ -1317,8 +1294,6 @@ void Writer::createStartFunction() {
WasmSym::startFunction = WasmSym::initMemory;
} else if (WasmSym::applyGlobalRelocs) {
WasmSym::startFunction = WasmSym::applyGlobalRelocs;
- } else if (WasmSym::applyDataRelocs) {
- WasmSym::startFunction = WasmSym::applyDataRelocs;
}
}
@@ -1381,7 +1356,8 @@ void Writer::createCallCtorsFunction() {
// If __wasm_call_ctors isn't referenced, there aren't any ctors, and we
// aren't calling `__wasm_apply_data_relocs` for Emscripten-style PIC, don't
// define the `__wasm_call_ctors` function.
- if (!WasmSym::callCtors->isLive() && initFunctions.empty())
+ if (!WasmSym::callCtors->isLive() && !WasmSym::applyDataRelocs &&
+ initFunctions.empty())
return;
// First write the body's contents to a string.
@@ -1390,7 +1366,7 @@ void Writer::createCallCtorsFunction() {
raw_string_ostream os(bodyContent);
writeUleb128(os, 0, "num locals");
- if (WasmSym::applyDataRelocs && !WasmSym::initMemory) {
+ if (WasmSym::applyDataRelocs) {
writeU8(os, WASM_OPCODE_CALL, "CALL");
writeUleb128(os, WasmSym::applyDataRelocs->getFunctionIndex(),
"function index");
More information about the llvm-commits
mailing list