Skip to content

Commit

Permalink
[lld][WebAssembly] Revert moving of data relocations to start function
Browse files Browse the repository at this point in the history 
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: emscripten-core/emscripten#17150

Differential Revision: https://reviews.llvm.org/D127333
  • Loading branch information
@sbc100
sbc100 committed Jun 10, 2022
1 parent 2a40267 commit 457f38a
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 52 deletions.
20 changes: 13 additions & 7 deletions lld/test/wasm/data-segments.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: {{.*}}
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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

Expand Down
19 changes: 11 additions & 8 deletions lld/test/wasm/pie.ll
View file
Original file line number Diff line number Diff line change
@@ -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"

Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lld/test/wasm/shared-weak-symbols.s
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 12 additions & 36 deletions lld/wasm/Writer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1289,36 +1275,25 @@ 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);
} else if (WasmSym::initMemory) {
WasmSym::startFunction = WasmSym::initMemory;
} else if (WasmSym::applyGlobalRelocs) {
WasmSym::startFunction = WasmSym::applyGlobalRelocs;
} else if (WasmSym::applyDataRelocs) {
WasmSym::startFunction = WasmSym::applyDataRelocs;
}
}

Expand Down Expand Up @@ -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.
Expand All @@ -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");
Expand Down

0 comments on commit 457f38a

Please sign in to comment.