Skip to content

Commit 88599bf

Browse files
sbc100sbc100
committedAug 30, 2018
[WebAssembly] Be a little more conservative in WebAssemblyFixFunctionBitcasts
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 llvm-svn: 341010
1 parent 1bb6453 commit 88599bf

File tree

3 files changed

+95
-22
lines changed

3 files changed

+95
-22
lines changed
 

‎llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

+27-7
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,18 @@ static void FindUses(Value *V, Function &F,
107107
// I32 vs pointer type) then we don't create a wrapper at all (return nullptr
108108
// instead).
109109
//
110-
// If there is a type mismatch that would result in an invalid wasm module
111-
// being written then generate wrapper that contains unreachable (i.e. abort
112-
// at runtime). Such programs are deep into undefined behaviour territory,
110+
// If there is a type mismatch that we know would result in an invalid wasm
111+
// module then generate wrapper that contains unreachable (i.e. abort at
112+
// runtime). Such programs are deep into undefined behaviour territory,
113113
// but we choose to fail at runtime rather than generate and invalid module
114114
// or fail at compiler time. The reason we delay the error is that we want
115115
// to support the CMake which expects to be able to compile and link programs
116116
// that refer to functions with entirely incorrect signatures (this is how
117117
// CMake detects the existence of a function in a toolchain).
118+
//
119+
// For bitcasts that involve struct types we don't know at this stage if they
120+
// would be equivalent at the wasm level and so we can't know if we need to
121+
// generate a wrapper.
118122
static Function *CreateWrapper(Function *F, FunctionType *Ty) {
119123
Module *M = F->getParent();
120124

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

139+
Type *ExpectedRtnType = F->getFunctionType()->getReturnType();
140+
Type *RtnType = Ty->getReturnType();
141+
135142
if ((F->getFunctionType()->getNumParams() != Ty->getNumParams()) ||
136-
(F->getFunctionType()->isVarArg() != Ty->isVarArg()))
143+
(F->getFunctionType()->isVarArg() != Ty->isVarArg()) ||
144+
(ExpectedRtnType != RtnType))
137145
WrapperNeeded = true;
138146

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

162-
if (!TypeMismatch) {
174+
if (WrapperNeeded && !TypeMismatch) {
163175
for (; PI != PE; ++PI)
164176
Args.push_back(UndefValue::get(*PI));
165177
if (F->isVarArg())
@@ -173,10 +185,9 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
173185
// Determine what value to return.
174186
if (RtnType->isVoidTy()) {
175187
ReturnInst::Create(M->getContext(), BB);
176-
WrapperNeeded = true;
177188
} else if (ExpectedRtnType->isVoidTy()) {
189+
LLVM_DEBUG(dbgs() << "Creating dummy return: " << *RtnType << "\n");
178190
ReturnInst::Create(M->getContext(), UndefValue::get(RtnType), BB);
179-
WrapperNeeded = true;
180191
} else if (RtnType == ExpectedRtnType) {
181192
ReturnInst::Create(M->getContext(), Call, BB);
182193
} else if (CastInst::isBitOrNoopPointerCastable(ExpectedRtnType, RtnType,
@@ -185,6 +196,10 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
185196
CastInst::CreateBitOrPointerCast(Call, RtnType, "cast");
186197
BB->getInstList().push_back(Cast);
187198
ReturnInst::Create(M->getContext(), Cast, BB);
199+
} else if (RtnType->isStructTy() || ExpectedRtnType->isStructTy()) {
200+
LLVM_DEBUG(dbgs() << "CreateWrapper: struct return type in bitcast: "
201+
<< F->getName() << "\n");
202+
WrapperNeeded = false;
188203
} else {
189204
LLVM_DEBUG(dbgs() << "CreateWrapper: return type mismatch calling: "
190205
<< F->getName() << "\n");
@@ -195,6 +210,10 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
195210
}
196211

197212
if (TypeMismatch) {
213+
// Create a new wrapper that simply contains `unreachable`.
214+
Wrapper->eraseFromParent();
215+
Wrapper = Function::Create(Ty, Function::PrivateLinkage, F->getName() + "_bitcast_invalid", M);
216+
BasicBlock *BB = BasicBlock::Create(M->getContext(), "body", Wrapper);
198217
new UnreachableInst(M->getContext(), BB);
199218
Wrapper->setName(F->getName() + "_bitcast_invalid");
200219
} else if (!WrapperNeeded) {
@@ -203,6 +222,7 @@ static Function *CreateWrapper(Function *F, FunctionType *Ty) {
203222
Wrapper->eraseFromParent();
204223
return nullptr;
205224
}
225+
LLVM_DEBUG(dbgs() << "CreateWrapper: " << F->getName() << "\n");
206226
return Wrapper;
207227
}
208228

‎llvm/test/CodeGen/WebAssembly/function-bitcasts.ll

+40-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
66
target triple = "wasm32-unknown-unknown"
77

88
declare void @has_i32_arg(i32)
9+
declare void @has_struct_arg({i32})
910
declare i32 @has_i32_ret()
1011
declare void @vararg(...)
1112
declare void @plain(i32)
@@ -16,9 +17,10 @@ declare void @foo2()
1617
declare void @foo3()
1718

1819
; CHECK-LABEL: test:
19-
; CHECK-NEXT: call .Lhas_i32_arg_bitcast@FUNCTION{{$}}
20-
; CHECK-NEXT: call .Lhas_i32_arg_bitcast@FUNCTION{{$}}
20+
; CHECK-NEXT: call .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
21+
; CHECK-NEXT: call .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
2122
; CHECK-NEXT: call .Lhas_i32_ret_bitcast@FUNCTION{{$}}
23+
; CHECK-NEXT: i32.call $drop=, has_i32_ret@FUNCTION
2224
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0
2325
; CHECK-NEXT: call .Lfoo0_bitcast@FUNCTION, $pop[[L0]]{{$}}
2426
; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, 0
@@ -36,6 +38,7 @@ entry:
3638
call void bitcast (void (i32)* @has_i32_arg to void ()*)()
3739
call void bitcast (void (i32)* @has_i32_arg to void ()*)()
3840
call void bitcast (i32 ()* @has_i32_ret to void ()*)()
41+
call i32 bitcast (i32 ()* @has_i32_ret to i32 ()*)()
3942
call void bitcast (void ()* @foo0 to void (i32)*)(i32 0)
4043
%p = bitcast void ()* @foo0 to void (i32)*
4144
call void %p(i32 0)
@@ -51,6 +54,30 @@ entry:
5154
ret void
5255
}
5356

57+
; CHECK-LABEL: test_structs:
58+
; CHECK: call .Lhas_i32_arg_bitcast.1@FUNCTION, $pop{{[0-9]+}}, $pop{{[0-9]+$}}
59+
; CHECK: call .Lhas_i32_arg_bitcast@FUNCTION, $0, $pop2
60+
; CHECK: call .Lhas_struct_arg_bitcast@FUNCTION{{$}}
61+
define void @test_structs() {
62+
entry:
63+
call void bitcast (void (i32)* @has_i32_arg to void (i32, {i32})*)(i32 5, {i32} {i32 6})
64+
call {i32, i64} bitcast (void (i32)* @has_i32_arg to {i32, i64} (i32)*)(i32 7)
65+
call void bitcast (void ({i32})* @has_struct_arg to void ()*)()
66+
ret void
67+
}
68+
69+
; CHECK-LABEL: test_structs_unhandled:
70+
; CHECK: call has_struct_arg@FUNCTION, $pop{{[0-9]+$}}
71+
; CHECK: call has_struct_arg@FUNCTION, $pop{{[0-9]+$}}
72+
; CHECK: call has_i32_ret@FUNCTION, $pop{{[0-9]+$}}
73+
define void @test_structs_unhandled() {
74+
entry:
75+
call void @has_struct_arg({i32} {i32 3})
76+
call void bitcast (void ({i32})* @has_struct_arg to void ({i64})*)({i64} {i64 4})
77+
call {i32, i32} bitcast (i32 ()* @has_i32_ret to {i32, i32} ()*)()
78+
ret void
79+
}
80+
5481
; CHECK-LABEL: test_varargs:
5582
; CHECK: set_global
5683
; CHECK: i32.const $push[[L3:[0-9]+]]=, 0{{$}}
@@ -113,7 +140,7 @@ define void @test_argument() {
113140
; CHECK: i32.const $push[[L3:[0-9]+]]=, call_func@FUNCTION{{$}}
114141
; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, has_i32_arg@FUNCTION{{$}}
115142
; CHECK-NEXT: call "__invoke_void_i32()*"@FUNCTION, $pop[[L3]], $pop[[L2]]{{$}}
116-
; CHECK: i32.const $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast@FUNCTION{{$}}
143+
; CHECK: i32.const $push[[L4:[0-9]+]]=, .Lhas_i32_arg_bitcast.2@FUNCTION{{$}}
117144
; CHECK-NEXT: call __invoke_void@FUNCTION, $pop[[L4]]{{$}}
118145
declare i32 @personality(...)
119146
define void @test_invoke() personality i32 (...)* @personality {
@@ -139,6 +166,16 @@ end:
139166
}
140167

141168
; CHECK-LABEL: .Lhas_i32_arg_bitcast:
169+
; CHECK-NEXT: .param i32, i32
170+
; CHECK-NEXT: call has_i32_arg@FUNCTION, $1{{$}}
171+
; CHECK-NEXT: end_function
172+
173+
; CHECK-LABEL: .Lhas_i32_arg_bitcast.1:
174+
; CHECK-NEXT: .param i32, i32
175+
; CHECK-NEXT: call has_i32_arg@FUNCTION, $0{{$}}
176+
; CHECK-NEXT: end_function
177+
178+
; CHECK-LABEL: .Lhas_i32_arg_bitcast.2:
142179
; CHECK-NEXT: call has_i32_arg@FUNCTION, $0{{$}}
143180
; CHECK-NEXT: end_function
144181

‎llvm/test/CodeGen/WebAssembly/unsupported-function-bitcasts.ll

+28-12
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,31 @@ target triple = "wasm32-unknown-unknown"
99
declare i32 @has_i64_arg(i64)
1010
declare i32 @has_ptr_arg(i8*)
1111

12+
; CHECK-LABEL: test_invalid_rtn:
13+
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0{{$}}
14+
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_i64_arg_bitcast_invalid.2@FUNCTION, $pop[[L0]]{{$}}
15+
; CHECK-NEXT: drop $pop[[L1]]{{$}}
16+
; CHECK-NEXT: i64.const $push[[L0:[0-9]+]]=, 0{{$}}
17+
; CHECK-NEXT: i64.call $push[[L1:[0-9]+]]=, .Lhas_i64_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
18+
; CHECK-NEXT: drop $pop[[L1]]{{$}}
19+
; CHECK-NEXT: end_function
1220
define void @test_invalid_rtn() {
1321
entry:
1422
call i32 bitcast (i32 (i64)* @has_i64_arg to i32 (i32)*)(i32 0)
23+
call [1 x i64] bitcast (i32 (i64)* @has_i64_arg to [1 x i64] (i64)*)(i64 0)
1524
ret void
1625
}
17-
; CHECK-LABEL: test_invalid_rtn:
18-
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 0{{$}}
19-
; CHECK-NEXT: i32.call $push1=, .Lhas_i64_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
20-
; CHECK-NEXT: drop $pop1
21-
; CHECK-NEXT: end_function
2226

23-
define void @test_invalid_arg() {
24-
entry:
25-
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i8)*)(i8 2)
26-
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i32)*)(i32 2)
27-
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i64)*)(i64 3)
27+
; CHECK-LABEL: test_struct_rtn:
28+
; CHECK: call has_i64_arg@FUNCTION, $pop6, $pop0
29+
define void @test_struct_rtn() {
30+
call {i32, i32} bitcast (i32 (i64)* @has_i64_arg to {i32, i32} (i64)*)(i64 0)
2831
ret void
2932
}
3033

3134
; CHECK-LABEL: test_invalid_arg:
3235
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 2{{$}}
33-
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid.1@FUNCTION, $pop[[L0]]{{$}}
36+
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid.4@FUNCTION, $pop[[L0]]{{$}}
3437
; CHECK-NEXT: drop $pop[[L1]]{{$}}
3538
; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 2{{$}}
3639
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, has_ptr_arg@FUNCTION, $pop[[L0]]{{$}}
@@ -39,8 +42,21 @@ entry:
3942
; CHECK-NEXT: i32.call $push[[L1:[0-9]+]]=, .Lhas_ptr_arg_bitcast_invalid@FUNCTION, $pop[[L0]]{{$}}
4043
; CHECK-NEXT: drop $pop[[L1]]{{$}}
4144
; CHECK-NEXT: end_function
45+
define void @test_invalid_arg() {
46+
entry:
47+
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i8)*)(i8 2)
48+
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i32)*)(i32 2)
49+
call i32 bitcast (i32 (i8*)* @has_ptr_arg to i32 (i64)*)(i64 3)
50+
ret void
51+
}
4252

4353
; CHECK-LABEL: .Lhas_i64_arg_bitcast_invalid:
54+
; CHECK-NEXT: .param i64
55+
; CHECK-NEXT: .result i64
56+
; CHECK-NEXT: unreachable
57+
; CHECK-NEXT: end_function
58+
59+
; CHECK-LABEL: .Lhas_i64_arg_bitcast_invalid.2:
4460
; CHECK-NEXT: .param i32
4561
; CHECK-NEXT: .result i32
4662
; CHECK-NEXT: unreachable
@@ -52,7 +68,7 @@ entry:
5268
; CHECK-NEXT: unreachable
5369
; CHECK-NEXT: end_function
5470

55-
; CHECK-LABEL: .Lhas_ptr_arg_bitcast_invalid.1:
71+
; CHECK-LABEL: .Lhas_ptr_arg_bitcast_invalid.4:
5672
; CHECK-NEXT: .param i32
5773
; CHECK-NEXT: .result i32
5874
; CHECK-NEXT: unreachable

0 commit comments

Comments
 (0)
Please sign in to comment.