Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

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

git-svn-id: https://llvm.org/svn/llvm-project/lld/trunk@366358 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
sbc100 committed Jul 17, 2019
1 parent 000acde commit 043712f
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 18 deletions.
4 changes: 2 additions & 2 deletions 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 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 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 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 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 043712f

Please sign in to comment.