Skip to content

Commit bc46556

Browse files
kripkenkripken
authored
Consistently handle possible traps in all cases (#1062)
* consistently handle possible traps in asm.js ffi results * consistently handle possible traps in i64/float conversions
1 parent 17f3d39 commit bc46556

11 files changed

Lines changed: 305 additions & 55 deletions

‎src/asm2wasm.h‎

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
#include "ast_utils.h"
3535
#include "wasm-builder.h"
3636
#include "wasm-emscripten.h"
37-
#include "wasm-printing.h"
3837
#include "wasm-module-building.h"
3938

4039
namespace wasm {
@@ -788,7 +787,7 @@ class Asm2WasmBuilder {
788787
}
789788

790789
// Some conversions might trap, so emit them safely if necessary
791-
Expression* makeTrappingFloatToInt(bool signed_, Expression* value) {
790+
Expression* makeTrappingFloatToInt32(bool signed_, Expression* value) {
792791
if (trapMode == TrapMode::Allow) {
793792
auto ret = allocator.alloc<Unary>();
794793
ret->value = value;
@@ -879,6 +878,76 @@ class Asm2WasmBuilder {
879878
return ret;
880879
}
881880

881+
Expression* makeTrappingFloatToInt64(bool signed_, Expression* value) {
882+
if (trapMode == TrapMode::Allow) {
883+
auto ret = allocator.alloc<Unary>();
884+
ret->value = value;
885+
bool isF64 = ret->value->type == f64;
886+
if (signed_) {
887+
ret->op = isF64 ? TruncSFloat64ToInt64 : TruncSFloat32ToInt64;
888+
} else {
889+
ret->op = isF64 ? TruncUFloat64ToInt64 : TruncUFloat32ToInt64;
890+
}
891+
ret->type = WasmType::i64;
892+
return ret;
893+
}
894+
// WebAssembly traps on float-to-int overflows, but asm.js wouldn't, so we must do something
895+
// First, normalize input to f64
896+
auto input = value;
897+
if (input->type == f32) {
898+
auto conv = allocator.alloc<Unary>();
899+
conv->op = PromoteFloat32;
900+
conv->value = input;
901+
conv->type = WasmType::f64;
902+
input = conv;
903+
}
904+
// There is no "JS" way to handle this, as no i64s in JS, so always clamp if we don't allow traps
905+
Call *ret = allocator.alloc<Call>();
906+
ret->target = F64_TO_INT64;
907+
ret->operands.push_back(input);
908+
ret->type = i64;
909+
static bool added = false;
910+
if (!added) {
911+
added = true;
912+
auto func = new Function;
913+
func->name = ret->target;
914+
func->params.push_back(f64);
915+
func->result = i64;
916+
func->body = builder.makeUnary(TruncSFloat64ToInt64,
917+
builder.makeGetLocal(0, f64)
918+
);
919+
// too small
920+
func->body = builder.makeIf(
921+
builder.makeBinary(LeFloat64,
922+
builder.makeGetLocal(0, f64),
923+
builder.makeConst(Literal(double(std::numeric_limits<int64_t>::min()) - 1))
924+
),
925+
builder.makeConst(Literal(int64_t(std::numeric_limits<int64_t>::min()))),
926+
func->body
927+
);
928+
// too big
929+
func->body = builder.makeIf(
930+
builder.makeBinary(GeFloat64,
931+
builder.makeGetLocal(0, f64),
932+
builder.makeConst(Literal(double(std::numeric_limits<int64_t>::max()) + 1))
933+
),
934+
builder.makeConst(Literal(int64_t(std::numeric_limits<int64_t>::min()))), // NB: min here as well. anything out of range => to the min
935+
func->body
936+
);
937+
// nan
938+
func->body = builder.makeIf(
939+
builder.makeBinary(NeFloat64,
940+
builder.makeGetLocal(0, f64),
941+
builder.makeGetLocal(0, f64)
942+
),
943+
builder.makeConst(Literal(int64_t(std::numeric_limits<int64_t>::min()))), // NB: min here as well. anything invalid => to the min
944+
func->body
945+
);
946+
wasm.addFunction(func);
947+
}
948+
return ret;
949+
}
950+
882951
Expression* truncateToInt32(Expression* value) {
883952
if (value->type == i64) return builder.makeUnary(UnaryOp::WrapInt64, value);
884953
// either i32, or a call_import whose type we don't know yet (but would be legalized to i32 anyhow)
@@ -1302,10 +1371,12 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
13021371
}
13031372
auto importResult = getModule()->getFunctionType(getModule()->getImport(curr->target)->functionType)->result;
13041373
if (curr->type != importResult) {
1374+
auto old = curr->type;
1375+
curr->type = importResult;
13051376
if (importResult == f64) {
13061377
// we use a JS f64 value which is the most general, and convert to it
1307-
switch (curr->type) {
1308-
case i32: replaceCurrent(parent->builder.makeUnary(TruncSFloat64ToInt32, curr)); break;
1378+
switch (old) {
1379+
case i32: replaceCurrent(parent->makeTrappingFloatToInt32(true /* signed, asm.js ffi */, curr)); break;
13091380
case f32: replaceCurrent(parent->builder.makeUnary(DemoteFloat64, curr)); break;
13101381
case none: {
13111382
// this function returns a value, but we are not using it, so it must be dropped.
@@ -1315,11 +1386,10 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
13151386
default: WASM_UNREACHABLE();
13161387
}
13171388
} else {
1318-
assert(curr->type == none);
1389+
assert(old == none);
13191390
// we don't want a return value here, but the import does provide one
13201391
// autodrop will do that for us.
13211392
}
1322-
curr->type = importResult;
13231393
}
13241394
}
13251395

@@ -1836,7 +1906,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
18361906
// ~, might be ~~ as a coercion or just a not
18371907
if (ast[2]->isArray(UNARY_PREFIX) && ast[2][1] == B_NOT) {
18381908
// if we have an unsigned coercion on us, it is an unsigned op
1839-
return makeTrappingFloatToInt(!isParentUnsignedCoercion(astStackHelper.getParent()), process(ast[2][2]));
1909+
return makeTrappingFloatToInt32(!isParentUnsignedCoercion(astStackHelper.getParent()), process(ast[2][2]));
18401910
}
18411911
// no bitwise unary not, so do xor with -1
18421912
auto ret = allocator.alloc<Binary>();
@@ -2038,10 +2108,8 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
20382108
if (name == I64_S2D) return builder.makeUnary(UnaryOp::ConvertSInt64ToFloat64, value);
20392109
if (name == I64_U2F) return builder.makeUnary(UnaryOp::ConvertUInt64ToFloat32, value);
20402110
if (name == I64_U2D) return builder.makeUnary(UnaryOp::ConvertUInt64ToFloat64, value);
2041-
if (name == I64_F2S) return builder.makeUnary(UnaryOp::TruncSFloat32ToInt64, value);
2042-
if (name == I64_D2S) return builder.makeUnary(UnaryOp::TruncSFloat64ToInt64, value);
2043-
if (name == I64_F2U) return builder.makeUnary(UnaryOp::TruncUFloat32ToInt64, value);
2044-
if (name == I64_D2U) return builder.makeUnary(UnaryOp::TruncUFloat64ToInt64, value);
2111+
if (name == I64_F2S || name == I64_D2S) return makeTrappingFloatToInt64(true /* signed */, value);
2112+
if (name == I64_F2U || name == I64_D2U) return makeTrappingFloatToInt64(false /* unsigned */, value);
20452113
if (name == I64_BC2D) return builder.makeUnary(UnaryOp::ReinterpretInt64, value);
20462114
if (name == I64_BC2I) return builder.makeUnary(UnaryOp::ReinterpretFloat64, value);
20472115
if (name == I64_CTTZ) return builder.makeUnary(UnaryOp::CtzInt64, value);

‎src/asmjs/shared-constants.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ cashew::IString GLOBAL("global"),
4040
ASM2WASM("asm2wasm"),
4141
F64_REM("f64-rem"),
4242
F64_TO_INT("f64-to-int"),
43+
F64_TO_INT64("f64-to-int64"),
4344
I32S_DIV("i32s-div"),
4445
I32U_DIV("i32u-div"),
4546
I32S_REM("i32s-rem"),

‎src/asmjs/shared-constants.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ extern cashew::IString GLOBAL,
4343
ASM2WASM,
4444
F64_REM,
4545
F64_TO_INT,
46+
F64_TO_INT64,
4647
I32S_DIV,
4748
I32U_DIV,
4849
I32S_REM,

‎test/unit.fromasm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@
613613
(drop
614614
(if (result i32)
615615
(call $return_int)
616-
(i32.trunc_s/f64
616+
(call $f64-to-int
617617
(call $abort
618618
(f64.convert_s/i32
619619
(i32.const 5)

‎test/unit.fromasm.clamp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@
637637
(drop
638638
(if (result i32)
639639
(call $return_int)
640-
(i32.trunc_s/f64
640+
(call $f64-to-int
641641
(call $abort
642642
(f64.convert_s/i32
643643
(i32.const 5)

‎test/unit.fromasm.clamp.no-opts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,7 @@
10971097
(set_local $x
10981098
(if (result i32)
10991099
(call $return_int)
1100-
(i32.trunc_s/f64
1100+
(call $f64-to-int
11011101
(call $abort
11021102
(f64.convert_s/i32
11031103
(i32.const 5)

‎test/unit.fromasm.no-opts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@
10731073
(set_local $x
10741074
(if (result i32)
10751075
(call $return_int)
1076-
(i32.trunc_s/f64
1076+
(call $f64-to-int
10771077
(call $abort
10781078
(f64.convert_s/i32
10791079
(i32.const 5)

‎test/wasm-only.fromasm‎

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,38 @@
191191
)
192192
)
193193
)
194+
(func $f64-to-int64 (param $0 f64) (result i64)
195+
(if (result i64)
196+
(f64.ne
197+
(get_local $0)
198+
(get_local $0)
199+
)
200+
(i64.const -9223372036854775808)
201+
(if (result i64)
202+
(f64.ge
203+
(get_local $0)
204+
(f64.const 9223372036854775808)
205+
)
206+
(i64.const -9223372036854775808)
207+
(if (result i64)
208+
(f64.le
209+
(get_local $0)
210+
(f64.const -9223372036854775808)
211+
)
212+
(i64.const -9223372036854775808)
213+
(i64.trunc_s/f64
214+
(get_local $0)
215+
)
216+
)
217+
)
218+
)
219+
)
194220
(func $test64
195221
(local $0 i64)
196222
(local $1 i64)
197223
(local $2 i32)
224+
(local $3 f32)
225+
(local $4 f64)
198226
(drop
199227
(call $i64s-rem
200228
(call $i64u-rem
@@ -203,58 +231,90 @@
203231
(i64.mul
204232
(i64.sub
205233
(i64.add
206-
(tee_local $0
234+
(tee_local $1
207235
(i64.const 128849018897)
208236
)
209237
(i64.const 100)
210238
)
211-
(get_local $0)
239+
(get_local $1)
212240
)
213-
(get_local $0)
241+
(get_local $1)
214242
)
215-
(get_local $0)
243+
(get_local $1)
216244
)
217-
(get_local $0)
245+
(get_local $1)
218246
)
219-
(get_local $0)
247+
(get_local $1)
220248
)
221-
(get_local $0)
249+
(get_local $1)
222250
)
223251
)
224252
(i64.store
225253
(i32.const 120)
226-
(tee_local $1
254+
(tee_local $0
227255
(i64.load
228256
(i32.const 120)
229257
)
230258
)
231259
)
232260
(i64.store
233261
(i32.const 120)
234-
(get_local $1)
262+
(get_local $0)
235263
)
236264
(i64.store align=2
237265
(i32.const 120)
238-
(get_local $1)
266+
(get_local $0)
239267
)
240268
(i64.store align=4
241269
(i32.const 120)
242-
(get_local $1)
270+
(get_local $0)
243271
)
244272
(i64.store
245273
(i32.const 120)
246-
(get_local $1)
274+
(get_local $0)
247275
)
248276
(set_local $2
249277
(i32.wrap/i64
250-
(get_local $1)
278+
(get_local $0)
251279
)
252280
)
253-
(set_local $1
281+
(set_local $0
254282
(i64.extend_u/i32
255283
(get_local $2)
256284
)
257285
)
286+
(drop
287+
(call $f64-to-int64
288+
(f64.promote/f32
289+
(tee_local $3
290+
(f32.convert_u/i64
291+
(get_local $0)
292+
)
293+
)
294+
)
295+
)
296+
)
297+
(drop
298+
(call $f64-to-int64
299+
(tee_local $4
300+
(f64.convert_u/i64
301+
(get_local $0)
302+
)
303+
)
304+
)
305+
)
306+
(drop
307+
(call $f64-to-int64
308+
(f64.promote/f32
309+
(get_local $3)
310+
)
311+
)
312+
)
313+
(drop
314+
(call $f64-to-int64
315+
(get_local $4)
316+
)
317+
)
258318
)
259319
(func $imports (result i64)
260320
(call $legalfunc$illegalImport

0 commit comments

Comments
 (0)