Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

Commit

Permalink
[WebAssembly] Be a little more conservative in WebAssemblyFixFunction…
Browse files Browse the repository at this point in the history
…Bitcasts

We don't have enough information to know if struct types being
bitcast will cause validation failures or not, so be conservative
and allow such cases to persist (fot now).

Fixes: https://bugs.llvm.org/show_bug.cgi?id=38711

Subscribers: dschuff, jgravelle-google, aheejin, sunfish, llvm-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@341010 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
sbc100 committed Aug 30, 2018
1 parent 6f7036e commit bdeb8b8
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 22 deletions.
34 changes: 27 additions & 7 deletions lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ static void FindUses(Value *V, Function &F,
// I32 vs pointer type) then we don't create a wrapper at all (return nullptr
// instead).
//
// If there is a type mismatch that would result in an invalid wasm module
// being written then generate wrapper that contains unreachable (i.e. abort
// at runtime). Such programs are deep into undefined behaviour territory,
// If there is a type mismatch that we know would result in an invalid wasm
// module then generate wrapper that contains unreachable (i.e. abort at
// runtime). Such programs are deep into undefined behaviour territory,
// but we choose to fail at runtime rather than generate and invalid module
// or fail at compiler time. The reason we delay the error is that we want
// to support the CMake which expects to be able to compile and link programs
// that refer to functions with entirely incorrect signatures (this is how
// CMake detects the existence of a function in a toolchain).
//
// For bitcasts that involve struct types we don't know at this stage if they
// would be equivalent at the wasm level and so we can't know if we need to
// generate a wrapper.
static Function *CreateWrapper(Function *F, FunctionType *Ty) {
Module *M = F->getParent();

Expand All @@ -132,8 +136,12 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
bool TypeMismatch = false;
bool WrapperNeeded = false;

Type *ExpectedRtnType = F->getFunctionType()->getReturnType();
Type *RtnType = Ty->getReturnType();

if ((F->getFunctionType()->getNumParams() != Ty->getNumParams()) ||
(F->getFunctionType()->isVarArg() != Ty->isVarArg()))
(F->getFunctionType()->isVarArg() != Ty->isVarArg()) ||
(ExpectedRtnType != RtnType))
WrapperNeeded = true;

for (; AI != AE && PI != PE; ++AI, ++PI) {
Expand All @@ -148,6 +156,10 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
CastInst::CreateBitOrPointerCast(AI, ParamType, "cast");
BB->getInstList().push_back(PtrCast);
Args.push_back(PtrCast);
} else if (ArgType->isStructTy() || ParamType->isStructTy()) {
LLVM_DEBUG(dbgs() << "CreateWrapper: struct param type in bitcast: "
<< F->getName() << "\n");
WrapperNeeded = false;
} else {
LLVM_DEBUG(dbgs() << "CreateWrapper: arg type mismatch calling: "
<< F->getName() << "\n");
Expand All @@ -159,7 +171,7 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
}
}

if (!TypeMismatch) {
if (WrapperNeeded && !TypeMismatch) {
for (; PI != PE; ++PI)
Args.push_back(UndefValue::get(*PI));
if (F->isVarArg())
Expand All @@ -173,10 +185,9 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
// Determine what value to return.
if (RtnType->isVoidTy()) {
ReturnInst::Create(M->getContext(), BB);
WrapperNeeded = true;
} else if (ExpectedRtnType->isVoidTy()) {
LLVM_DEBUG(dbgs() << "Creating dummy return: " << *RtnType << "\n");
ReturnInst::Create(M->getContext(), UndefValue::get(RtnType), BB);
WrapperNeeded = true;
} else if (RtnType == ExpectedRtnType) {
ReturnInst::Create(M->getContext(), Call, BB);
} else if (CastInst::isBitOrNoopPointerCastable(ExpectedRtnType, RtnType,
Expand All @@ -185,6 +196,10 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
CastInst::CreateBitOrPointerCast(Call, RtnType, "cast");
BB->getInstList().push_back(Cast);
ReturnInst::Create(M->getContext(), Cast, BB);
} else if (RtnType->isStructTy() || ExpectedRtnType->isStructTy()) {
LLVM_DEBUG(dbgs() << "CreateWrapper: struct return type in bitcast: "
<< F->getName() << "\n");
WrapperNeeded = false;
} else {
LLVM_DEBUG(dbgs() << "CreateWrapper: return type mismatch calling: "
<< F->getName() << "\n");
Expand All @@ -195,6 +210,10 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
}

if (TypeMismatch) {
// Create a new wrapper that simply contains `unreachable`.
Wrapper->eraseFromParent();
Wrapper = Function::Create(Ty, Function::PrivateLinkage, F->getName() + "_bitcast_invalid", M);
BasicBlock *BB = BasicBlock::Create(M->getContext(), "body", Wrapper);
new UnreachableInst(M->getContext(), BB);
Wrapper->setName(F->getName() + "_bitcast_invalid");
} else if (!WrapperNeeded) {
Expand All @@ -203,6 +222,7 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
Wrapper->eraseFromParent();
return nullptr;
}
LLVM_DEBUG(dbgs() << "CreateWrapper: " << F->getName() << "\n");
return Wrapper;
}

Expand Down
43 changes: 40 additions & 3 deletions test/CodeGen/WebAssembly/function-bitcasts.ll
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
target triple = "wasm32-unknown-unknown"

declare void @has_i32_arg(i32)
declare void @has_struct_arg({i32})
declare i32 @has_i32_ret()
declare void @vararg(...)
declare void @plain(i32)
Expand All @@ -16,9 +17,10 @@ declare void @foo2()
declare void @foo3()

; CHECK-LABEL: test:
; CHECK-NEXT: call .Lhas_i32_arg_bitcast@FUNCTION{{$}}
; CHECK-NEXT: call .Lhas_i32_arg_bitcast@FUNCTION{{$}}
; CHECK-NEXT: call .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
; CHECK-NEXT: call .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
; CHECK-NEXT: call .Lhas_i32_ret_bitcast@FUNCTION{{$}}
; CHECK-NEXT: i32.call $drop=, has_i32_ret@FUNCTION
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0
; CHECK-NEXT: call .Lfoo0_bitcast@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, 0
Expand All @@ -36,6 +38,7 @@ entry:
call void bitcast (void (i32)* @has_i32_arg to void ()*)()
call void bitcast (void (i32)* @has_i32_arg to void ()*)()
call void bitcast (i32 ()* @has_i32_ret to void ()*)()
call i32 bitcast (i32 ()* @has_i32_ret to i32 ()*)()
call void bitcast (void ()* @foo0 to void (i32)*)(i32 0)
%p = bitcast void ()* @foo0 to void (i32)*
call void %p(i32 0)
Expand All @@ -51,6 +54,30 @@ entry:
ret void
}

; CHECK-LABEL: test_structs:
; CHECK: call .Lhas_i32_arg_bitcast.1@FUNCTION, $pop{{[0-9]+}}, $pop{{[0-9]+$}}
; CHECK: call .Lhas_i32_arg_bitcast@FUNCTION, $0, $pop2
; CHECK: call .Lhas_struct_arg_bitcast@FUNCTION{{$}}
define void @test_structs() {
entry:
call void bitcast (void (i32)* @has_i32_arg to void (i32, {i32})*)(i32 5, {i32} {i32 6})
call {i32, i64} bitcast (void (i32)* @has_i32_arg to {i32, i64} (i32)*)(i32 7)
call void bitcast (void ({i32})* @has_struct_arg to void ()*)()
ret void
}

; CHECK-LABEL: test_structs_unhandled:
; CHECK: call has_struct_arg@FUNCTION, $pop{{[0-9]+$}}
; CHECK: call has_struct_arg@FUNCTION, $pop{{[0-9]+$}}
; CHECK: call has_i32_ret@FUNCTION, $pop{{[0-9]+$}}
define void @test_structs_unhandled() {
entry:
call void @has_struct_arg({i32} {i32 3})
call void bitcast (void ({i32})* @has_struct_arg to void ({i64})*)({i64} {i64 4})
call {i32, i32} bitcast (i32 ()* @has_i32_ret to {i32, i32} ()*)()
ret void
}

; CHECK-LABEL: test_varargs:
; CHECK: set_global
; CHECK: i32.const $push[[L3:[0-9]+]]=, 0{{$}}
Expand Down Expand Up @@ -113,7 +140,7 @@ define void @test_argument() {
; CHECK: i32.const $push[[L3:[0-9]+]]=, call_func@FUNCTION{{$}}
; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, has_i32_arg@FUNCTION{{$}}
; CHECK-NEXT: call "__invoke_void_i32()*"@FUNCTION, $pop[[L3]], $pop[[L2]]{{$}}
; CHECK: i32.const $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast@FUNCTION{{$}}
; CHECK: i32.const $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
; CHECK-NEXT: call __invoke_void@FUNCTION, $pop[[L4]]{{$}}
declare i32 @personality(...)
define void @test_invoke() personality i32 (...)* @personality {
Expand All @@ -139,6 +166,16 @@ end:
}

; CHECK-LABEL: .Lhas_i32_arg_bitcast:
; CHECK-NEXT: .param i32, i32
; CHECK-NEXT: call has_i32_arg@FUNCTION, $1{{$}}
; CHECK-NEXT: end_function

; CHECK-LABEL: .Lhas_i32_arg_bitcast.1:
; CHECK-NEXT: .param i32, i32
; CHECK-NEXT: call has_i32_arg@FUNCTION, $0{{$}}
; CHECK-NEXT: end_function

; CHECK-LABEL: .Lhas_i32_arg_bitcast.2:
; CHECK-NEXT: call has_i32_arg@FUNCTION, $0{{$}}
; CHECK-NEXT: end_function

Expand Down
40 changes: 28 additions & 12 deletions test/CodeGen/WebAssembly/unsupported-function-bitcasts.ll
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,31 @@ target triple = "wasm32-unknown-unknown"
declare i32 @has_i64_arg(i64)
declare i32 @has_ptr_arg(i8*)

; CHECK-LABEL: test_invalid_rtn:
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0{{$}}
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_i64_arg_bitcast_invalid.2@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: drop $pop[[L1]]{{$}}
; CHECK-NEXT: i64.const $push[[L0:[0-9]+]]=, 0{{$}}
; CHECK-NEXT: i64.call $push[[L1:[0-9]+]]=, .Lhas_i64_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: drop $pop[[L1]]{{$}}
; CHECK-NEXT: end_function
define void @test_invalid_rtn() {
entry:
call i32 bitcast (i32 (i64)* @has_i64_arg to i32 (i32)*)(i32 0)
call [1 x i64] bitcast (i32 (i64)* @has_i64_arg to [1 x i64] (i64)*)(i64 0)
ret void
}
; CHECK-LABEL: test_invalid_rtn:
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0{{$}}
; CHECK-NEXT: i32.call $push1=, .Lhas_i64_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: drop $pop1
; CHECK-NEXT: end_function

define void @test_invalid_arg() {
entry:
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i8)*)(i8 2)
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i32)*)(i32 2)
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i64)*)(i64 3)
; CHECK-LABEL: test_struct_rtn:
; CHECK: call has_i64_arg@FUNCTION, $pop6, $pop0
define void @test_struct_rtn() {
call {i32, i32} bitcast (i32 (i64)* @has_i64_arg to {i32, i32} (i64)*)(i64 0)
ret void
}

; CHECK-LABEL: test_invalid_arg:
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 2{{$}}
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid.1@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid.4@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: drop $pop[[L1]]{{$}}
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 2{{$}}
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, has_ptr_arg@FUNCTION, $pop[[L0]]{{$}}
Expand All @@ -39,8 +42,21 @@ entry:
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
; CHECK-NEXT: drop $pop[[L1]]{{$}}
; CHECK-NEXT: end_function
define void @test_invalid_arg() {
entry:
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i8)*)(i8 2)
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i32)*)(i32 2)
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i64)*)(i64 3)
ret void
}

; CHECK-LABEL: .Lhas_i64_arg_bitcast_invalid:
; CHECK-NEXT: .param i64
; CHECK-NEXT: .result i64
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function

; CHECK-LABEL: .Lhas_i64_arg_bitcast_invalid.2:
; CHECK-NEXT: .param i32
; CHECK-NEXT: .result i32
; CHECK-NEXT: unreachable
Expand All @@ -52,7 +68,7 @@ entry:
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function

; CHECK-LABEL: .Lhas_ptr_arg_bitcast_invalid.1:
; CHECK-LABEL: .Lhas_ptr_arg_bitcast_invalid.4:
; CHECK-NEXT: .param i32
; CHECK-NEXT: .result i32
; CHECK-NEXT: unreachable
Expand Down

0 comments on commit bdeb8b8

Please sign in to comment.