Repository: tetratelabs/wazero
Pull request #1695 (2023-09-11 05:04:06)

wazevo(frontend): fix if-end nesting unreachable if-else-end
#1496 


Patch:
diff --git a/internal/engine/wazevo/frontend/lower.go b/internal/engine/wazevo/frontend/lower.go
index 1b1dc931..6ae99c71 100644
--- a/internal/engine/wazevo/frontend/lower.go
+++ b/internal/engine/wazevo/frontend/lower.go
@@ -48,9 +48,15 @@ func (l *loweringState) String() string {
 	for _, v := range l.values {
 		str = append(str, fmt.Sprintf("v%v", v.ID()))
 	}
-	return fmt.Sprintf("\n\tunreachable=%v(depth=%d), \n\tstack: %s",
+	var frames []string
+	for i := range l.controlFrames {
+		frames = append(frames, l.controlFrames[i].kind.String())
+	}
+	return fmt.Sprintf("\n\tunreachable=%v(depth=%d)\n\tstack: %s\n\tcontrol frames: %s",
 		l.unreachable, l.unreachableDepth,
-		strings.Join(str, ", "))
+		strings.Join(str, ", "),
+		strings.Join(frames, ", "),
+	)
 }
 
 const (
@@ -61,6 +67,24 @@ const (
 	controlFrameKindBlock
 )
 
+// String implements fmt.Stringer for debugging.
+func (k controlFrameKind) String() string {
+	switch k {
+	case controlFrameKindFunction:
+		return "function"
+	case controlFrameKindLoop:
+		return "loop"
+	case controlFrameKindIfWithElse:
+		return "if_with_else"
+	case controlFrameKindIfWithoutElse:
+		return "if_without_else"
+	case controlFrameKindBlock:
+		return "block"
+	default:
+		panic(k)
+	}
+}
+
 // isLoop returns true if this is a loop frame.
 func (ctrl *controlFrame) isLoop() bool {
 	return ctrl.kind == controlFrameKindLoop
@@ -1037,14 +1061,13 @@ func (c *Compiler) lowerCurrentOpcode() {
 		builder.Seal(elseBlk)
 	case wasm.OpcodeElse:
 		ifctrl := state.ctrlPeekAt(0)
-		ifctrl.kind = controlFrameKindIfWithElse
-
 		if unreachable := state.unreachable; unreachable && state.unreachableDepth > 0 {
 			// If it is currently in unreachable and is a nested if,
 			// we just remove the entire else block.
 			break
 		}
 
+		ifctrl.kind = controlFrameKindIfWithElse
 		if !state.unreachable {
 			// If this Then block is currently reachable, we have to insert the branching to the following BB.
 			followingBlk := ifctrl.followingBlock // == the BB after if-then-else.


Test patch:
diff --git a/internal/engine/wazevo/backend/isa/arm64/lower_instr_operands_test.go b/internal/engine/wazevo/backend/isa/arm64/lower_instr_operands_test.go
index 374f73da..4a846f0c 100644
--- a/internal/engine/wazevo/backend/isa/arm64/lower_instr_operands_test.go
+++ b/internal/engine/wazevo/backend/isa/arm64/lower_instr_operands_test.go
@@ -404,8 +404,6 @@ func TestMachine_getOperand_ER_SR_NR(t *testing.T) {
 				extArgConst:  true,
 				instructions: []string{"movz w100?, #0xffff, lsl 0"},
 			},
-
-			///////////
 			{
 				name: "8(VReg)->64->64: signed",
 				from: 8, to: 64, signed: true, mode: extModeZeroExtend64,
diff --git a/internal/engine/wazevo/frontend/frontend_test.go b/internal/engine/wazevo/frontend/frontend_test.go
index 8b3a04ce..b9855506 100644
--- a/internal/engine/wazevo/frontend/frontend_test.go
+++ b/internal/engine/wazevo/frontend/frontend_test.go
@@ -1676,6 +1676,53 @@ blk12: () <-- (blk0)
 
 blk13: () <-- (blk0)
 	Jump blk6
+`,
+		},
+		{
+			name: "if_then_end_nesting_unreachable_if_then_else_end", m: testcases.IfThenEndNestingUnreachableIfThenElseEnd.Module,
+			exp: `
+blk0: (exec_ctx:i64, module_ctx:i64, v2:f64, v3:f64, v4:f64)
+	v6:i32 = Load module_ctx, 0x10
+	v7:i32 = Iconst_32 0x10
+	v8:i32 = Ushr v6, v7
+	Brz v8, blk3
+	Jump blk2
+
+blk1: (v5:i64) <-- (blk4)
+	Jump blk_ret
+
+blk2: () <-- (blk0)
+	v9:i32 = Load module_ctx, 0x10
+	v10:i32 = Iconst_32 0x10
+	v11:i32 = Ushr v9, v10
+	Jump blk4
+
+blk3: () <-- (blk0)
+	Jump blk4
+
+blk4: () <-- (blk2,blk3)
+	v12:i64 = Iconst_64 0x0
+	Jump blk1, v12
+`,
+			expAfterOpt: `
+blk0: (exec_ctx:i64, module_ctx:i64, v2:f64, v3:f64, v4:f64)
+	v6:i32 = Load module_ctx, 0x10
+	v7:i32 = Iconst_32 0x10
+	v8:i32 = Ushr v6, v7
+	Brz v8, blk3
+	Jump blk2
+
+blk1: () <-- (blk4)
+	Jump blk_ret
+
+blk2: () <-- (blk0)
+	Jump blk4
+
+blk3: () <-- (blk0)
+	Jump blk4
+
+blk4: () <-- (blk2,blk3)
+	Jump blk1
 `,
 		},
 	} {
diff --git a/internal/engine/wazevo/testcases/testcases.go b/internal/engine/wazevo/testcases/testcases.go
index 1455faa7..7875b2cc 100644
--- a/internal/engine/wazevo/testcases/testcases.go
+++ b/internal/engine/wazevo/testcases/testcases.go
@@ -1461,6 +1461,57 @@ var (
 			},
 		},
 	}
+
+	IfThenEndNestingUnreachableIfThenElseEnd = TestCase{
+		// This has been detected by fuzzing. This should belong to internal/integration_test/fuzzcases eventually,
+		// but for now, wazevo should have its own cases under engine/wazevo.
+		Name: "if_then_end_nesting_unreachable_if_then_else_end",
+		// (module
+		//  (type (;0;) (func (param f64 f64 f64)))
+		//  (func (;0;) (type 0) (param f64 f64 f64)
+		//    block (result i64) ;; label = @1
+		//      memory.size
+		//      if ;; label = @2
+		//        memory.size
+		//        br 0 (;@2;)
+		//        if ;; label = @3
+		//        else
+		//        end
+		//        drop
+		//      end
+		//      i64.const 0
+		//    end
+		//    drop
+		//  )
+		//  (memory (;0;) 4554)
+		//)
+		Module: &wasm.Module{
+			TypeSection: []wasm.FunctionType{
+				{Params: []wasm.ValueType{f64, f64, f64}},
+				{Results: []wasm.ValueType{i64}},
+			},
+
+			ExportSection:   []wasm.Export{{Name: ExportedFunctionName, Type: wasm.ExternTypeFunc, Index: 0}},
+			MemorySection:   &wasm.Memory{Min: 4554},
+			FunctionSection: []wasm.Index{0},
+			CodeSection: []wasm.Code{{Body: []byte{
+				wasm.OpcodeBlock, 1, // Signature v_i64,
+				wasm.OpcodeMemorySize, 0,
+				wasm.OpcodeIf, blockSignature_vv,
+				wasm.OpcodeMemorySize, 0,
+				wasm.OpcodeBr, 0x0, // label=0
+				wasm.OpcodeIf, blockSignature_vv,
+				wasm.OpcodeElse,
+				wasm.OpcodeEnd,
+				wasm.OpcodeDrop,
+				wasm.OpcodeEnd,
+				wasm.OpcodeI64Const, 0,
+				wasm.OpcodeEnd,
+				wasm.OpcodeDrop,
+				wasm.OpcodeEnd,
+			}}},
+		},
+	}
 )
 
 type TestCase struct {