Repository: tetratelabs/wazero
Pull request #122 (2022-01-08 02:13:05)

wat: completes coverage except TODOs


Patch:
diff --git a/wasm/wat/parser.go b/wasm/wat/parser.go
index 967b9da8..91582671 100644
--- a/wasm/wat/parser.go
+++ b/wasm/wat/parser.go
@@ -97,9 +97,9 @@ func (p *moduleParser) parse(tok tokenType, tokenBytes []byte, line, col uint32)
 	return p.tokenParser(tok, tokenBytes, line, col)
 }
 
-func (p *moduleParser) ensureLParen(tok tokenType, _ []byte, _, _ uint32) error {
+func (p *moduleParser) ensureLParen(tok tokenType, tokenBytes []byte, _, _ uint32) error {
 	if tok != tokenLParen {
-		return fmt.Errorf("expected '(', but found %s", tok)
+		return fmt.Errorf("expected '(', but found %s: %s", tok, tokenBytes)
 	}
 	p.tokenParser = p.beginField
 	return nil
@@ -142,13 +142,12 @@ func (p *moduleParser) beginField(tok tokenType, fieldName []byte, _, _ uint32)
 		}
 	case fieldModuleImport:
 		// Add the next import func object and ready for parsing it.
-		if string(fieldName) == "func" { // cannot repeat
-			if uint32(len(p.module.importFuncs)) > p.currentImportIndex {
-				return fmt.Errorf("redundant field: %s", string(fieldName))
-			}
-			if err := p.addNextImportFunc(); err != nil {
-				return err
-			}
+		if string(fieldName) == "func" {
+			p.module.importFuncs = append(p.module.importFuncs, &importFunc{
+				module:      string(p.currentValue0),
+				name:        string(p.currentValue1),
+				importIndex: p.currentImportIndex,
+			})
 			p.currentField = fieldModuleImportFunc
 			p.tokenParser = p.parseImportFunc
 		} // TODO: table, memory or global
@@ -178,7 +177,7 @@ func (p *moduleParser) beginField(tok tokenType, fieldName []byte, _, _ uint32)
 //
 // Design Note: This is an alternative to using a stack as the structure parsed by moduleParser is fixed depth. For
 // example, any function body may be parsed in a more dynamic way.
-func (p *moduleParser) endField() error {
+func (p *moduleParser) endField() {
 	switch p.currentField {
 	case fieldModuleImportFuncParam, fieldModuleImportFuncResult:
 		p.currentField = fieldModuleImportFunc
@@ -191,11 +190,10 @@ func (p *moduleParser) endField() error {
 		p.tokenParser = p.parseModule
 	case fieldModule:
 		p.currentField = fieldInitial
-		p.tokenParser = p.parseUnexpectedToken // only one module is allowed and nothing else
-	default:
-		return errors.New("parent field is unknown") // bug
+		p.tokenParser = p.parseUnexpectedTrailingCharacters // only one module is allowed and nothing else
+	default: // currentField is an enum, we expect to have handled all cases above. panic if we didn't
+		panic(fmt.Errorf("BUG: unhandled parsing state: %v", p.currentField))
 	}
-	return nil
 }
 
 func (p *moduleParser) parseModule(tok tokenType, tokenBytes []byte, _, _ uint32) error {
@@ -210,7 +208,7 @@ func (p *moduleParser) parseModule(tok tokenType, tokenBytes []byte, _, _ uint32
 		p.tokenParser = p.beginField // after this look for a field name
 		return nil
 	case tokenRParen: // end of module
-		return p.endField()
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
@@ -230,21 +228,31 @@ func (p *moduleParser) parseImport(tok tokenType, tokenBytes []byte, _, _ uint32
 			return fmt.Errorf("redundant name: %s", name)
 		}
 	case tokenLParen: // start fields, ex. (func
+		// Err if there's a second description. Ex. (import "" "" (func) (func))
+		if uint32(len(p.module.importFuncs)) > p.currentImportIndex {
+			return p.unexpectedToken(tok, tokenBytes)
+		}
+		// Err if there are not enough names when we reach a description. Ex. (import func())
+		if err := p.validateImportModuleAndName(); err != nil {
+			return err
+		}
 		p.tokenParser = p.beginField
 		return nil
 	case tokenRParen: // end of this import
-		// If we've not yet added the current import, determine the best error message.
+		// Err if we never reached a description...
 		if uint32(len(p.module.importFuncs)) == p.currentImportIndex {
 			if err := p.validateImportModuleAndName(); err != nil {
-				return err // Ex. (module) or (module "Math")
+				return err // Ex. missing (func) and names: (import) or (import "Math")
 			}
-			return errors.New("missing description field") // Ex. (module "Math" "Pi")
+			return errors.New("missing description field") // Ex. missing (func): (import "Math" "Pi")
 		}
 
-		// multiple imports are allowed, so advance in case there's a next.
+		// Multiple imports are allowed, so advance in case there's a next.
 		p.currentImportIndex++
 
-		return p.endField()
+		// Reset parsing state: this is late to help give correct error messages on multiple descriptions.
+		p.currentValue0, p.currentValue1 = nil, nil
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
@@ -261,19 +269,6 @@ func (p *moduleParser) validateImportModuleAndName() error {
 	return nil
 }
 
-func (p *moduleParser) addNextImportFunc() error {
-	if err := p.validateImportModuleAndName(); err != nil {
-		return err
-	}
-
-	p.module.importFuncs = append(p.module.importFuncs, &importFunc{
-		module:      string(p.currentValue0),
-		name:        string(p.currentValue1),
-		importIndex: p.currentImportIndex,
-	})
-	return nil
-}
-
 func (p *moduleParser) parseImportFunc(tok tokenType, tokenBytes []byte, _, _ uint32) error {
 	switch tok {
 	case tokenID: // Ex. $main
@@ -310,9 +305,8 @@ func (p *moduleParser) parseImportFunc(tok tokenType, tokenBytes []byte, _, _ ui
 		}
 
 		// reset parsing state
-		p.currentValue0, p.currentValue1 = nil, nil
 		p.currentParamIndex = 0
-		return p.endField()
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
@@ -328,11 +322,6 @@ func (p *moduleParser) parseImportFuncParam(tok tokenType, tokenBytes []byte, _,
 		if err != nil {
 			return err
 		}
-
-		// Here, we have to check for the abbreviated form, ex. (param i32 i32), noting this does not allow names!
-		if p.currentParamIndex > 0 && p.currentParams != nil { // ex. (param i32) (param i32 i32)
-			return errors.New("multiple type abbreviation is unexpected with multiple params")
-		}
 		p.currentParams = append(p.currentParams, vt)
 	case tokenRParen: // end of this field
 		if p.currentParams == nil {
@@ -349,7 +338,7 @@ func (p *moduleParser) parseImportFuncParam(tok tokenType, tokenBytes []byte, _,
 		p.currentParams = nil
 		p.currentParamIndex++
 
-		return p.endField()
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
@@ -359,7 +348,11 @@ func (p *moduleParser) parseImportFuncParam(tok tokenType, tokenBytes []byte, _,
 func (p *moduleParser) parseImportFuncResult(tok tokenType, tokenBytes []byte, _, _ uint32) error {
 	switch tok {
 	case tokenKeyword: // Ex. i32
-		if p.currentType.results != nil { // Ex. (result i32 i32)
+		if p.currentType == nil { // Ex. no params: (func (result i32))
+			p.currentType = &typeFunc{}
+		}
+
+		if p.currentType.results != nil { // Ex. double result: (func (result i32 i32))
 			return errors.New("redundant type")
 		}
 
@@ -369,16 +362,12 @@ func (p *moduleParser) parseImportFuncResult(tok tokenType, tokenBytes []byte, _
 		}
 
 		// add the result to the current type
-		if p.currentType == nil {
-			p.currentType = &typeFunc{}
-		}
-
 		p.currentType.results = append(p.currentType.results, vt)
 	case tokenRParen: // end of this field
 		if p.currentType == nil || p.currentType.results == nil {
 			return errors.New("expected a type")
 		}
-		return p.endField()
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
@@ -414,20 +403,20 @@ func (p *moduleParser) parseStart(tok tokenType, tokenBytes []byte, line, col ui
 		if p.module.startFunction == nil {
 			return errors.New("missing funcidx")
 		}
-		return p.endField()
+		p.endField()
 	default:
 		return p.unexpectedToken(tok, tokenBytes)
 	}
 	return nil
 }
 
-func (p *moduleParser) parseUnexpectedToken(tok tokenType, tokenBytes []byte, _, _ uint32) error {
-	return p.unexpectedToken(tok, tokenBytes)
+func (p *moduleParser) parseUnexpectedTrailingCharacters(_ tokenType, tokenBytes []byte, _, _ uint32) error {
+	return fmt.Errorf("unexpected trailing characters: %s", tokenBytes)
 }
 
 func (p *moduleParser) unexpectedToken(tok tokenType, tokenBytes []byte) error {
-	if tok == tokenLParen || tok == tokenRParen {
-		return fmt.Errorf("unexpected %s", tok)
+	if tok == tokenLParen { // unbalanced tokenRParen is caught at the lexer layer
+		return errors.New("unexpected '('")
 	}
 	return fmt.Errorf("unexpected %s: %s", tok, tokenBytes)
 }
@@ -452,6 +441,7 @@ func (p *moduleParser) errorContext() string {
 		}
 		// TODO: table, memory or global
 		return fmt.Sprintf("module.import[%d]", p.currentImportIndex)
+	default: // currentField is an enum, we expect to have handled all cases above. panic if we didn't
+		panic(fmt.Errorf("BUG: unhandled parsing state: %v", p.currentField))
 	}
-	return ""
 }


Test patch:
diff --git a/wasm/wat/convert_test.go b/wasm/wat/convert_test.go
index e11d64dc..cb19249a 100644
--- a/wasm/wat/convert_test.go
+++ b/wasm/wat/convert_test.go
@@ -13,6 +13,7 @@ import (
 
 func TestTextToBinary(t *testing.T) {
 	zero := uint32(0)
+	i32, i64 := wasm.ValueTypeI32, wasm.ValueTypeI64
 	tests := []struct {
 		name     string
 		input    string
@@ -38,22 +39,28 @@ func TestTextToBinary(t *testing.T) {
 			},
 		},
 		{
-			name:  "import func two",
-			input: "(module (import \"foo\" \"bar\" (func)) (import \"baz\" \"qux\" (func)))",
+			name: "multiple import func with different inlined type",
+			input: `(module
+	(import "wasi_snapshot_preview1" "path_open" (func $runtime.path_open (param i32 i32 i32 i32 i32 i64 i64 i32 i32) (result i32)))
+	(import "wasi_snapshot_preview1" "fd_write" (func $runtime.fd_write (param i32 i32 i32 i32) (result i32)))
+)`,
 			expected: &wasm.Module{
-				TypeSection: []*wasm.FunctionType{{}},
+				TypeSection: []*wasm.FunctionType{
+					{Params: []wasm.ValueType{i32, i32, i32, i32, i32, i64, i64, i32, i32}, Results: []wasm.ValueType{i32}},
+					{Params: []wasm.ValueType{i32, i32, i32, i32}, Results: []wasm.ValueType{i32}},
+				},
 				ImportSection: []*wasm.ImportSegment{
 					{
-						Module: "foo", Name: "bar",
+						Module: "wasi_snapshot_preview1", Name: "path_open",
 						Desc: &wasm.ImportDesc{
 							Kind:          wasm.ImportKindFunction,
 							FuncTypeIndex: 0,
 						},
 					}, {
-						Module: "baz", Name: "qux",
+						Module: "wasi_snapshot_preview1", Name: "fd_write",
 						Desc: &wasm.ImportDesc{
 							Kind:          wasm.ImportKindFunction,
-							FuncTypeIndex: 0,
+							FuncTypeIndex: 1,
 						},
 					},
 				},
@@ -110,6 +117,11 @@ func TestTextToBinary(t *testing.T) {
 
 func TestTextToBinary_Errors(t *testing.T) {
 	tests := []struct{ name, input, expectedErr string }{
+		{
+			name:        "invalid",
+			input:       "module",
+			expectedErr: "1:1: expected '(', but found keyword: module",
+		},
 		{
 			name:        "start, but no funcs",
 			input:       "(module (start $main))",
diff --git a/wasm/wat/lexer_test.go b/wasm/wat/lexer_test.go
index 44ce2370..abc14565 100644
--- a/wasm/wat/lexer_test.go
+++ b/wasm/wat/lexer_test.go
@@ -507,6 +507,13 @@ func TestLex_Errors(t *testing.T) {
 			expectedCol:  2,
 			expectedErr:  "expected an ASCII character, not 私",
 		},
+		{
+			name:         "truncated string",
+			input:        []byte("\"hello"),
+			expectedLine: 1,
+			expectedCol:  6,
+			expectedErr:  "expected end quote",
+		},
 		{
 			name:         "0x80 in block comment",
 			input:        []byte("(; \200)"),
diff --git a/wasm/wat/parser_test.go b/wasm/wat/parser_test.go
index c61b31e6..6551aca7 100644
--- a/wasm/wat/parser_test.go
+++ b/wasm/wat/parser_test.go
@@ -47,7 +47,33 @@ func TestParseModule(t *testing.T) {
 		{
 			name: "import func inlined type",
 			input: `(module
+	(import "wasi_snapshot_preview1" "fd_write" (func $runtime.fd_write (param i32) (param i32) (param i32) (param i32) (result i32)))
+)`,
+			expected: &module{
+				types: []*typeFunc{{params: []wasm.ValueType{i32, i32, i32, i32}, results: []wasm.ValueType{i32}}},
+				importFuncs: []*importFunc{
+					{importIndex: 0, module: "wasi_snapshot_preview1", name: "fd_write", funcName: "$runtime.fd_write"},
+				},
+			},
+		},
+		{
+			name: "import func inlined type - abbreviated",
+			input: `(module
 	(import "wasi_snapshot_preview1" "fd_write" (func $runtime.fd_write (param i32 i32 i32 i32) (result i32)))
+)`,
+			expected: &module{
+				types: []*typeFunc{{params: []wasm.ValueType{i32, i32, i32, i32}, results: []wasm.ValueType{i32}}},
+				importFuncs: []*importFunc{
+					{importIndex: 0, module: "wasi_snapshot_preview1", name: "fd_write", funcName: "$runtime.fd_write"},
+				},
+			},
+		},
+		{
+			// Spec says expand abbreviations first. It doesn't explicitly say you can't mix forms.
+			// See https://www.w3.org/TR/wasm-core-1/#abbreviations%E2%91%A0
+			name: "import func inlined type - mixed abbreviated",
+			input: `(module
+	(import "wasi_snapshot_preview1" "fd_write" (func $runtime.fd_write (param i32) (param i32 i32) (param i32) (result i32)))
 )`,
 			expected: &module{
 				types: []*typeFunc{{params: []wasm.ValueType{i32, i32, i32, i32}, results: []wasm.ValueType{i32}}},
@@ -68,9 +94,32 @@ func TestParseModule(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:  "import func inlined type no param",
+			input: `(module (import "" "" (func (result i32))))`,
+			expected: &module{
+				types:       []*typeFunc{{results: []wasm.ValueType{i32}}},
+				importFuncs: []*importFunc{{}},
+			},
+		},
 		{
 			name: "import func inlined type different param types",
 			input: `(module
+	(import "wasi_snapshot_preview1" "path_open" (func $runtime.path_open (param i32) (param i32) (param i32) (param i32) (param i32) (param i64) (param i64) (param i32) (param i32) (result i32)))
+)`,
+			expected: &module{
+				types: []*typeFunc{{
+					params:  []wasm.ValueType{i32, i32, i32, i32, i32, i64, i64, i32, i32},
+					results: []wasm.ValueType{i32},
+				}},
+				importFuncs: []*importFunc{
+					{importIndex: 0, module: "wasi_snapshot_preview1", name: "path_open", funcName: "$runtime.path_open"},
+				},
+			},
+		},
+		{
+			name: "import func inlined type different param types - abbreviated",
+			input: `(module
 	(import "wasi_snapshot_preview1" "path_open" (func $runtime.path_open (param i32 i32 i32 i32 i32 i64 i64 i32 i32) (result i32)))
 )`,
 			expected: &module{
@@ -158,8 +207,39 @@ func TestParseModule(t *testing.T) {
 	}
 }
 
+func TestParseValueType(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected wasm.ValueType
+	}{
+		{"i32", wasm.ValueTypeI32},
+		{"i64", wasm.ValueTypeI64},
+		{"f32", wasm.ValueTypeF32},
+		{"f64", wasm.ValueTypeF64},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+
+		t.Run(tc.input, func(t *testing.T) {
+			m, err := parseValueType([]byte(tc.input))
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, m)
+		})
+	}
+	t.Run("unknown type", func(t *testing.T) {
+		_, err := parseValueType([]byte("f65"))
+		require.EqualError(t, err, "unknown type: f65")
+	})
+}
+
 func TestParseModule_Errors(t *testing.T) {
 	tests := []struct{ name, input, expectedErr string }{
+		{
+			name:        "forgot parens",
+			input:       "module",
+			expectedErr: "1:1: expected '(', but found keyword: module",
+		},
 		{
 			name:        "no module",
 			input:       "()",
@@ -168,13 +248,18 @@ func TestParseModule_Errors(t *testing.T) {
 		{
 			name:        "double module",
 			input:       "(module) (module)",
-			expectedErr: "1:10: unexpected (",
+			expectedErr: "1:10: unexpected trailing characters: (",
 		},
 		{
 			name:        "module invalid name",
 			input:       "(module test)", // must start with $
 			expectedErr: "1:9: unexpected keyword: test in module",
 		},
+		{
+			name:        "module invalid field",
+			input:       "(module (test))",
+			expectedErr: "1:10: unexpected field: test in module",
+		},
 		{
 			name:        "module double name",
 			input:       "(module $foo $bar)",
@@ -195,11 +280,21 @@ func TestParseModule_Errors(t *testing.T) {
 			input:       "(module (import))",
 			expectedErr: "1:16: missing module and name in module.import[0]",
 		},
+		{
+			name:        "import with desc, but no module",
+			input:       "(module (import (func))",
+			expectedErr: "1:17: missing module and name in module.import[0]",
+		},
 		{
 			name:        "import missing name",
 			input:       "(module (import \"\"))",
 			expectedErr: "1:19: missing name in module.import[0]",
 		},
+		{
+			name:        "import with desc, no name",
+			input:       "(module (import \"\" (func)))",
+			expectedErr: "1:20: missing name in module.import[0]",
+		},
 		{
 			name:        "import unquoted module",
 			input:       "(module (import foo bar))",
@@ -225,6 +320,11 @@ func TestParseModule_Errors(t *testing.T) {
 			input:       "(module (import \"foo\" \"bar\" (func baz)))",
 			expectedErr: "1:35: unexpected keyword: baz in module.import[0].func",
 		},
+		{
+			name:        "import func double name",
+			input:       "(module (import \"foo\" \"bar\" (func $baz $qux)))",
+			expectedErr: "1:40: redundant name: $qux in module.import[0].func",
+		},
 		{
 			name:        "import func missing param0 type",
 			input:       "(module (import \"\" \"\" (func (param))))",
@@ -260,10 +360,30 @@ func TestParseModule_Errors(t *testing.T) {
 			input:       "(module (import \"\" \"\" (func (param i32) (result f65))))",
 			expectedErr: "1:49: unknown type: f65 in module.import[0].func.result",
 		},
+		{
+			name:        "import func wrong no param type",
+			input:       "(module (import \"\" \"\" (func (param))))",
+			expectedErr: "1:35: expected a type in module.import[0].func.param[0]",
+		},
+		{
+			name:        "import func no result type",
+			input:       "(module (import \"\" \"\" (func (param i32) (result))))",
+			expectedErr: "1:48: expected a type in module.import[0].func.result",
+		},
+		{
+			name:        "import func wrong param token",
+			input:       "(module (import \"\" \"\" (func (param () ))))",
+			expectedErr: "1:36: unexpected '(' in module.import[0].func.param[0]",
+		},
+		{
+			name:        "import func wrong result token",
+			input:       "(module (import \"\" \"\" (func (result () ))))",
+			expectedErr: "1:37: unexpected '(' in module.import[0].func.result",
+		},
 		{
 			name:        "import func double desc",
 			input:       "(module (import \"foo\" \"bar\" (func $main) (func $mein)))",
-			expectedErr: "1:43: redundant field: func in module.import[0]",
+			expectedErr: "1:42: unexpected '(' in module.import[0]",
 		},
 		{
 			name:        "start missing funcidx",
@@ -280,6 +400,11 @@ func TestParseModule_Errors(t *testing.T) {
 			input:       "(module (start $main) (start $main))",
 			expectedErr: "1:24: redundant start in module",
 		},
+		{
+			name:        "wrong start",
+			input:       "(module (start main))",
+			expectedErr: "1:16: unexpected keyword: main in module.start",
+		},
 	}
 
 	for _, tt := range tests {