From accad76c1425492e700178f557573e661d0c0afa Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 17 Jul 2019 18:43:36 +0000 Subject: [PATCH] [lld][WebAssembly] Fix handling of comdat functions in init array. When hidden symbols are discarded by comdat rules we still want to create a local defined symbol, otherwise `Symbol::isDiscarded()` relies on begin able to check `getChunk->discarded`. This is a followup on rL362769. The comdat.ll test was previously GC'ing the `__wasm_call_ctors` functions so `do_init` was not actually being included in the link. Once that function was included in triggered the crash bug that this change addresses. Fixes: https://github.com/emscripten-core/emscripten/issues/8981 Differential Revision: https://reviews.llvm.org/D64872 llvm-svn: 366358 --- lld/test/wasm/Inputs/comdat1.ll | 4 ++-- lld/test/wasm/Inputs/comdat2.ll | 4 ++-- lld/test/wasm/comdats.ll | 24 ++++++++++++++++-------- lld/wasm/InputFiles.cpp | 10 ++++------ lld/wasm/Writer.cpp | 1 + 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lld/test/wasm/Inputs/comdat1.ll b/lld/test/wasm/Inputs/comdat1.ll index 9e5bd1a6de9c0e..34f7f161d10cda 100644 --- a/lld/test/wasm/Inputs/comdat1.ll +++ b/lld/test/wasm/Inputs/comdat1.ll @@ -12,8 +12,8 @@ define internal void @do_init() comdat($foo) { ret void } -@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void -()*, i8* } { i32 65535, void ()* @do_init, i8* null }] +%0 = type { i32, void ()*, i8* } +@llvm.global_ctors = appending global [1 x %0 ] [%0 { i32 65535, void ()* @do_init, i8* null }] ; Everything above this is part of the `foo` comdat group diff --git a/lld/test/wasm/Inputs/comdat2.ll b/lld/test/wasm/Inputs/comdat2.ll index 0618d55aa2f5dc..b1478ad7c32100 100644 --- a/lld/test/wasm/Inputs/comdat2.ll +++ b/lld/test/wasm/Inputs/comdat2.ll @@ -12,8 +12,8 @@ define internal void @do_init() comdat($foo) { ret void } -@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void -()*, i8* } { i32 65535, void ()* @do_init, i8* null }] +%0 = type { i32, void ()*, i8* } +@llvm.global_ctors = appending global [1 x %0] [ %0 { i32 65535, void ()* @do_init, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @constantData, i32 0, i32 0) }] ; Everything above this is part of the `foo` comdat group diff --git a/lld/test/wasm/comdats.ll b/lld/test/wasm/comdats.ll index 602cd313afe873..8fc301e9a10e02 100644 --- a/lld/test/wasm/comdats.ll +++ b/lld/test/wasm/comdats.ll @@ -6,10 +6,12 @@ target triple = "wasm32-unknown-unknown" +declare void @__wasm_call_ctors() declare i32 @comdatFn() define void @_start() { entry: + call void @__wasm_call_ctors() %call = call i32 @comdatFn() ret void } @@ -35,38 +37,44 @@ entry: ; CHECK-NEXT: Index: 0 ; CHECK-NEXT: - Name: _start ; CHECK-NEXT: Kind: FUNCTION -; CHECK-NEXT: Index: 0 +; CHECK-NEXT: Index: 1 ; CHECK-NEXT: - Name: comdatFn ; CHECK-NEXT: Kind: FUNCTION -; CHECK-NEXT: Index: 1 +; CHECK-NEXT: Index: 2 ; CHECK-NEXT: - Name: constantData ; CHECK-NEXT: Kind: GLOBAL ; CHECK-NEXT: Index: 1 ; CHECK-NEXT: - Name: callComdatFn1 ; CHECK-NEXT: Kind: FUNCTION -; CHECK-NEXT: Index: 2 +; CHECK-NEXT: Index: 4 ; CHECK-NEXT: - Name: callComdatFn2 ; CHECK-NEXT: Kind: FUNCTION -; CHECK-NEXT: Index: 3 +; CHECK-NEXT: Index: 5 ; CHECK-NEXT: - Type: ELEM ; CHECK-NEXT: Segments: ; CHECK-NEXT: - Offset: ; CHECK-NEXT: Opcode: I32_CONST ; CHECK-NEXT: Value: 1 -; CHECK-NEXT: Functions: [ 1 ] +; CHECK-NEXT: Functions: [ 2 ] ; CHECK-NEXT: - Type: CODE ; CHECK-NEXT: Functions: ; CHECK-NEXT: - Index: 0 ; CHECK-NEXT: Locals: -; CHECK-NEXT: Body: 1081808080001A0B +; CHECK-NEXT: Body: 10030B ; CHECK-NEXT: - Index: 1 ; CHECK-NEXT: Locals: -; CHECK-NEXT: Body: 4180888080000B +; CHECK-NEXT: Body: 1080808080001082808080001A0B ; CHECK-NEXT: - Index: 2 ; CHECK-NEXT: Locals: -; CHECK-NEXT: Body: 4181808080000B +; CHECK-NEXT: Body: 4180888080000B ; CHECK-NEXT: - Index: 3 ; CHECK-NEXT: Locals: +; CHECK-NEXT: Body: 0B +; CHECK-NEXT: - Index: 4 +; CHECK-NEXT: Locals: +; CHECK-NEXT: Body: 4181808080000B +; CHECK-NEXT: - Index: 5 +; CHECK-NEXT: Locals: ; CHECK-NEXT: Body: 4181808080000B ; CHECK-NEXT: - Type: DATA ; CHECK-NEXT: Segments: diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp index b4945469f931cf..33ae3325c7472c 100644 --- a/lld/wasm/InputFiles.cpp +++ b/lld/wasm/InputFiles.cpp @@ -380,22 +380,20 @@ Symbol *ObjFile::createDefined(const WasmSymbol &sym) { case WASM_SYMBOL_TYPE_FUNCTION: { InputFunction *func = functions[sym.Info.ElementIndex - wasmObj->getNumImportedFunctions()]; - if (func->discarded) - return nullptr; if (sym.isBindingLocal()) return make(name, flags, this, func); + if (func->discarded) + return nullptr; return symtab->addDefinedFunction(name, flags, this, func); } case WASM_SYMBOL_TYPE_DATA: { InputSegment *seg = segments[sym.Info.DataRef.Segment]; - if (seg->discarded) - return nullptr; - uint32_t offset = sym.Info.DataRef.Offset; uint32_t size = sym.Info.DataRef.Size; - if (sym.isBindingLocal()) return make(name, flags, this, seg, offset, size); + if (seg->discarded) + return nullptr; return symtab->addDefinedData(name, flags, this, seg, offset, size); } case WASM_SYMBOL_TYPE_GLOBAL: { diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index 23a63edee7cca1..4ad91ab1117135 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -821,6 +821,7 @@ void Writer::calculateInitFunctions() { assert(sym->isLive()); if (*sym->signature != WasmSignature{{}, {}}) error("invalid signature for init func: " + toString(*sym)); + LLVM_DEBUG(dbgs() << "initFunctions: " << toString(*sym) << "\n"); initFunctions.emplace_back(WasmInitEntry{sym, f.Priority}); } }