Repository: tetratelabs/wazero (go)

Issue:
wasi_snapshot_preview1.random_get nil pointer dereference (Go 1.21rc2 built application panics)
**Describe the bug**

A basic application build with Go 1.21rc2 when loaded into the wazero runtime panics when the `random_get` wasi host function is called.

**To Reproduce**

Build a wasm that doesn't do anything:

```go
package main
func main() { }
```

```
GOOS=wasip1 GOARCH=wasm go build -o main.wasm main.go
```

Build a host using wazero v1.2.1 a host that supports wasip1:

```go
package main

import (
	"context"
	_ "embed"
	"fmt"

	"github.com/tetratelabs/wazero"
	"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
)

//go:embed main.wasm
var wasm []byte

func main() {
	ctx := context.Background()

	r := wazero.NewRuntime(ctx)
	defer r.Close(ctx)

	_, err := wasi_snapshot_preview1.Instantiate(ctx, r)
	if err != nil {
		panic(err)
	}

	mod, err := r.Instantiate(ctx, wasm)
	if err != nil {
		panic(err)
	}

	f := mod.ExportedFunction("_start")
	if f != nil {
		res, err := f.Call(ctx)
		if err != nil {
			panic(err)
		}
		fmt.Println("start =", res)
	}
}
```

Run the host Go application.

**Expected behavior**

The application to execute.

**Actual behavior**
```
go run ./host
panic: runtime error: invalid memory address or nil pointer dereference (recovered by wazero)
wasm stack trace:
        wasi_snapshot_preview1.random_get(i32,i32) i32
        .runtime.random_get(i32) i32
        .runtime.getRandomData(i32) i32
        .runtime.alginit(i32) i32
        .runtime.schedinit(i32) i32
        .runtime.rt0_go(i32) i32
        ._rt0_wasm_wasip1()

Go runtime stack trace:
goroutine 1 [running]:
runtime/debug.Stack()
        /Users/leighmcculloch/.local/bin/go/latest/src/runtime/debug/stack.go:24 +0x64
github.com/tetratelabs/wazero/internal/wasmdebug.(*stackTrace).FromRecovered(0x140004277e0?, {0x100a6dcc0?, 0x100d1d8a0?})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/wasmdebug/debug.go:139 +0xc4
github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).deferredOnCall(0x14000000360, {0x100aa3740, 0x100d5cbe0}, 0x100aa5358?, {0x100a6dcc0, 0x100d1d8a0})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/engine/compiler/engine.go:855 +0x37c
github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).call.func1()
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/engine/compiler/engine.go:732 +0x68
panic({0x100a6dcc0?, 0x100d1d8a0?})
        /Users/leighmcculloch/.local/bin/go/latest/src/runtime/panic.go:914 +0x218
github.com/tetratelabs/wazero/internal/sys.(*Context).RandSource(...)
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/sys/sys.go:109
github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1.randomGetFn({0x0?, 0x100d567b0?}, {0x100aa5358?, 0x14000163a40?}, {0x14000134110?, 0x100a72c40?, 0x14000427c01?})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/imports/wasi_snapshot_preview1/random.go:41 +0x3c
github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1.wasiFunc.Call(0x1289dc3e8?, {0x100aa3740?, 0x100d5cbe0?}, {0x100aa5358?, 0x14000163a40?}, {0x14000134110, 0x2, 0x0?})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/imports/wasi_snapshot_preview1/wasi.go:293 +0x58
github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).execWasmFunction(0x14000000360, {0x100aa3740, 0x100d5cbe0}, 0x100e5c108?)
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/engine/compiler/engine.go:1024 +0x118
github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).call(0x14000000360, {0x100aa3740, 0x100d5cbe0}, {0x0?, 0x14000427e48?, 0x10099f84c?}, {0x0, 0x0, 0x0})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/engine/compiler/engine.go:747 +0x224
github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).Call(0x14000163a40?, {0x100aa3740?, 0x100d5cbe0?}, {0x0?, 0x153849?, 0x153849?})
        /Users/leighmcculloch/.local/gopath/pkg/mod/github.com/tetratelabs/wazero@v1.2.1/internal/engine/compiler/engine.go:701 +0xc0
main.main()
        /Users/leighmcculloch/Code/wazero-play/host/main.go:33 +0x13c


goroutine 1 [running]:
main.main()
        /Users/leighmcculloch/Code/wazero-play/host/main.go:35 +0x1c4
```

**Screenshots**
N/A

**Environment (please complete the relevant information):**
- Go version: 1.21rc2
- wazero Version: v1.2.1
- Host architecture: macOS arm64
- Runtime mode: interpreter

**Additional context**
Add any other context about the problem here.


Pull request description:
Exposes `Module.IsClosed` to prevent calling functions when closed
This adds `Module.IsClosed` to allow callers to prevent calling functions that would fail due to closed state.

Fixes #1561

Patch:
diff --git a/RATIONALE.md b/RATIONALE.md
index 9e8a3f49..8952ced9 100644
--- a/RATIONALE.md
+++ b/RATIONALE.md
@@ -484,26 +484,26 @@ https://github.com/bytecodealliance/wasmtime/blob/v0.29.0/crates/lightbeam/src/m
 
 ## Exit
 
-### Why do we return a `sys.ExitError` on exit code zero?
-
-It may be surprising to find an error returned on success (exit code zero).
-This can be explained easier when you think of function returns: When results
-aren't empty, then you must return an error. This is trickier to explain when
-results are empty, such as the case in the "_start" function in WASI.
-
-The main rationale for returning an exit error even if the code is success is
-that the module is no longer functional. For example, function exports would
-error later. In cases like these, it is better to handle errors where they
-occur.
-
-Luckily, it is not common to exit a module during the "_start" function. For
-example, the only known compilation target that does this is Emscripten. Most,
-such as Rust, TinyGo, or normal wasi-libc, don't. If they did, it would
-invalidate their function exports. This means it is unlikely most compilers
-will change this behavior.
-
-In summary, we return a `sys.ExitError` to the caller whenever we get it, as it
-properly reflects the state of the module, which would be closed on this error.
+### Why do we only return a `sys.ExitError` on a non-zero exit code?
+
+It is reasonable to think an exit error should be returned, even if the code is
+success (zero). Even on success, the module is no longer functional. For
+example, function exports would error later. However, wazero does not. The only
+time `sys.ExitError` is on error (non-zero).
+
+This decision was to improve performance and ergonomics for guests that both
+use WASI (have a `_start` function), and also allow custom exports.
+Specifically, Rust, TinyGo and normal wasi-libc, don't exit the module during
+`_start`. If they did, it would invalidate their function exports. This means
+it is unlikely most compilers will change this behavior.
+
+`GOOS=waspi1` from Go 1.21 does exit during `_start`. However, it doesn't
+support other exports besides `_start`, and `_start` is not defined to be
+called multiple times anyway.
+
+Since `sys.ExitError` is not always returned, we added `Module.IsClosed` for
+defensive checks. This helps integrators avoid calling functions which will
+always fail.
 
 ### Why panic with `sys.ExitError` after a host function exits?
 
diff --git a/api/wasm.go b/api/wasm.go
index 38728b42..f611d3ea 100644
--- a/api/wasm.go
+++ b/api/wasm.go
@@ -150,6 +150,10 @@ type Module interface {
 	Memory() Memory
 
 	// ExportedFunction returns a function exported from this module or nil if it wasn't.
+	//
+	// Note: The default wazero.ModuleConfig attempts to invoke `_start`, which
+	// in rare cases can close the module. When in doubt, check IsClosed prior
+	// to invoking a function export after instantiation.
 	ExportedFunction(name string) Function
 
 	// ExportedFunctionDefinitions returns all the exported function
@@ -190,6 +194,21 @@ type Module interface {
 	// Closer closes this module by delegating to CloseWithExitCode with an exit code of zero.
 	Closer
 
+	// IsClosed returns true if the module is closed, so no longer usable.
+	//
+	// This can happen for the following reasons:
+	//   - Closer was called directly.
+	//   - A guest function called Closer indirectly, such as `_start` calling
+	//     `proc_exit`, which internally closed the module.
+	//   - wazero.RuntimeConfig `WithCloseOnContextDone` was enabled and a
+	//     context completion closed the module.
+	//
+	// Where any of the above are possible, check this value before calling an
+	// ExportedFunction, even if you didn't formerly receive a sys.ExitError.
+	// sys.ExitError is only returned on non-zero code, something that closes
+	// the module successfully will not result it one.
+	IsClosed() bool
+
 	internalapi.WazeroOnly
 }
 
diff --git a/internal/wasm/module_instance.go b/internal/wasm/module_instance.go
index 20cdcd96..7527f253 100644
--- a/internal/wasm/module_instance.go
+++ b/internal/wasm/module_instance.go
@@ -106,6 +106,11 @@ func (m *ModuleInstance) CloseWithExitCode(ctx context.Context, exitCode uint32)
 	return m.ensureResourcesClosed(ctx)
 }
 
+// IsClosed implements the same method as documented on api.Module.
+func (m *ModuleInstance) IsClosed() bool {
+	return atomic.LoadUint64(&m.Closed) != 0
+}
+
 func (m *ModuleInstance) closeWithExitCodeWithoutClosingResource(exitCode uint32) (err error) {
 	if !m.setExitCode(exitCode, exitCodeFlagResourceNotClosed) {
 		return nil // not an error to have already closed
diff --git a/runtime.go b/runtime.go
index 57147875..ce873416 100644
--- a/runtime.go
+++ b/runtime.go
@@ -320,7 +320,8 @@ func (r *runtime) InstantiateModule(
 		return
 	}
 
-	// Attach the code closer so that anything afterwards closes the compiled code when closing the module.
+	// Attach the code closer so that anything afterward closes the compiled
+	// code when closing the module.
 	if code.closeWithModule {
 		mod.(*wasm.ModuleInstance).CodeCloser = code
 	}


Test patch:
diff --git a/experimental/wazerotest/wazerotest.go b/experimental/wazerotest/wazerotest.go
index 722a9fd5..2bbaf7a4 100644
--- a/experimental/wazerotest/wazerotest.go
+++ b/experimental/wazerotest/wazerotest.go
@@ -52,14 +52,17 @@ func NewModule(memory *Memory, functions ...*Function) *Module {
 	return &Module{Functions: functions, ExportMemory: memory}
 }
 
+// String implements fmt.Stringer.
 func (m *Module) String() string {
 	return "module[" + m.ModuleName + "]"
 }
 
+// Name implements the same method as documented on api.Module.
 func (m *Module) Name() string {
 	return m.ModuleName
 }
 
+// Memory implements the same method as documented on api.Module.
 func (m *Module) Memory() api.Memory {
 	if m.ExportMemory != nil {
 		m.once.Do(m.initialize)
@@ -68,16 +71,19 @@ func (m *Module) Memory() api.Memory {
 	return nil
 }
 
+// ExportedFunction implements the same method as documented on api.Module.
 func (m *Module) ExportedFunction(name string) api.Function {
 	m.once.Do(m.initialize)
 	return m.exportedFunctions[name]
 }
 
+// ExportedFunctionDefinitions implements the same method as documented on api.Module.
 func (m *Module) ExportedFunctionDefinitions() map[string]api.FunctionDefinition {
 	m.once.Do(m.initialize)
 	return m.exportedFunctionDefinitions
 }
 
+// ExportedMemory implements the same method as documented on api.Module.
 func (m *Module) ExportedMemory(name string) api.Memory {
 	if m.ExportMemory != nil && name == "memory" {
 		m.once.Do(m.initialize)
@@ -86,41 +92,55 @@ func (m *Module) ExportedMemory(name string) api.Memory {
 	return nil
 }
 
+// ExportedMemoryDefinitions implements the same method as documented on api.Module.
 func (m *Module) ExportedMemoryDefinitions() map[string]api.MemoryDefinition {
 	m.once.Do(m.initialize)
 	return m.exportedMemoryDefinitions
 }
 
+// ExportedGlobal implements the same method as documented on api.Module.
 func (m *Module) ExportedGlobal(name string) api.Global {
 	m.once.Do(m.initialize)
 	return m.exportedGlobals[name]
 }
 
-func (m *Module) NumFunction() int {
-	return len(m.Functions)
+// Close implements the same method as documented on api.Closer.
+func (m *Module) Close(ctx context.Context) error {
+	return m.CloseWithExitCode(ctx, 0)
 }
 
-func (m *Module) Function(i int) api.Function {
-	m.once.Do(m.initialize)
-	return m.Functions[i]
+// CloseWithExitCode implements the same method as documented on api.Closer.
+func (m *Module) CloseWithExitCode(ctx context.Context, exitCode uint32) error {
+	atomic.CompareAndSwapUint64(&m.exitStatus, 0, exitStatusMarker|uint64(exitCode))
+	return nil
+}
+
+// IsClosed implements the same method as documented on api.Module.
+func (m *Module) IsClosed() bool {
+	_, exited := m.ExitStatus()
+	return exited
 }
 
+// NumGlobal implements the same method as documented on experimental.InternalModule.
 func (m *Module) NumGlobal() int {
 	return len(m.Globals)
 }
 
+// Global implements the same method as documented on experimental.InternalModule.
 func (m *Module) Global(i int) api.Global {
 	m.once.Do(m.initialize)
 	return m.Globals[i]
 }
 
-func (m *Module) Close(ctx context.Context) error {
-	return m.CloseWithExitCode(ctx, 0)
+// Below are undocumented extensions
+
+func (m *Module) NumFunction() int {
+	return len(m.Functions)
 }
 
-func (m *Module) CloseWithExitCode(ctx context.Context, exitCode uint32) error {
-	atomic.CompareAndSwapUint64(&m.exitStatus, 0, exitStatusMarker|uint64(exitCode))
-	return nil
+func (m *Module) Function(i int) api.Function {
+	m.once.Do(m.initialize)
+	return m.Functions[i]
 }
 
 func (m *Module) ExitStatus() (exitCode uint32, exited bool) {
diff --git a/internal/wasm/module_instance_test.go b/internal/wasm/module_instance_test.go
index 44b1998b..cc1f2c91 100644
--- a/internal/wasm/module_instance_test.go
+++ b/internal/wasm/module_instance_test.go
@@ -100,6 +100,9 @@ func TestModuleInstance_Close(t *testing.T) {
 
 				require.Equal(t, tc.expectedClosed, m.Closed)
 
+				// Outside callers should be able to know it was closed.
+				require.True(t, m.IsClosed())
+
 				// Verify our intended side-effect
 				require.Nil(t, s.Module(moduleName))