Repository: tetratelabs/wazero
Pull request #685 (2022-07-12 23:58:58)

Fixes error message on validate/applyData elements
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/wasm/store.go b/internal/wasm/store.go
index e5e83c8b..f0a28a67 100644
--- a/internal/wasm/store.go
+++ b/internal/wasm/store.go
@@ -273,25 +273,30 @@ func (m *ModuleInstance) buildExports(exports []*Export) {
 	}
 }
 
+// validateData ensures that data segments are valid in terms of memory boundary.
+// Note: this is used only when bulk-memory/reference type feature is disabled.
 func (m *ModuleInstance) validateData(data []*DataSegment) (err error) {
 	for i, d := range data {
 		if !d.IsPassive() {
 			offset := int(executeConstExpression(m.Globals, d.OffsetExpression).(int32))
 			ceil := offset + len(d.Init)
 			if offset < 0 || ceil > len(m.Memory.Buffer) {
-				return fmt.Errorf("%s[%d] out of bounds memory access", SectionIDName(SectionIDElement), i)
+				return fmt.Errorf("%s[%d]: out of bounds memory access", SectionIDName(SectionIDData), i)
 			}
 		}
 	}
 	return
 }
 
+// applyData uses the given data segments and mutate the memory according to the initial contents on it.
+// This is called after all the validation phase passes and out of bounds memory access error here is
+// not a validation error, but rather a runtime error.
 func (m *ModuleInstance) applyData(data []*DataSegment) error {
 	for i, d := range data {
 		if !d.IsPassive() {
 			offset := executeConstExpression(m.Globals, d.OffsetExpression).(int32)
 			if offset < 0 || int(offset)+len(d.Init) > len(m.Memory.Buffer) {
-				return fmt.Errorf("%s[%d] out of bounds memory access", SectionIDName(SectionIDElement), i)
+				return fmt.Errorf("%s[%d]: out of bounds memory access", SectionIDName(SectionIDData), i)
 			}
 			copy(m.Memory.Buffer[offset:], d.Init)
 		}
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/wasm/store_test.go b/internal/wasm/store_test.go
index 0e1a99f2..0cf2f638 100644
--- a/internal/wasm/store_test.go
+++ b/internal/wasm/store_test.go
@@ -732,7 +732,7 @@ func TestModuleInstance_validateData(t *testing.T) {
 	tests := []struct {
 		name   string
 		data   []*DataSegment
-		expErr bool
+		expErr string
 	}{
 		{
 			name: "ok",
@@ -746,14 +746,15 @@ func TestModuleInstance_validateData(t *testing.T) {
 			data: []*DataSegment{
 				{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeInt32(5)}, Init: []byte{0}},
 			},
-			expErr: true,
+			expErr: "data[0]: out of bounds memory access",
 		},
 		{
 			name: "out of bounds - multi bytes",
 			data: []*DataSegment{
+				{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeInt32(0)}, Init: []byte{0}},
 				{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeInt32(3)}, Init: []byte{0, 1, 2}},
 			},
-			expErr: true,
+			expErr: "data[1]: out of bounds memory access",
 		},
 	}
 
@@ -761,8 +762,8 @@ func TestModuleInstance_validateData(t *testing.T) {
 		tc := tt
 		t.Run(tc.name, func(t *testing.T) {
 			err := m.validateData(tc.data)
-			if tc.expErr {
-				require.Error(t, err)
+			if tc.expErr != "" {
+				require.EqualError(t, err, tc.expErr)
 			} else {
 				require.NoError(t, err)
 			}
@@ -771,13 +772,22 @@ func TestModuleInstance_validateData(t *testing.T) {
 }
 
 func TestModuleInstance_applyData(t *testing.T) {
-	m := &ModuleInstance{Memory: &MemoryInstance{Buffer: make([]byte, 10)}}
-	err := m.applyData([]*DataSegment{
-		{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: const0}, Init: []byte{0xa, 0xf}},
-		{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeUint32(8)}, Init: []byte{0x1, 0x5}},
+	t.Run("ok", func(t *testing.T) {
+		m := &ModuleInstance{Memory: &MemoryInstance{Buffer: make([]byte, 10)}}
+		err := m.applyData([]*DataSegment{
+			{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: const0}, Init: []byte{0xa, 0xf}},
+			{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeUint32(8)}, Init: []byte{0x1, 0x5}},
+		})
+		require.NoError(t, err)
+		require.Equal(t, []byte{0xa, 0xf, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x5}, m.Memory.Buffer)
+	})
+	t.Run("error", func(t *testing.T) {
+		m := &ModuleInstance{Memory: &MemoryInstance{Buffer: make([]byte, 5)}}
+		err := m.applyData([]*DataSegment{
+			{OffsetExpression: &ConstantExpression{Opcode: OpcodeI32Const, Data: leb128.EncodeUint32(8)}, Init: []byte{}},
+		})
+		require.EqualError(t, err, "data[0]: out of bounds memory access")
 	})
-	require.NoError(t, err)
-	require.Equal(t, []byte{0xa, 0xf, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x5}, m.Memory.Buffer)
 }
 
 func globalsContain(globals []*GlobalInstance, want *GlobalInstance) bool {
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)
+		})
+	}
+}