Skip to content

Commit c821845

Browse files
kripkenkripken
authored
fix asm2wasm wasm-only i64 switches with a large (>32 bit) offset #1109 (#1111)
1 parent cfe0fa1 commit c821845

8 files changed

Lines changed: 476 additions & 22 deletions

‎src/asm2wasm.h‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2539,6 +2539,12 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
25392539

25402540
auto top = allocator.alloc<Block>();
25412541
if (canSwitch) {
2542+
2543+
// we may need a break for the case where the condition doesn't match
2544+
// any of the cases. it should go to the default, if we have one, or
2545+
// outside if not
2546+
Break* breakWhenNotMatching = nullptr;
2547+
25422548
if (br->condition->type == i32) {
25432549
Binary* offsetor = allocator.alloc<Binary>();
25442550
offsetor->op = BinaryOp::SubInt32;
@@ -2554,7 +2560,28 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
25542560
offsetor->left = br->condition;
25552561
offsetor->right = builder.makeConst(Literal(int64_t(min)));
25562562
offsetor->type = i64;
2557-
br->condition = builder.makeUnary(UnaryOp::WrapInt64, offsetor); // TODO: check this fits in 32 bits
2563+
// the switch itself can be 32-bit, as the range is in a reasonable range. so after
2564+
// offsetting, we need to make sure there are no high bits, then we can just look
2565+
// at the lower 32 bits
2566+
auto temp = Builder::addVar(function, i64);
2567+
auto* block = builder.makeBlock();
2568+
block->list.push_back(builder.makeSetLocal(temp, offsetor));
2569+
// if high bits, we can break to the default (we'll fill in the name later)
2570+
breakWhenNotMatching = builder.makeBreak(Name(), nullptr,
2571+
builder.makeUnary(
2572+
UnaryOp::WrapInt64,
2573+
builder.makeBinary(BinaryOp::ShrUInt64,
2574+
builder.makeGetLocal(temp, i64),
2575+
builder.makeConst(Literal(int64_t(32)))
2576+
)
2577+
)
2578+
);
2579+
block->list.push_back(breakWhenNotMatching);
2580+
block->list.push_back(
2581+
builder.makeGetLocal(temp, i64)
2582+
);
2583+
block->finalize();
2584+
br->condition = builder.makeUnary(UnaryOp::WrapInt64, block);
25582585
}
25592586

25602587
top->list.push_back(br);
@@ -2595,6 +2622,9 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
25952622
if (br->default_.isNull()) {
25962623
br->default_ = top->name;
25972624
}
2625+
if (breakWhenNotMatching) {
2626+
breakWhenNotMatching->name = br->default_;
2627+
}
25982628
for (size_t i = 0; i < br->targets.size(); i++) {
25992629
if (br->targets[i].isNull()) br->targets[i] = br->default_;
26002630
}

‎test/wasm-only.asm.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ function asm(global, env, buffer) {
2626

2727
var _fabsf = env._fabsf;
2828
var do_i64 = env.do_i64;
29+
var abort = env.abort;
2930

3031
function loads() {
3132
var i = 0, f = fround(0), d = +0;
@@ -416,6 +417,28 @@ function asm(global, env, buffer) {
416417
return ($cond|0);
417418
}
418419

420+
function switch64_big_condition1($x) {
421+
$x = i64($x);
422+
switch (i64($x)) {
423+
case i64_const(0,2146435072): {
424+
abort();
425+
break;
426+
}
427+
default: {
428+
return;
429+
}
430+
}
431+
}
432+
function switch64_big_condition2($x) {
433+
$x = i64($x);
434+
switch (i64($x)) {
435+
case i64_const(0,2146435072): {
436+
abort();
437+
break;
438+
}
439+
}
440+
}
441+
419442
function keepAlive() {
420443
loads();
421444
stores();
@@ -431,6 +454,8 @@ function asm(global, env, buffer) {
431454
unreachable_leftovers(0, 0, 0);
432455
_memchr(0, 0, 0) | 0;
433456
switch64TOOMUCH(i64(0)) | 0;
457+
switch64_big_condition1(i64(0));
458+
switch64_big_condition2(i64(0));
434459
}
435460

436461
function __emscripten_dceable_type_decls() { // dce-able, but this defines the type of fabsf which has no other use

‎test/wasm-only.fromasm‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
(type $FUNCSIG$vdji (func (param f64 i64 i32)))
33
(type $FUNCSIG$j (func (result i64)))
44
(type $FUNCSIG$ff (func (param f32) (result f32)))
5+
(type $FUNCSIG$v (func))
56
(type $legaltype$illegalImport (func (param f64 i32 i32 i32)))
67
(type $legaltype$illegalImportResult (func (result i32)))
78
(type $legaltype$_fabsf (func (param f64) (result f64)))
@@ -10,6 +11,7 @@
1011
(import "env" "illegalImportResult" (func $illegalImportResult (result i64)))
1112
(import "env" "_fabsf" (func $_fabsf (param f32) (result f32)))
1213
(import "env" "do_i64" (func $do_i64 (result i64)))
14+
(import "env" "abort" (func $abort))
1315
(import "env" "illegalImport" (func $legalimport$illegalImport (param f64 i32 i32 i32)))
1416
(import "env" "illegalImportResult" (func $legalimport$illegalImportResult (result i32)))
1517
(import "env" "_fabsf" (func $legalimport$_fabsf (param f64) (result f64)))
@@ -510,14 +512,24 @@
510512
(block $switch-default
511513
(block $switch-case0
512514
(block $switch-case
513-
(br_table $switch-case0 $switch-default $switch-case $switch-default
515+
(br_if $switch-default
514516
(i32.wrap/i64
515-
(i64.sub
516-
(get_local $0)
517-
(i64.const 42949672965)
517+
(i64.shr_u
518+
(tee_local $0
519+
(i64.sub
520+
(get_local $0)
521+
(i64.const 42949672965)
522+
)
523+
)
524+
(i64.const 32)
518525
)
519526
)
520527
)
528+
(br_table $switch-case0 $switch-default $switch-case $switch-default
529+
(i32.wrap/i64
530+
(get_local $0)
531+
)
532+
)
521533
)
522534
(br $switch
523535
(i32.const 11000)
@@ -863,6 +875,32 @@
863875
(get_local $0)
864876
)
865877
)
878+
(func $switch64_big_condition1 (param $0 i64)
879+
(local $1 i64)
880+
(block $switch-default
881+
(block $switch-case
882+
(br_if $switch-default
883+
(i32.wrap/i64
884+
(i64.shr_u
885+
(tee_local $1
886+
(i64.sub
887+
(get_local $0)
888+
(i64.const 9218868437227405312)
889+
)
890+
)
891+
(i64.const 32)
892+
)
893+
)
894+
)
895+
(br_table $switch-case $switch-default
896+
(i32.wrap/i64
897+
(get_local $1)
898+
)
899+
)
900+
)
901+
(call $abort)
902+
)
903+
)
866904
(func $keepAlive
867905
(call $loads)
868906
(call $stores)
@@ -920,6 +958,12 @@
920958
(i64.const 0)
921959
)
922960
)
961+
(call $switch64_big_condition1
962+
(i64.const 0)
963+
)
964+
(call $switch64_big_condition1
965+
(i64.const 0)
966+
)
923967
)
924968
(func $legalstub$illegalParam (param $0 i32) (param $1 i32) (param $2 i32) (param $3 f64)
925969
(call $illegalParam

‎test/wasm-only.fromasm.clamp‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
(type $FUNCSIG$vdji (func (param f64 i64 i32)))
33
(type $FUNCSIG$j (func (result i64)))
44
(type $FUNCSIG$ff (func (param f32) (result f32)))
5+
(type $FUNCSIG$v (func))
56
(type $legaltype$illegalImport (func (param f64 i32 i32 i32)))
67
(type $legaltype$illegalImportResult (func (result i32)))
78
(type $legaltype$_fabsf (func (param f64) (result f64)))
@@ -10,6 +11,7 @@
1011
(import "env" "illegalImportResult" (func $illegalImportResult (result i64)))
1112
(import "env" "_fabsf" (func $_fabsf (param f32) (result f32)))
1213
(import "env" "do_i64" (func $do_i64 (result i64)))
14+
(import "env" "abort" (func $abort))
1315
(import "env" "illegalImport" (func $legalimport$illegalImport (param f64 i32 i32 i32)))
1416
(import "env" "illegalImportResult" (func $legalimport$illegalImportResult (result i32)))
1517
(import "env" "_fabsf" (func $legalimport$_fabsf (param f64) (result f64)))
@@ -510,14 +512,24 @@
510512
(block $switch-default
511513
(block $switch-case0
512514
(block $switch-case
513-
(br_table $switch-case0 $switch-default $switch-case $switch-default
515+
(br_if $switch-default
514516
(i32.wrap/i64
515-
(i64.sub
516-
(get_local $0)
517-
(i64.const 42949672965)
517+
(i64.shr_u
518+
(tee_local $0
519+
(i64.sub
520+
(get_local $0)
521+
(i64.const 42949672965)
522+
)
523+
)
524+
(i64.const 32)
518525
)
519526
)
520527
)
528+
(br_table $switch-case0 $switch-default $switch-case $switch-default
529+
(i32.wrap/i64
530+
(get_local $0)
531+
)
532+
)
521533
)
522534
(br $switch
523535
(i32.const 11000)
@@ -863,6 +875,32 @@
863875
(get_local $0)
864876
)
865877
)
878+
(func $switch64_big_condition1 (param $0 i64)
879+
(local $1 i64)
880+
(block $switch-default
881+
(block $switch-case
882+
(br_if $switch-default
883+
(i32.wrap/i64
884+
(i64.shr_u
885+
(tee_local $1
886+
(i64.sub
887+
(get_local $0)
888+
(i64.const 9218868437227405312)
889+
)
890+
)
891+
(i64.const 32)
892+
)
893+
)
894+
)
895+
(br_table $switch-case $switch-default
896+
(i32.wrap/i64
897+
(get_local $1)
898+
)
899+
)
900+
)
901+
(call $abort)
902+
)
903+
)
866904
(func $keepAlive
867905
(call $loads)
868906
(call $stores)
@@ -920,6 +958,12 @@
920958
(i64.const 0)
921959
)
922960
)
961+
(call $switch64_big_condition1
962+
(i64.const 0)
963+
)
964+
(call $switch64_big_condition1
965+
(i64.const 0)
966+
)
923967
)
924968
(func $legalstub$illegalParam (param $0 i32) (param $1 i32) (param $2 i32) (param $3 f64)
925969
(call $illegalParam

0 commit comments

Comments
 (0)