Repository: tetratelabs/wazero
Pull request #202 (2022-02-04 08:09:44)

text: fixes code section ID parsing a function
This moves logic to func_parser.go, which allows us to keep a coherent
function index when parsing code.


Patch:
diff --git a/wasm/text/decoder.go b/wasm/text/decoder.go
index 88c01c34..521f0fff 100644
--- a/wasm/text/decoder.go
+++ b/wasm/text/decoder.go
@@ -121,7 +121,7 @@ func newModuleParser(module *wasm.Module) *moduleParser {
 	}
 	p.typeParser = newTypeParser(p.typeNamespace, p.onTypeEnd)
 	p.typeUseParser = newTypeUseParser(module, p.typeNamespace)
-	p.funcParser = newFuncParser(p.funcNamespace, p.endFunc)
+	p.funcParser = newFuncParser(p.typeUseParser, p.funcNamespace, p.endFunc)
 	p.memoryParser = newMemoryParser(p.memoryNamespace, p.endMemory)
 	return &p
 }
@@ -160,7 +160,8 @@ func (p *moduleParser) beginModuleField(tok tokenType, tokenBytes []byte, _, _ u
 			return p.parseImportModule, nil
 		case "func":
 			p.pos = positionModuleFunc
-			return p.parseFuncID, nil
+			p.funcParser.currentIdx = wasm.Index(len(p.module.FunctionSection))
+			return p.funcParser.begin, nil
 		case "memory":
 			p.pos = positionModuleMemory
 			return p.memoryParser.begin, nil
@@ -302,14 +303,25 @@ func (p *moduleParser) beginImportDesc(tok tokenType, tokenBytes []byte, _, _ ui
 //                  calls parseImportFunc here --^
 func (p *moduleParser) parseImportFuncID(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
 	if tok == tokenID { // Ex. $main
-		if err := p.setFuncID(tokenBytes); err != nil {
+		id, err := p.funcNamespace.setID(tokenBytes)
+		if err != nil {
 			return nil, err
 		}
+		p.addFunctionName(id)
 		return p.parseImportFunc, nil
 	}
 	return p.parseImportFunc(tok, tokenBytes, line, col)
 }
 
+// addFunctionName appends the current imported or module-defined function name to the wasm.NameSection
+func (p *moduleParser) addFunctionName(name string) {
+	if name == "" {
+		return // there's no value in an empty name
+	}
+	na := &wasm.NameAssoc{Index: p.funcNamespace.count, Name: name}
+	p.module.NameSection.FunctionNames = append(p.module.NameSection.FunctionNames, na)
+}
+
 // parseImportFunc passes control to the typeUseParser until any signature is read, then returns onImportFunc.
 //
 // Ex. `(import "Math" "PI" (func $math.pi (result f32)))`
@@ -330,7 +342,7 @@ func (p *moduleParser) parseImportFunc(tok tokenType, tokenBytes []byte, line, c
 // onImportFunc records the type index and local names of the current imported function, and increments
 // funcNamespace as it is shared across imported and module-defined functions. Finally, this returns parseImportEnd to
 // the current import into the ImportSection.
-func (p *moduleParser) onImportFunc(typeIdx wasm.Index, paramNames wasm.NameMap, pos onTypeUsePosition, tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
+func (p *moduleParser) onImportFunc(typeIdx wasm.Index, paramNames wasm.NameMap, pos onTypeUsePosition, tok tokenType, tokenBytes []byte, _, _ uint32) (tokenParser, error) {
 	i := p.currentModuleField.(*wasm.Import)
 	i.Kind = wasm.ImportKindFunc
 	i.DescFunc = typeIdx
@@ -380,61 +392,10 @@ func (p *moduleParser) parseImportEnd(tok tokenType, tokenBytes []byte, _, _ uin
 	return p.parseModule, nil
 }
 
-// parseFuncID records the ID of the current function, if present, and resumes with parseFunc.
-//
-// Ex. A function ID is present `(module (func $math.pi (result f32))`
-//                      records math.pi here --^
-//                             parseFunc resumes here --^
-//
-// Ex. No function ID `(module (func (result f32))`
-//            calls parseFunc here --^
-func (p *moduleParser) parseFuncID(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
-	if tok == tokenID { // Ex. $main
-		if err := p.setFuncID(tokenBytes); err != nil {
-			return nil, err
-		}
-		return p.parseFunc, nil
-	}
-	return p.parseFunc(tok, tokenBytes, line, col)
-}
-
-// setFuncID adds the normalized ('$' stripped) function ID to the funcNamespace and the wasm.NameSection.
-func (p *moduleParser) setFuncID(idToken []byte) error {
-	id, err := p.funcNamespace.setID(idToken)
-	if err != nil {
-		return err
-	}
-	na := &wasm.NameAssoc{Index: p.funcNamespace.count, Name: id}
-	p.module.NameSection.FunctionNames = append(p.module.NameSection.FunctionNames, na)
-	return nil
-}
-
-// parseFunc passes control to the typeUseParser until any signature is read, then funcParser until and locals or body
-// are read. Finally, this finishes via endFunc.
-//
-// Ex. `(module (func $math.pi (result f32))`
-//               starts here --^           ^
-//                  endFunc resumes here --+
-//
-// Ex.    `(module (func $math.pi (result f32) (local i32) )`
-//                  starts here --^            ^           ^
-//             funcParser.begin resumes here --+           |
-//                                  endFunc resumes here --+
-//
-// Ex. If there is no signature `(func)`
-//              calls endFunc here ---^
-func (p *moduleParser) parseFunc(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
-	idx := wasm.Index(len(p.module.FunctionSection))
-	if tok == tokenID { // Ex. (func $main $main)
-		return nil, fmt.Errorf("redundant ID %s", tokenBytes)
-	}
-
-	return p.typeUseParser.begin(wasm.SectionIDFunction, idx, p.funcParser.begin, tok, tokenBytes, line, col)
-}
-
 // endFunc adds the type index, code and local names for the current function, and increments funcNamespace as it is
 // shared across imported and module-defined functions. Finally, this returns parseModule to prepare for the next field.
-func (p *moduleParser) endFunc(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error) {
+func (p *moduleParser) endFunc(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error) {
+	p.addFunctionName(name)
 	p.module.FunctionSection = append(p.module.FunctionSection, typeIdx)
 	p.module.CodeSection = append(p.module.CodeSection, code)
 	p.addLocalNames(localNames)
@@ -625,8 +586,6 @@ func (p *moduleParser) resolveTypeIndices(module *wasm.Module) error {
 
 // resolveFunctionIndices ensures any indices point are numeric or returns a FormatError if they cannot be bound.
 func (p *moduleParser) resolveFunctionIndices(module *wasm.Module) error {
-	// TODO: remove when funcParser points to the correct index
-	importCount := p.funcNamespace.count - uint32(len(module.CodeSection))
 	for _, unresolved := range p.funcNamespace.unresolvedIndices {
 		target, err := p.funcNamespace.resolve(unresolved)
 		if err != nil {
@@ -637,7 +596,7 @@ func (p *moduleParser) resolveFunctionIndices(module *wasm.Module) error {
 			if target > 255 {
 				return errors.New("TODO: unresolved function indexes that don't fit in a byte")
 			}
-			module.CodeSection[unresolved.idx-importCount].Body[unresolved.bodyOffset] = byte(target)
+			module.CodeSection[unresolved.idx].Body[unresolved.bodyOffset] = byte(target)
 		case wasm.SectionIDExport:
 			p.unresolvedExports[unresolved.idx].Index = target
 		case wasm.SectionIDStart:
diff --git a/wasm/text/func_parser.go b/wasm/text/func_parser.go
index 3473ba86..efac06ed 100644
--- a/wasm/text/func_parser.go
+++ b/wasm/text/func_parser.go
@@ -8,11 +8,11 @@ import (
 	"github.com/tetratelabs/wazero/wasm/internal/leb128"
 )
 
-func newFuncParser(funcNamespace *indexNamespace, onFunc onFunc) *funcParser {
-	return &funcParser{funcNamespace: funcNamespace, onFunc: onFunc}
+func newFuncParser(typeUseParser *typeUseParser, funcNamespace *indexNamespace, onFunc onFunc) *funcParser {
+	return &funcParser{typeUseParser: typeUseParser, funcNamespace: funcNamespace, onFunc: onFunc}
 }
 
-type onFunc func(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error)
+type onFunc func(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error)
 
 // funcParser parses any instructions and dispatches to onFunc.
 //
@@ -25,9 +25,14 @@ type funcParser struct {
 	// onFunc is called when complete parsing the body. Unless testing, this should be moduleParser.onFuncEnd
 	onFunc onFunc
 
+	// typeUseParser is described by moduleParser.typeUseParser
+	typeUseParser *typeUseParser
+
 	// funcNamespace is described by moduleParser.funcNamespace
 	funcNamespace *indexNamespace
 
+	currentIdx        wasm.Index
+	currentName       string
 	currentTypeIdx    wasm.Index
 	currentParamNames wasm.NameMap
 
@@ -40,17 +45,63 @@ type funcParser struct {
 var end = []byte{wasm.OpcodeEnd}
 var codeEnd = &wasm.Code{Body: end}
 
-// begin is a tokenParser that starts after a type use.
+// begin should be called after reaching the "func" keyword in a module field. Parsing continues until onFunc or
+// error.
+//
+// This stage records the ID of the current function, if present, and resumes with onFunc.
+//
+// Ex. A func ID is present `(func $main nop)`
+//                  records main --^     ^
+//              parseFunc resumes here --+
+//
+// Ex. No func ID `(func nop)`
+//     calls parseFunc --^
+func (p *funcParser) begin(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
+	if tok == tokenID { // Ex. $main
+		if id, err := p.funcNamespace.setID(tokenBytes); err != nil {
+			return nil, err
+		} else {
+			p.currentName = id
+		}
+		return p.parseFunc, nil
+	}
+	p.currentName = ""
+	return p.parseFunc(tok, tokenBytes, line, col)
+}
+
+// parseFunc passes control to the typeUseParser until any signature is read, then funcParser until and locals or body
+// are read. Finally, this finishes via endFunc.
+//
+// Ex. `(module (func $math.pi (result f32))`
+//               starts here --^           ^
+//                  endFunc resumes here --+
+//
+// Ex.    `(module (func $math.pi (result f32) (local i32) )`
+//                  starts here --^            ^           ^
+//      funcParser.afterTypeUse resumes here --+           |
+//                                  endFunc resumes here --+
+//
+// Ex. If there is no signature `(func)`
+//              calls endFunc here ---^
+func (p *funcParser) parseFunc(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
+	if tok == tokenID { // Ex. (func $main $main)
+		return nil, fmt.Errorf("redundant ID %s", tokenBytes)
+	}
+
+	return p.typeUseParser.begin(wasm.SectionIDFunction, p.currentIdx, p.afterTypeUse, tok, tokenBytes, line, col)
+}
+
+// afterTypeUse is a tokenParser that starts after a type use.
 //
 // The onFunc field is invoked once any instructions are written into currentBody.
 //
 // Ex. Given the source `(module (func nop))`
-//                 begin starts here --^  ^
+//          afterTypeUse starts here --^  ^
 //                    calls onFunc here --+
-func (p *funcParser) begin(typeIdx wasm.Index, paramNames wasm.NameMap, pos onTypeUsePosition, tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
+func (p *funcParser) afterTypeUse(typeIdx wasm.Index, paramNames wasm.NameMap, pos onTypeUsePosition, tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
 	switch pos {
 	case onTypeUseEndField:
-		return p.onFunc(typeIdx, codeEnd, paramNames)
+		return p.onFunc(typeIdx, codeEnd, p.currentName, paramNames)
 	case onTypeUseUnhandledField:
 		return sExpressionsUnsupported(tok, tokenBytes, line, col)
 	}
@@ -66,8 +117,12 @@ func sExpressionsUnsupported(tok tokenType, tokenBytes []byte, _, _ uint32) (tok
 		return nil, unexpectedToken(tok, tokenBytes)
 	}
 	switch string(tokenBytes) {
-	case "result", "param": // TODO: local
-		return nil, fmt.Errorf("%s declared out of order", tokenBytes)
+	case "param":
+		return nil, errors.New("param after result")
+	case "result":
+		return nil, errors.New("duplicate result")
+	case "local":
+		return nil, errors.New("TODO: local")
 	}
 	return nil, fmt.Errorf("TODO: s-expressions are not yet supported: %s", tokenBytes)
 }
@@ -112,15 +167,14 @@ func (p *funcParser) end() (tokenParser, error) {
 	} else {
 		code = &wasm.Code{Body: append(p.currentBody, wasm.OpcodeEnd)}
 	}
-	return p.onFunc(p.currentTypeIdx, code, p.currentParamNames)
+	return p.onFunc(p.currentTypeIdx, code, p.currentName, p.currentParamNames)
 }
 
 // parseFuncIndex parses an index in the function namespace and appends it to the currentBody. If it was an ID, a
 // placeholder byte(0) is added instead and will be resolved later.
 func (p *funcParser) parseFuncIndex(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
-	funcIdx := p.funcNamespace.count // TODO: move all function logic here and use the code index, not including imports
 	bodyOffset := uint32(len(p.currentBody))
-	idx, resolved, err := p.funcNamespace.parseIndex(wasm.SectionIDCode, funcIdx, bodyOffset, tok, tokenBytes, line, col)
+	idx, resolved, err := p.funcNamespace.parseIndex(wasm.SectionIDCode, p.currentIdx, bodyOffset, tok, tokenBytes, line, col)
 	if err != nil {
 		return nil, err
 	}
diff --git a/wasm/text/memory_parser.go b/wasm/text/memory_parser.go
index 46491c11..3137e666 100644
--- a/wasm/text/memory_parser.go
+++ b/wasm/text/memory_parser.go
@@ -33,7 +33,7 @@ type memoryParser struct {
 	currentMax *uint32
 }
 
-// begin should be called after reaching the "memory" keyword in a memory field. Parsing continues until onMemory or
+// begin should be called after reaching the "memory" keyword in a module field. Parsing continues until onMemory or
 // error.
 //
 // This stage records the ID of the current memory, if present, and resumes with beginMin.
diff --git a/wasm/text/type_parser.go b/wasm/text/type_parser.go
index 5e44d42a..d0d3cc01 100644
--- a/wasm/text/type_parser.go
+++ b/wasm/text/type_parser.go
@@ -50,7 +50,7 @@ type typeParser struct {
 	parsedParamID bool
 }
 
-// begin should be called after reaching the "type" keyword in a type field. Parsing continues until onType or error.
+// begin should be called after reaching the "type" keyword in a module field. Parsing continues until onType or error.
 //
 // This stage records the ID of the current type, if present, and resumes with tryBeginFunc.
 //


Test patch:
diff --git a/wasm/text/decoder_test.go b/wasm/text/decoder_test.go
index 22cbb892..26dbb815 100644
--- a/wasm/text/decoder_test.go
+++ b/wasm/text/decoder_test.go
@@ -1830,9 +1830,9 @@ func TestParseModule_Errors(t *testing.T) {
 			expectedErr: "1:34: unknown type: f65 in module.func[0].param[1]",
 		},
 		{
-			name:        "func double result",
+			name:        "func duplicate result",
 			input:       "(module (func (param i32) (result i32) (result i32)))",
-			expectedErr: "1:41: result declared out of order in module.func[0]",
+			expectedErr: "1:41: duplicate result in module.func[0]",
 		},
 		{
 			name:        "func double result type",
@@ -1922,7 +1922,7 @@ func TestParseModule_Errors(t *testing.T) {
 		{
 			name:        "func param after result",
 			input:       "(module (func (result i32) (param i32)))",
-			expectedErr: "1:29: param declared out of order in module.func[0]",
+			expectedErr: "1:29: param after result in module.func[0]",
 		},
 		{
 			name:        "func points nowhere",
diff --git a/wasm/text/func_parser_test.go b/wasm/text/func_parser_test.go
index 77852b18..0c2e2e7c 100644
--- a/wasm/text/func_parser_test.go
+++ b/wasm/text/func_parser_test.go
@@ -46,12 +46,12 @@ func TestFuncParser(t *testing.T) {
 
 		t.Run(tc.name, func(t *testing.T) {
 			var parsedCode *wasm.Code
-			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error) {
+			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error) {
 				parsedCode = code
 				return parseErr, nil
 			}
 
-			require.NoError(t, parseFunc(newFuncParser(newIndexNamespace(), setFunc), tc.source))
+			require.NoError(t, parseFunc(newFuncParser(&typeUseParser{module: &wasm.Module{}}, newIndexNamespace(), setFunc), tc.source))
 			require.Equal(t, tc.expected, parsedCode)
 		})
 	}
@@ -102,12 +102,12 @@ func TestFuncParser_Call_Unresolved(t *testing.T) {
 
 		t.Run(tc.name, func(t *testing.T) {
 			var parsedCode *wasm.Code
-			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error) {
+			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error) {
 				parsedCode = code
 				return parseErr, nil
 			}
 
-			fp := newFuncParser(newIndexNamespace(), setFunc)
+			fp := newFuncParser(&typeUseParser{module: &wasm.Module{}}, newIndexNamespace(), setFunc)
 			require.NoError(t, parseFunc(fp, tc.source))
 			require.Equal(t, tc.expectedCode, parsedCode)
 			require.Equal(t, []*unresolvedIndex{tc.expectedUnresolvedIndex}, fp.funcNamespace.unresolvedIndices)
@@ -159,12 +159,12 @@ func TestFuncParser_Call_Resolved(t *testing.T) {
 			funcNamespace.count++
 
 			var parsedCode *wasm.Code
-			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error) {
+			var setFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error) {
 				parsedCode = code
 				return parseErr, nil
 			}
 
-			fp := newFuncParser(funcNamespace, setFunc)
+			fp := newFuncParser(&typeUseParser{module: &wasm.Module{}}, funcNamespace, setFunc)
 			require.NoError(t, parseFunc(fp, tc.source))
 			require.Equal(t, tc.expected, parsedCode)
 		})
@@ -206,14 +206,14 @@ func TestFuncParser_Errors(t *testing.T) {
 			expectedErr: "1:8: TODO: s-expressions are not yet supported: f32.const",
 		},
 		{
-			name:        "param out of order", // because this parser is after the type use, so it must be wrong
-			source:      "(func (param i32))",
-			expectedErr: "1:8: param declared out of order",
+			name:        "param after result",
+			source:      "(func (result i32) (param i32))",
+			expectedErr: "1:21: param after result",
 		},
 		{
-			name:        "result out of order", // because this parser is after the type use, so it must be wrong
-			source:      "(func (result i32))",
-			expectedErr: "1:8: result declared out of order",
+			name:        "duplicate result",
+			source:      "(func (result i32) (result i32))",
+			expectedErr: "1:21: duplicate result",
 		},
 	}
 
@@ -221,23 +221,18 @@ func TestFuncParser_Errors(t *testing.T) {
 		tc := tt
 
 		t.Run(tc.name, func(t *testing.T) {
-			fp := newFuncParser(newIndexNamespace(), failOnFunc)
+			fp := newFuncParser(&typeUseParser{module: &wasm.Module{}}, newIndexNamespace(), failOnFunc)
 			require.EqualError(t, parseFunc(fp, tc.source), tc.expectedErr)
 		})
 	}
 }
 
-var failOnFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, localNames wasm.NameMap) (tokenParser, error) {
+var failOnFunc onFunc = func(typeIdx wasm.Index, code *wasm.Code, name string, localNames wasm.NameMap) (tokenParser, error) {
 	return nil, errors.New("unexpected to call onFunc on error")
 }
 
 func parseFunc(fp *funcParser, source string) error {
-	// TODO: all func hooks into func_parser.go, so that we don't have to fake the onTypeUse position
-	var parser tokenParser = func(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
-		return fp.begin(0, nil, onTypeUseUnhandledToken, tok, tokenBytes, line, col)
-	}
-
-	line, col, err := lex(skipTokens(2, parser), []byte(source)) // skip the leading (func
+	line, col, err := lex(skipTokens(2, fp.begin), []byte(source)) // skip the leading (func
 	if err != nil {
 		err = &FormatError{Line: line, Col: col, cause: err}
 	}