Repository: tetratelabs/wazero
Pull request #683 (2022-07-12 12:01:57)

ir: fixes a range for drop operation on vector values
Fixes the second bug found by wazero-fuzz. Previously, Drop instructions on the
vector values were wrongly translated and resulted in panic both on amd64 and arm64,
though that is not covered by spectes and practically doesn't appear in the real world program. 


Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>


Patch:
diff --git a/internal/engine/compiler/impl_arm64.go b/internal/engine/compiler/impl_arm64.go
index 6e7185d8..587f7ad4 100644
--- a/internal/engine/compiler/impl_arm64.go
+++ b/internal/engine/compiler/impl_arm64.go
@@ -167,7 +167,7 @@ func (c *arm64Compiler) markRegisterUnused(regs ...asm.Register) {
 	}
 }
 
-func (c *arm64Compiler) String() (ret string) { return }
+func (c *arm64Compiler) String() (ret string) { return c.locationStack.String() }
 
 // pushFunctionParams pushes any function parameters onto the stack, setting appropriate register types.
 func (c *arm64Compiler) pushFunctionParams() {
diff --git a/internal/wasm/binary/element.go b/internal/wasm/binary/element.go
index c8fb45fa..a0841b91 100644
--- a/internal/wasm/binary/element.go
+++ b/internal/wasm/binary/element.go
@@ -37,10 +37,10 @@ func decodeElementInitValueVector(r *bytes.Reader) ([]*wasm.Index, error) {
 	return vec, nil
 }
 
-func decodeElementConstExprVector(r *bytes.Reader, enabledFeatures wasm.Features) ([]*wasm.Index, error) {
+func decodeElementConstExprVector(r *bytes.Reader, elemType wasm.RefType, enabledFeatures wasm.Features) ([]*wasm.Index, error) {
 	vs, _, err := leb128.DecodeUint32(r)
 	if err != nil {
-		return nil, fmt.Errorf("get size of vector: %w", err)
+		return nil, fmt.Errorf("failed to get the size of constexpr vector: %w", err)
 	}
 	vec := make([]*wasm.Index, vs)
 	for i := range vec {
@@ -50,11 +50,15 @@ func decodeElementConstExprVector(r *bytes.Reader, enabledFeatures wasm.Features
 		}
 		switch expr.Opcode {
 		case wasm.OpcodeRefFunc:
+			if elemType != wasm.RefTypeFuncref {
+				return nil, fmt.Errorf("element type mismatch: want %s, but constexpr has funcref", wasm.RefTypeName(elemType))
+			}
 			v, _, _ := leb128.DecodeUint32(bytes.NewReader(expr.Data))
 			vec[i] = &v
 		case wasm.OpcodeRefNull:
-			if expr.Data[0] != wasm.RefTypeFuncref {
-				return nil, errors.New("ref type must be funcref for ref.null const expression as of WebAssembly 2.0")
+			if elemType != expr.Data[0] {
+				return nil, fmt.Errorf("element type mismatch: want %s, but constexpr has %s",
+					wasm.RefTypeName(elemType), wasm.RefTypeName(expr.Data[0]))
 			}
 			// vec[i] is already nil, so nothing to do.
 		default:
@@ -64,14 +68,14 @@ func decodeElementConstExprVector(r *bytes.Reader, enabledFeatures wasm.Features
 	return vec, nil
 }
 
-func decodeElementRefType(r *bytes.Reader, enabledFeatures wasm.Features) (ret wasm.RefType, err error) {
+func decodeElementRefType(r *bytes.Reader) (ret wasm.RefType, err error) {
 	ret, err = r.ReadByte()
 	if err != nil {
 		err = fmt.Errorf("read element ref type: %w", err)
 		return
 	}
-	if ret != wasm.RefTypeFuncref {
-		return 0, errors.New("ref type must be funcref for element as of WebAssembly 2.0")
+	if ret != wasm.RefTypeFuncref && ret != wasm.RefTypeExternref {
+		return 0, errors.New("ref type must be funcref or externref for element as of WebAssembly 2.0")
 	}
 	return
 }
@@ -199,7 +203,7 @@ func decodeElementSegment(r *bytes.Reader, enabledFeatures wasm.Features) (*wasm
 			return nil, fmt.Errorf("read expr for offset: %w", err)
 		}
 
-		init, err := decodeElementConstExprVector(r, enabledFeatures)
+		init, err := decodeElementConstExprVector(r, wasm.RefTypeFuncref, enabledFeatures)
 		if err != nil {
 			return nil, err
 		}
@@ -212,11 +216,11 @@ func decodeElementSegment(r *bytes.Reader, enabledFeatures wasm.Features) (*wasm
 			TableIndex: 0,
 		}, nil
 	case elementSegmentPrefixPassiveConstExprVector:
-		refType, err := decodeElementRefType(r, enabledFeatures)
+		refType, err := decodeElementRefType(r)
 		if err != nil {
 			return nil, err
 		}
-		init, err := decodeElementConstExprVector(r, enabledFeatures)
+		init, err := decodeElementConstExprVector(r, refType, enabledFeatures)
 		if err != nil {
 			return nil, err
 		}
@@ -241,12 +245,12 @@ func decodeElementSegment(r *bytes.Reader, enabledFeatures wasm.Features) (*wasm
 			return nil, fmt.Errorf("read expr for offset: %w", err)
 		}
 
-		refType, err := decodeElementRefType(r, enabledFeatures)
+		refType, err := decodeElementRefType(r)
 		if err != nil {
 			return nil, err
 		}
 
-		init, err := decodeElementConstExprVector(r, enabledFeatures)
+		init, err := decodeElementConstExprVector(r, refType, enabledFeatures)
 		if err != nil {
 			return nil, err
 		}
@@ -259,11 +263,11 @@ func decodeElementSegment(r *bytes.Reader, enabledFeatures wasm.Features) (*wasm
 			TableIndex: tableIndex,
 		}, nil
 	case elementSegmentPrefixDeclarativeConstExprVector:
-		refType, err := decodeElementRefType(r, enabledFeatures)
+		refType, err := decodeElementRefType(r)
 		if err != nil {
 			return nil, err
 		}
-		init, err := decodeElementConstExprVector(r, enabledFeatures)
+		init, err := decodeElementConstExprVector(r, refType, enabledFeatures)
 		if err != nil {
 			return nil, err
 		}
diff --git a/internal/wazeroir/compiler.go b/internal/wazeroir/compiler.go
index 19fca988..6c3a89e4 100644
--- a/internal/wazeroir/compiler.go
+++ b/internal/wazeroir/compiler.go
@@ -305,12 +305,24 @@ func compile(enabledFeatures wasm.Features,
 func (c *compiler) handleInstruction() error {
 	op := c.body[c.pc]
 	if buildoptions.IsDebugMode {
-		fmt.Printf("handling %s, unreachable_state(on=%v,depth=%d)\n",
-			wasm.InstructionName(op),
-			c.unreachableState.on, c.unreachableState.depth,
+		var instName string
+		if op == wasm.OpcodeVecPrefix {
+			instName = wasm.VectorInstructionName(c.body[c.pc+1])
+		} else if op == wasm.OpcodeMiscPrefix {
+			instName = wasm.MiscInstructionName(c.body[c.pc+1])
+		} else {
+			instName = wasm.InstructionName(op)
+		}
+		fmt.Printf("handling %s, unreachable_state(on=%v,depth=%d), stack=%v\n",
+			instName, c.unreachableState.on, c.unreachableState.depth, c.stack,
 		)
 	}
 
+	var peekValueType UnsignedType
+	if len(c.stack) > 0 {
+		peekValueType = c.stackPeek()
+	}
+
 	// Modify the stack according the current instruction.
 	// Note that some instructions will read "index" in
 	// applyToStack and advance c.pc inside the function.
@@ -717,8 +729,14 @@ operatorSwitch:
 			&OperationCallIndirect{TypeIndex: *index, TableIndex: tableIndex},
 		)
 	case wasm.OpcodeDrop:
+		r := &InclusiveRange{Start: 0, End: 0}
+		if peekValueType == UnsignedTypeV128 {
+			// InclusiveRange is the range in uint64 representation, so dropping a vector value on top
+			// should be translated as drop [0..1] inclusively.
+			r.End++
+		}
 		c.emit(
-			&OperationDrop{Depth: &InclusiveRange{Start: 0, End: 0}},
+			&OperationDrop{Depth: r},
 		)
 	case wasm.OpcodeSelect:
 		c.emit(
@@ -2904,6 +2922,11 @@ func (c *compiler) applyToStack(opcode wasm.Opcode) (*uint32, error) {
 	return ptr, nil
 }
 
+func (c *compiler) stackPeek() (ret UnsignedType) {
+	ret = c.stack[len(c.stack)-1]
+	return
+}
+
 func (c *compiler) stackPop() (ret UnsignedType) {
 	// No need to check stack bound
 	// as we can assume that all the operations


Test patch:
diff --git a/internal/wasm/binary/element_test.go b/internal/wasm/binary/element_test.go
index 6e2ff7f7..5b642995 100644
--- a/internal/wasm/binary/element_test.go
+++ b/internal/wasm/binary/element_test.go
@@ -46,12 +46,14 @@ func Test_decodeElementInitValueVector(t *testing.T) {
 func Test_decodeElementConstExprVector(t *testing.T) {
 	tests := []struct {
 		in       []byte
+		refType  wasm.RefType
 		exp      []*wasm.Index
 		features wasm.Features
 	}{
 		{
 			in:       []byte{0},
 			exp:      []*wasm.Index{},
+			refType:  wasm.RefTypeFuncref,
 			features: wasm.FeatureBulkMemoryOperations,
 		},
 		{
@@ -61,6 +63,7 @@ func Test_decodeElementConstExprVector(t *testing.T) {
 				wasm.OpcodeRefFunc, 100, wasm.OpcodeEnd,
 			},
 			exp:      []*wasm.Index{nil, uint32Ptr(100)},
+			refType:  wasm.RefTypeFuncref,
 			features: wasm.FeatureBulkMemoryOperations,
 		},
 		{
@@ -73,6 +76,17 @@ func Test_decodeElementConstExprVector(t *testing.T) {
 				wasm.OpcodeRefNull, wasm.RefTypeFuncref, wasm.OpcodeEnd,
 			},
 			exp:      []*wasm.Index{nil, uint32Ptr(165675008), nil},
+			refType:  wasm.RefTypeFuncref,
+			features: wasm.FeatureBulkMemoryOperations,
+		},
+		{
+			in: []byte{
+				2, // Two indexes.
+				wasm.OpcodeRefNull, wasm.RefTypeExternref, wasm.OpcodeEnd,
+				wasm.OpcodeRefNull, wasm.RefTypeExternref, wasm.OpcodeEnd,
+			},
+			exp:      []*wasm.Index{nil, nil},
+			refType:  wasm.RefTypeExternref,
 			features: wasm.FeatureBulkMemoryOperations,
 		},
 	}
@@ -80,13 +94,69 @@ func Test_decodeElementConstExprVector(t *testing.T) {
 	for i, tt := range tests {
 		tc := tt
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			actual, err := decodeElementConstExprVector(bytes.NewReader(tc.in), tc.features)
+			actual, err := decodeElementConstExprVector(bytes.NewReader(tc.in), tc.refType, tc.features)
 			require.NoError(t, err)
 			require.Equal(t, tc.exp, actual)
 		})
 	}
 }
 
+func Test_decodeElementConstExprVector_errors(t *testing.T) {
+	tests := []struct {
+		name     string
+		in       []byte
+		refType  wasm.RefType
+		expErr   string
+		features wasm.Features
+	}{
+		{
+			name:   "eof",
+			expErr: "failed to get the size of constexpr vector: EOF",
+		},
+		{
+			name:   "feature",
+			in:     []byte{1, wasm.OpcodeRefNull, wasm.RefTypeExternref, wasm.OpcodeEnd},
+			expErr: "ref.null is not supported as feature \"bulk-memory-operations\" is disabled",
+		},
+		{
+			name:     "type mismatch - ref.null",
+			in:       []byte{1, wasm.OpcodeRefNull, wasm.RefTypeExternref, wasm.OpcodeEnd},
+			refType:  wasm.RefTypeFuncref,
+			features: wasm.Features20220419,
+			expErr:   "element type mismatch: want funcref, but constexpr has externref",
+		},
+		{
+			name:     "type mismatch - ref.null",
+			in:       []byte{1, wasm.OpcodeRefNull, wasm.RefTypeFuncref, wasm.OpcodeEnd},
+			refType:  wasm.RefTypeExternref,
+			features: wasm.Features20220419,
+			expErr:   "element type mismatch: want externref, but constexpr has funcref",
+		},
+		{
+			name:     "invalid ref type",
+			in:       []byte{1, wasm.OpcodeRefNull, 0xff, wasm.OpcodeEnd},
+			refType:  wasm.RefTypeExternref,
+			features: wasm.Features20220419,
+			expErr:   "invalid type for ref.null: 0xff",
+		},
+		{
+			name:     "type mismatch - ref.fuc",
+			in:       []byte{1, wasm.OpcodeRefFunc, 0, wasm.OpcodeEnd},
+			refType:  wasm.RefTypeExternref,
+			features: wasm.Features20220419,
+			expErr:   "element type mismatch: want externref, but constexpr has funcref",
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := decodeElementConstExprVector(bytes.NewReader(tc.in), tc.refType, tc.features)
+			require.EqualError(t, err, tc.expErr)
+		})
+	}
+}
+
 func TestDecodeElementSegment(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -146,7 +216,6 @@ func TestDecodeElementSegment(t *testing.T) {
 			features: wasm.FeatureBulkMemoryOperations,
 		},
 		{
-
 			name: "active with table index encoded.",
 			in: []byte{
 				2, // Prefix.
@@ -258,22 +327,13 @@ func TestDecodeElementSegment(t *testing.T) {
 			},
 			features: wasm.FeatureBulkMemoryOperations,
 		},
-		{
-			name: "passive const expr vector - extern ref type",
-			in: []byte{
-				5, // Prefix.
-				wasm.RefTypeExternref,
-			},
-			expErr:   `ref type must be funcref for element as of WebAssembly 2.0`,
-			features: wasm.FeatureBulkMemoryOperations | wasm.FeatureReferenceTypes,
-		},
 		{
 			name: "passive const expr vector - unknown ref type",
 			in: []byte{
 				5, // Prefix.
 				0xff,
 			},
-			expErr:   `ref type must be funcref for element as of WebAssembly 2.0`,
+			expErr:   `ref type must be funcref or externref for element as of WebAssembly 2.0`,
 			features: wasm.FeatureBulkMemoryOperations | wasm.FeatureReferenceTypes,
 		},
 		{
diff --git a/internal/wazeroir/compiler_test.go b/internal/wazeroir/compiler_test.go
index 605d10e2..fa9a5152 100644
--- a/internal/wazeroir/compiler_test.go
+++ b/internal/wazeroir/compiler_test.go
@@ -2507,3 +2507,43 @@ func TestCompile_unreachable_br_brif(t *testing.T) {
 		})
 	}
 }
+
+// TestCompile_drop_vectors ensures that wasm.OpcodeDrop on vector values is correctly compiled,
+// which is not covered by spectest.
+func TestCompile_drop_vectors(t *testing.T) {
+	tests := []struct {
+		name     string
+		mod      *wasm.Module
+		expected []Operation
+	}{
+		{
+			name: "basic",
+			mod: &wasm.Module{
+				TypeSection:     []*wasm.FunctionType{{}},
+				FunctionSection: []wasm.Index{0},
+				CodeSection: []*wasm.Code{{Body: []byte{
+					wasm.OpcodeVecPrefix,
+					wasm.OpcodeVecV128Const, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0,
+					wasm.OpcodeDrop,
+					wasm.OpcodeEnd,
+				}}},
+			},
+			expected: []Operation{
+				&OperationV128Const{Lo: 0x1, Hi: 0x2},
+				// InclusiveRange is the range in uint64 representation, so dropping a vector value on top
+				// should be translated as drop [0..1] inclusively.
+				&OperationDrop{Depth: &InclusiveRange{Start: 0, End: 1}},
+				&OperationBr{Target: &BranchTarget{}},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			res, err := CompileFunctions(ctx, wasm.Features20220419, tc.mod)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, res[0].Operations)
+		})
+	}
+}