ir: rename NeedsAccess* -> Has*, and make them module-scoped (#699)

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>

internal/engine/compiler/compiler_initialization_test.go

@@ -111,10 +111,10 @@ func TestCompiler_compileModuleContextInitialization(t *testing.T) {
 			ce := env.callEngine()
 
 			ir := &wazeroir.CompilationResult{
-				HasMemory:                     tc.moduleInstance.Memory != nil,
+				HasMemory:           tc.moduleInstance.Memory != nil,
-				HasTable:                      len(tc.moduleInstance.Tables) > 0,
+				HasTable:            len(tc.moduleInstance.Tables) > 0,
-				NeedsAccessToDataInstances:    len(tc.moduleInstance.DataInstances) > 0,
+				HasDataInstances:    len(tc.moduleInstance.DataInstances) > 0,
-				NeedsAccessToElementInstances: len(tc.moduleInstance.ElementInstances) > 0,
+				HasElementInstances: len(tc.moduleInstance.ElementInstances) > 0,
 			}
 			for _, g := range tc.moduleInstance.Globals {
 				ir.Globals = append(ir.Globals, g.Type)

internal/engine/compiler/compiler_post1_0_test.go

@@ -332,7 +332,7 @@ func TestCompiler_compileDataDrop(t *testing.T) {
 			copy(env.module().DataInstances, origins)
 
 			compiler := env.requireNewCompiler(t, newCompiler, &wazeroir.CompilationResult{
-				NeedsAccessToDataInstances: true, Signature: &wasm.FunctionType{}})
+				HasDataInstances: true, Signature: &wasm.FunctionType{}})
 
 			err := compiler.compilePreamble()
 			require.NoError(t, err)
@@ -406,7 +406,7 @@ func TestCompiler_compileMemoryInit(t *testing.T) {
 			env.module().DataInstances = dataInstances
 
 			compiler := env.requireNewCompiler(t, newCompiler, &wazeroir.CompilationResult{
-				NeedsAccessToDataInstances: true, HasMemory: true,
+				HasDataInstances: true, HasMemory: true,
 				Signature: &wasm.FunctionType{}})
 
 			err := compiler.compilePreamble()
@@ -471,7 +471,7 @@ func TestCompiler_compileElemDrop(t *testing.T) {
 			}
 
 			compiler := env.requireNewCompiler(t, newCompiler, &wazeroir.CompilationResult{
-				NeedsAccessToElementInstances: true, Signature: &wasm.FunctionType{}})
+				HasElementInstances: true, Signature: &wasm.FunctionType{}})
 
 			err := compiler.compilePreamble()
 			require.NoError(t, err)
@@ -633,7 +633,7 @@ func TestCompiler_compileTableInit(t *testing.T) {
 			env.module().ElementInstances = elementInstances
 
 			compiler := env.requireNewCompiler(t, newCompiler, &wazeroir.CompilationResult{
-				NeedsAccessToElementInstances: true, HasTable: true,
+				HasElementInstances: true, HasTable: true,
 				Signature: &wasm.FunctionType{}})
 
 			err := compiler.compilePreamble()

internal/engine/compiler/impl_amd64.go

@@ -4992,7 +4992,7 @@ func (c *amd64Compiler) compileModuleContextInitialization() error {
 	}
 
 	// Update dataInstancesElement0Address.
-	if c.ir.NeedsAccessToDataInstances {
+	if c.ir.HasDataInstances {
 		// "tmpRegister = &moduleInstance.DataInstances[0]"
 		c.assembler.CompileMemoryToRegister(
 			amd64.MOVQ,
@@ -5008,7 +5008,7 @@ func (c *amd64Compiler) compileModuleContextInitialization() error {
 	}
 
 	// Update callEngine.moduleContext.elementInstancesElement0Address
-	if c.ir.NeedsAccessToElementInstances {
+	if c.ir.HasElementInstances {
 		// "tmpRegister = &moduleInstance.ElementInstnaces[0]"
 		c.assembler.CompileMemoryToRegister(
 			amd64.MOVQ,

internal/engine/compiler/impl_arm64.go

@@ -4323,7 +4323,7 @@ func (c *arm64Compiler) compileModuleContextInitialization() error {
 	}
 
 	// Update dataInstancesElement0Address.
-	if c.ir.NeedsAccessToDataInstances {
+	if c.ir.HasDataInstances {
 		// "tmpX = &moduleInstance.DataInstances[0]"
 		c.assembler.CompileMemoryToRegister(
 			arm64.MOVD,
@@ -4339,7 +4339,7 @@ func (c *arm64Compiler) compileModuleContextInitialization() error {
 	}
 
 	// Update callEngine.moduleContext.elementInstancesElement0Address
-	if c.ir.NeedsAccessToElementInstances {
+	if c.ir.HasElementInstances {
 		// "tmpX = &moduleInstance.DataInstances[0]"
 		c.assembler.CompileMemoryToRegister(
 			arm64.MOVD,

internal/integration_test/fuzzcases/fuzzcases_test.go

@@ -17,6 +17,8 @@ var (
 	case695 []byte
 	//go:embed testdata/696.wasm
 	case696 []byte
+	//go:embed testdata/699.wasm
+	case699 []byte
 )
 
 func newRuntimeCompiler() wazero.Runtime {
@@ -87,3 +89,26 @@ func Test696(t *testing.T) {
 		})
 	}
 }
+
+// Test699 ensures that accessing element instances and data instances works
+// without crash even when the access happens in the nested function call.
+func Test699(t *testing.T) {
+	if !platform.CompilerSupported() {
+		return
+	}
+
+	for _, tc := range []struct {
+		name string
+		r    wazero.Runtime
+	}{
+		{name: "compiler", r: newRuntimeCompiler()},
+		{name: "interpreter", r: newRuntimeInterpreter()},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			defer tc.r.Close(ctx)
+			_, err := tc.r.InstantiateModuleFromBinary(ctx, case699)
+			require.NoError(t, err)
+		})
+	}
+}

internal/integration_test/fuzzcases/testdata/699.wat

@@ -0,0 +1,10 @@
+(module
+  (func call 1) ;; call the function below.
+  (func ;; accessing the element/data instances in the subroutine.
+    elem.drop 0
+    data.drop 0
+  )
+  (start 0)
+  (elem func)
+  (data "x")
+)

internal/wazeroir/compiler.go

@@ -206,10 +206,10 @@ type CompilationResult struct {
 	HasMemory bool
 	// HasTable is true if the module from which this function is compiled has table declaration.
 	HasTable bool
-	// NeedsAccessToDataInstances is true if the function needs access to data instances via memory.init or data.drop instructions.
+	// HasDataInstances is true if the module has data instances which might be used by memory.init or data.drop instructions.
-	NeedsAccessToDataInstances bool
+	HasDataInstances bool
-	// NeedsAccessToDataInstances is true if the function needs access to element instances via table.init or elem.drop instructions.
+	// HasDataInstances is true if the module has element instances which might be used by table.init or elem.drop instructions.
-	NeedsAccessToElementInstances bool
+	HasElementInstances bool
 }
 
 func CompileFunctions(_ context.Context, enabledFeatures wasm.Features, module *wasm.Module) ([]*CompilationResult, error) {
@@ -220,7 +220,8 @@ func CompileFunctions(_ context.Context, enabledFeatures wasm.Features, module *
 		return nil, err
 	}
 
-	hasMemory, hasTable := mem != nil, len(tables) > 0
+	hasMemory, hasTable, hasDataInstances, hasElementInstances := mem != nil, len(tables) > 0,
+		len(module.DataSection) > 0, len(module.ElementSection) > 0
 
 	tableTypes := make([]wasm.ValueType, len(tables))
 	for i := range tableTypes {
@@ -242,6 +243,8 @@ func CompileFunctions(_ context.Context, enabledFeatures wasm.Features, module *
 		r.Types = module.TypeSection
 		r.HasMemory = hasMemory
 		r.HasTable = hasTable
+		r.HasDataInstances = hasDataInstances
+		r.HasElementInstances = hasElementInstances
 		r.Signature = sig
 		r.TableTypes = tableTypes
 		ret = append(ret, r)
@@ -1668,7 +1671,6 @@ operatorSwitch:
 			c.emit(
 				&OperationMemoryInit{DataIndex: dataIndex},
 			)
-			c.result.NeedsAccessToDataInstances = true
 		case wasm.OpcodeMiscDataDrop:
 			dataIndex, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
 			if err != nil {
@@ -1678,7 +1680,6 @@ operatorSwitch:
 			c.emit(
 				&OperationDataDrop{DataIndex: dataIndex},
 			)
-			c.result.NeedsAccessToDataInstances = true
 		case wasm.OpcodeMiscMemoryCopy:
 			c.pc += 2 // +2 to skip two memory indexes which are fixed to zero.
 			c.emit(
@@ -1704,7 +1705,6 @@ operatorSwitch:
 			c.emit(
 				&OperationTableInit{ElemIndex: elemIndex, TableIndex: tableIndex},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		case wasm.OpcodeMiscElemDrop:
 			elemIndex, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
 			if err != nil {
@@ -1714,7 +1714,6 @@ operatorSwitch:
 			c.emit(
 				&OperationElemDrop{ElemIndex: elemIndex},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		case wasm.OpcodeMiscTableCopy:
 			// Read the source table index.
 			dst, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
@@ -1732,7 +1731,6 @@ operatorSwitch:
 			c.emit(
 				&OperationTableCopy{SrcTableIndex: src, DstTableIndex: dst},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		case wasm.OpcodeMiscTableGrow:
 			// Read the source table index.
 			tableIndex, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
@@ -1743,7 +1741,6 @@ operatorSwitch:
 			c.emit(
 				&OperationTableGrow{TableIndex: tableIndex},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		case wasm.OpcodeMiscTableSize:
 			// Read the source table index.
 			tableIndex, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
@@ -1754,7 +1751,6 @@ operatorSwitch:
 			c.emit(
 				&OperationTableSize{TableIndex: tableIndex},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		case wasm.OpcodeMiscTableFill:
 			// Read the source table index.
 			tableIndex, num, err := leb128.DecodeUint32(bytes.NewReader(c.body[c.pc+1:]))
@@ -1765,7 +1761,6 @@ operatorSwitch:
 			c.emit(
 				&OperationTableFill{TableIndex: tableIndex},
 			)
-			c.result.NeedsAccessToElementInstances = true
 		default:
 			return fmt.Errorf("unsupported misc instruction in wazeroir: 0x%x", op)
 		}

internal/wazeroir/compiler_test.go

@@ -241,13 +241,13 @@ func TestCompile_BulkMemoryOperations(t *testing.T) {
 			&OperationDataDrop{1},                 // []
 			&OperationBr{Target: &BranchTarget{}}, // return!
 		},
-		HasMemory:                  true,
+		HasMemory:        true,
-		NeedsAccessToDataInstances: true,
+		HasDataInstances: true,
-		LabelCallers:               map[string]uint32{},
+		LabelCallers:     map[string]uint32{},
-		Signature:                  v_v,
+		Signature:        v_v,
-		Functions:                  []wasm.Index{0},
+		Functions:        []wasm.Index{0},
-		Types:                      []*wasm.FunctionType{v_v},
+		Types:            []*wasm.FunctionType{v_v},
-		TableTypes:                 []wasm.RefType{},
+		TableTypes:       []wasm.RefType{},
 	}
 
 	res, err := CompileFunctions(ctx, wasm.FeatureBulkMemoryOperations, module)