Skip to content

Commit

Permalink
[lld][WebAssembly] Fix handling of comdat functions in init array.
Browse files Browse the repository at this point in the history
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: emscripten-core/emscripten#8981

Differential Revision: https://reviews.llvm.org/D64872

llvm-svn: 366358
  • Loading branch information
sbc100 committed Jul 17, 2019
1 parent 48f5a43 commit accad76
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 18 deletions.
4 changes: 2 additions & 2 deletions lld/test/wasm/Inputs/comdat1.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions lld/test/wasm/Inputs/comdat2.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 16 additions & 8 deletions lld/test/wasm/comdats.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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:
Expand Down
10 changes: 4 additions & 6 deletions lld/wasm/InputFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<DefinedFunction>(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<DefinedData>(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: {
Expand Down
1 change: 1 addition & 0 deletions lld/wasm/Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}
}
Expand Down

0 comments on commit accad76

Please sign in to comment.