From 837a17a351307dc215c3b352cba154209c814532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Sat, 27 Apr 2024 13:06:44 +0200 Subject: [PATCH] Fix a bug with unreachable control flow in IRBuilder When branches target control flow structures other than blocks or loops, the IRBuilder wraps those control flow structures with an extra block for the branches to target in Binaryen IR. When the control flow structure is unreachable because all its bodies are unreachable, the wrapper block may still need to have a non-unreachable type if it is targeted by branches. This is achieved by tracking whether the wrapper block will be targeted by any branches and use the control flow structure's original, non-unreachable type if so. However, this was not properly tracked when moving into the `else` branch of an `if` or the `catch`/`cath_all` handlers of a `try` block. --- src/wasm-ir-builder.h | 33 ++++--- src/wasm/wasm-ir-builder.cpp | 11 ++- test/lit/wat-kitchen-sink.wast | 160 ++++++++++++++++++++++++--------- 3 files changed, 148 insertions(+), 56 deletions(-) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index addbd1a30dc..3c4a52bea91 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -311,9 +311,11 @@ class IRBuilder : public UnifiedExpressionVisitor> { ScopeCtx() : scope(NoScope{}) {} ScopeCtx(Scope scope) : scope(scope) {} - ScopeCtx(Scope scope, Name label) : scope(scope), label(label) {} - ScopeCtx(Scope scope, Name label, Name branchLabel) - : scope(scope), label(label), branchLabel(branchLabel) {} + ScopeCtx(Scope scope, Name label, bool labelUsed) + : scope(scope), label(label), labelUsed(labelUsed) {} + ScopeCtx(Scope scope, Name label, bool labelUsed, Name branchLabel) + : scope(scope), label(label), branchLabel(branchLabel), + labelUsed(labelUsed) {} static ScopeCtx makeFunc(Function* func) { return ScopeCtx(FuncScope{func}); @@ -324,20 +326,29 @@ class IRBuilder : public UnifiedExpressionVisitor> { static ScopeCtx makeIf(If* iff, Name originalLabel = {}) { return ScopeCtx(IfScope{iff, originalLabel}); } - static ScopeCtx makeElse(If* iff, Name originalLabel, Name label) { - return ScopeCtx(ElseScope{iff, originalLabel}, label); + static ScopeCtx + makeElse(If* iff, Name originalLabel, Name label, bool labelUsed) { + return ScopeCtx(ElseScope{iff, originalLabel}, label, labelUsed); } static ScopeCtx makeLoop(Loop* loop) { return ScopeCtx(LoopScope{loop}); } static ScopeCtx makeTry(Try* tryy, Name originalLabel = {}) { return ScopeCtx(TryScope{tryy, originalLabel}); } - static ScopeCtx - makeCatch(Try* tryy, Name originalLabel, Name label, Name branchLabel) { - return ScopeCtx(CatchScope{tryy, originalLabel}, label, branchLabel); + static ScopeCtx makeCatch(Try* tryy, + Name originalLabel, + Name label, + bool labelUsed, + Name branchLabel) { + return ScopeCtx( + CatchScope{tryy, originalLabel}, label, labelUsed, branchLabel); } - static ScopeCtx - makeCatchAll(Try* tryy, Name originalLabel, Name label, Name branchLabel) { - return ScopeCtx(CatchAllScope{tryy, originalLabel}, label, branchLabel); + static ScopeCtx makeCatchAll(Try* tryy, + Name originalLabel, + Name label, + bool labelUsed, + Name branchLabel) { + return ScopeCtx( + CatchAllScope{tryy, originalLabel}, label, labelUsed, branchLabel); } static ScopeCtx makeTryTable(TryTable* trytable, Name originalLabel = {}) { return ScopeCtx(TryTableScope{trytable, originalLabel}); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index ccd2ee77ad7..6322a578656 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -810,10 +810,11 @@ Result<> IRBuilder::visitElse() { } auto originalLabel = scope.getOriginalLabel(); auto label = scope.label; + auto labelUsed = scope.labelUsed; auto expr = finishScope(); CHECK_ERR(expr); iff->ifTrue = *expr; - pushScope(ScopeCtx::makeElse(iff, originalLabel, label)); + pushScope(ScopeCtx::makeElse(iff, originalLabel, label, labelUsed)); return Ok{}; } @@ -830,6 +831,7 @@ Result<> IRBuilder::visitCatch(Name tag) { } auto originalLabel = scope.getOriginalLabel(); auto label = scope.label; + auto labelUsed = scope.labelUsed; auto branchLabel = scope.branchLabel; auto expr = finishScope(); CHECK_ERR(expr); @@ -839,7 +841,8 @@ Result<> IRBuilder::visitCatch(Name tag) { tryy->catchBodies.push_back(*expr); } tryy->catchTags.push_back(tag); - pushScope(ScopeCtx::makeCatch(tryy, originalLabel, label, branchLabel)); + pushScope( + ScopeCtx::makeCatch(tryy, originalLabel, label, labelUsed, branchLabel)); // Push a pop for the exception payload. auto params = wasm.getTag(tag)->sig.params; if (params != Type::none) { @@ -861,6 +864,7 @@ Result<> IRBuilder::visitCatchAll() { } auto originalLabel = scope.getOriginalLabel(); auto label = scope.label; + auto labelUsed = scope.labelUsed; auto branchLabel = scope.branchLabel; auto expr = finishScope(); CHECK_ERR(expr); @@ -869,7 +873,8 @@ Result<> IRBuilder::visitCatchAll() { } else { tryy->catchBodies.push_back(*expr); } - pushScope(ScopeCtx::makeCatchAll(tryy, originalLabel, label, branchLabel)); + pushScope( + ScopeCtx::makeCatchAll(tryy, originalLabel, label, labelUsed, branchLabel)); return Ok{}; } diff --git a/test/lit/wat-kitchen-sink.wast b/test/lit/wat-kitchen-sink.wast index 772d3f5df2d..8e4d26d4f36 100644 --- a/test/lit/wat-kitchen-sink.wast +++ b/test/lit/wat-kitchen-sink.wast @@ -62,49 +62,49 @@ ;; CHECK: (type $24 (func (param v128 i32) (result v128))) - ;; CHECK: (type $25 (func (param i32) (result i32 i64))) + ;; CHECK: (type $25 (func (param i32) (result i32))) + + ;; CHECK: (type $26 (func (param i32) (result i32 i64))) ;; CHECK: (type $packed-i16 (array (mut i16))) ;; CHECK: (type $any-array (array (mut anyref))) - ;; CHECK: (type $28 (func (param stringref))) + ;; CHECK: (type $29 (func (param stringref))) - ;; CHECK: (type $29 (func (param stringref stringref) (result i32))) + ;; CHECK: (type $30 (func (param stringref stringref) (result i32))) - ;; CHECK: (type $30 (func (param i64 v128) (result v128))) + ;; CHECK: (type $31 (func (param i64 v128) (result v128))) - ;; CHECK: (type $31 (func (param i64 v128))) + ;; CHECK: (type $32 (func (param i64 v128))) ;; CHECK: (type $cont-bind-before-func (func (param i32 i64 i32 i64) (result f32))) ;; CHECK: (type $cont-bind-before (cont $cont-bind-before-func)) - ;; CHECK: (type $34 (func (result structref arrayref))) - - ;; CHECK: (type $35 (func (result arrayref structref))) + ;; CHECK: (type $35 (func (result structref arrayref))) - ;; CHECK: (type $36 (func (result i32 i64 (ref null $simple-cont)))) + ;; CHECK: (type $36 (func (result arrayref structref))) - ;; CHECK: (type $37 (func (param i32 i32))) + ;; CHECK: (type $37 (func (result i32 i64 (ref null $simple-cont)))) - ;; CHECK: (type $38 (func (param exnref))) + ;; CHECK: (type $38 (func (param i32 i32))) - ;; CHECK: (type $39 (func (result anyref anyref))) + ;; CHECK: (type $39 (func (param exnref))) - ;; CHECK: (type $40 (func (param i32 i32 f64 f64))) + ;; CHECK: (type $40 (func (result anyref anyref))) - ;; CHECK: (type $41 (func (param i64))) + ;; CHECK: (type $41 (func (param i32 i32 f64 f64))) - ;; CHECK: (type $42 (func (param v128) (result i32))) + ;; CHECK: (type $42 (func (param i64))) - ;; CHECK: (type $43 (func (param v128 v128) (result v128))) + ;; CHECK: (type $43 (func (param v128) (result i32))) - ;; CHECK: (type $44 (func (param v128 v128 v128) (result v128))) + ;; CHECK: (type $44 (func (param v128 v128) (result v128))) - ;; CHECK: (type $45 (func (param i32 i32 i64 i64))) + ;; CHECK: (type $45 (func (param v128 v128 v128) (result v128))) - ;; CHECK: (type $46 (func (param i32) (result i32))) + ;; CHECK: (type $46 (func (param i32 i32 i64 i64))) ;; CHECK: (type $47 (func (param i64) (result i32 i64))) @@ -950,7 +950,7 @@ drop ) - ;; CHECK: (func $locals (type $37) (param $0 i32) (param $x i32) + ;; CHECK: (func $locals (type $38) (param $0 i32) (param $x i32) ;; CHECK-NEXT: (local $2 i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (drop @@ -2567,7 +2567,7 @@ ) ) - ;; CHECK: (func $try-table-throw-ref (type $38) (param $0 exnref) + ;; CHECK: (func $try-table-throw-ref (type $39) (param $0 exnref) ;; CHECK-NEXT: (throw_ref ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) @@ -2963,10 +2963,10 @@ end ) - ;; CHECK: (func $br-table-multivalue-glb (type $39) (result anyref anyref) - ;; CHECK-NEXT: (block $a (type $35) (result arrayref structref) + ;; CHECK: (func $br-table-multivalue-glb (type $40) (result anyref anyref) + ;; CHECK-NEXT: (block $a (type $36) (result arrayref structref) ;; CHECK-NEXT: (return - ;; CHECK-NEXT: (block $b (type $34) (result structref arrayref) + ;; CHECK-NEXT: (block $b (type $35) (result structref arrayref) ;; CHECK-NEXT: (br_table $a $b ;; CHECK-NEXT: (tuple.make 2 ;; CHECK-NEXT: (ref.null none) @@ -3013,7 +3013,7 @@ end ) - ;; CHECK: (func $binary (type $40) (param $0 i32) (param $1 i32) (param $2 f64) (param $3 f64) + ;; CHECK: (func $binary (type $41) (param $0 i32) (param $1 i32) (param $2 f64) (param $3 f64) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.add ;; CHECK-NEXT: (local.get $0) @@ -3038,7 +3038,7 @@ drop ) - ;; CHECK: (func $unary (type $41) (param $0 i64) + ;; CHECK: (func $unary (type $42) (param $0 i64) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i64.eqz ;; CHECK-NEXT: (local.get $0) @@ -3387,7 +3387,7 @@ drop ) - ;; CHECK: (func $simd-extract (type $42) (param $0 v128) (result i32) + ;; CHECK: (func $simd-extract (type $43) (param $0 v128) (result i32) ;; CHECK-NEXT: (i32x4.extract_lane 3 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) @@ -3409,7 +3409,7 @@ i32x4.replace_lane 2 ) - ;; CHECK: (func $simd-shuffle (type $43) (param $0 v128) (param $1 v128) (result v128) + ;; CHECK: (func $simd-shuffle (type $44) (param $0 v128) (param $1 v128) (result v128) ;; CHECK-NEXT: (i8x16.shuffle 0 1 2 3 4 5 6 7 16 17 18 19 20 21 22 23 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: (local.get $1) @@ -3421,7 +3421,7 @@ i8x16.shuffle 0 1 2 3 4 5 6 7 16 17 18 19 20 21 22 23 ) - ;; CHECK: (func $simd-ternary (type $44) (param $0 v128) (param $1 v128) (param $2 v128) (result v128) + ;; CHECK: (func $simd-ternary (type $45) (param $0 v128) (param $1 v128) (param $2 v128) (result v128) ;; CHECK-NEXT: (v128.bitselect ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: (local.get $1) @@ -3531,7 +3531,7 @@ data.drop $passive ) - ;; CHECK: (func $memory-copy (type $45) (param $0 i32) (param $1 i32) (param $2 i64) (param $3 i64) + ;; CHECK: (func $memory-copy (type $46) (param $0 i32) (param $1 i32) (param $2 i64) (param $3 i64) ;; CHECK-NEXT: (memory.copy $mimport$0 $mimport$0 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: (local.get $1) @@ -3602,7 +3602,7 @@ return ) - ;; CHECK: (func $return-one (type $46) (param $0 i32) (result i32) + ;; CHECK: (func $return-one (type $25) (param $0 i32) (result i32) ;; CHECK-NEXT: (return ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) @@ -3640,7 +3640,7 @@ return ) - ;; CHECK: (func $return-two-second-unreachable (type $25) (param $0 i32) (result i32 i64) + ;; CHECK: (func $return-two-second-unreachable (type $26) (param $0 i32) (result i32 i64) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) @@ -3654,7 +3654,7 @@ return ) - ;; CHECK: (func $return-two-second-unreachable-tuple (type $25) (param $0 i32) (result i32 i64) + ;; CHECK: (func $return-two-second-unreachable-tuple (type $26) (param $0 i32) (result i32 i64) ;; CHECK-NEXT: (return ;; CHECK-NEXT: (tuple.make 2 ;; CHECK-NEXT: (local.get $0) @@ -4513,7 +4513,7 @@ string.const "\00\00\00" ) - ;; CHECK: (func $string-measure (type $28) (param $0 stringref) + ;; CHECK: (func $string-measure (type $29) (param $0 stringref) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (string.measure_wtf8 ;; CHECK-NEXT: (local.get $0) @@ -4570,7 +4570,7 @@ stringview_wtf16.length ) - ;; CHECK: (func $string-encode (type $28) (param $0 stringref) + ;; CHECK: (func $string-encode (type $29) (param $0 stringref) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (string.encode_wtf8 ;; CHECK-NEXT: (local.get $0) @@ -4641,7 +4641,7 @@ string.concat ) - ;; CHECK: (func $string-eq (type $29) (param $0 stringref) (param $1 stringref) (result i32) + ;; CHECK: (func $string-eq (type $30) (param $0 stringref) (param $1 stringref) (result i32) ;; CHECK-NEXT: (string.eq ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: (local.get $1) @@ -4653,7 +4653,7 @@ string.eq ) - ;; CHECK: (func $string-compare (type $29) (param $0 stringref) (param $1 stringref) (result i32) + ;; CHECK: (func $string-compare (type $30) (param $0 stringref) (param $1 stringref) (result i32) ;; CHECK-NEXT: (string.compare ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: (local.get $1) @@ -4849,7 +4849,7 @@ ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (call_indirect $timport$0 (type $30) + ;; CHECK-NEXT: (call_indirect $timport$0 (type $31) ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: (local.get $0) @@ -4911,7 +4911,7 @@ ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (call_indirect $timport$0 (type $30) + ;; CHECK-NEXT: (call_indirect $timport$0 (type $31) ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: (local.get $0) @@ -4983,7 +4983,7 @@ ;; CHECK-NEXT: (return_call_indirect $funcs (type $void) ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (return_call_indirect $timport$0 (type $31) + ;; CHECK-NEXT: (return_call_indirect $timport$0 (type $32) ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: (local.get $0) @@ -5042,7 +5042,7 @@ ;; CHECK-NEXT: (return_call_indirect $funcs (type $void) ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (return_call_indirect $timport$0 (type $31) + ;; CHECK-NEXT: (return_call_indirect $timport$0 (type $32) ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: (local.get $0) @@ -5090,7 +5090,7 @@ ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block $block (result (ref $to-f32-cont)) ;; CHECK-NEXT: (tuple.drop 3 - ;; CHECK-NEXT: (block $block0 (type $36) (result i32 i64 (ref null $simple-cont)) + ;; CHECK-NEXT: (block $block0 (type $37) (result i32 i64 (ref null $simple-cont)) ;; CHECK-NEXT: (local.set $f ;; CHECK-NEXT: (resume $simple-cont (tag $empty $block) (tag $tag-pair-to-pair $block0) ;; CHECK-NEXT: (i32.const 0) @@ -5280,4 +5280,80 @@ (param (ref $submany)) (param (ref $all-types)) ) + + ;; The if is unreachable except through the break; make sure this is + ;; parse correctly + ;; CHECK: (func $if-else-br-return (type $25) (param $a i32) (result i32) + ;; CHECK-NEXT: (block $label + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $a) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (br $label) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $if-else-br-return (param $a i32) (result i32) + (if (local.get $a) + (then + (br 0) + ) + (else + (return (i32.const 0)) + ) + ) + (i32.const 1) + ) + + ;; CHECK: (func $try-br-catch-all-return (type $void) + ;; CHECK-NEXT: (block $label + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (br $label) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch_all + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $try-br-catch-all-return + (try + (do + (br 0) + ) + (catch_all + (return) + ) + ) + ) + + ;; CHECK: (func $try-br-catch-return (type $void) + ;; CHECK-NEXT: (block $label + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (br $label) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $empty + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $try-br-catch-return + (try + (do + (br 0) + ) + (catch $empty + (return) + ) + ) + ) )