wazero: 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:

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:

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.

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 27 (3 by maintainers)

Commits related to this issue

Most upvoted comments

p.s. wazero takes 1.0 very seriously. If we removed _start we would break everyone. We’ve automatically called this on their behalf for well over a year.

When wasip2 is actionable (maybe 2024) we’ll likely add that to the list so that people can transparently use it. Bear in mind that wasip2 is based on components and so there’s a lot more going to happen than just the “_start” -> “run” thing. My guess is wasip2 will be defined by https://github.com/WebAssembly/wasi-cli but really we need to see any concrete use of this before making decisions about it. Since preview 2 is scheduled for fall, we’ll defer that as guessing hasn’t been correct for us so far…

2. … remove them from exports

I don’t have a strong opinion on most of the options, however want to point out that option two will cause wazero’s representation of a module to be incomplete. It’s reasonable to ask wazero for all exported functions. Once wazero is omitting exported functions it isn’t as useful as a composable utility. At least we’re replacing one surprising behavior with another surprising behavior. Hiding functions in some cases and not other is pretty unintuitive.

since all options require docs I raised this https://github.com/tetratelabs/wazero/pull/1568

We can adjust those docs if we do something else about it.

Leaving the start function in the export list but making it return a helpful error if it’s ever called by user code might be a reasonable middle ground.

I also see nothing inherently wrong about invoking _start more than once, even though it may not be exactly what you want.

That may be true for WASM conceptually, but it’s going to depend heavily on what your WASM+WASI compiler generates.

go calls exit at the end of main and then it’s done. Also for sure it’s doing init stuff (running init functions), and probably not guarding those for multiple runs, which violates the Go spec.

wasi-sdk clang is initing wasi-libc and running other constructor functions even before calling your main. Any of those may not expect to run multiple times, and not from a clean state.

tinygo, rust I have no idea, but I’ve seen bugs about running _start multiple times on wasmtime for stuff compiled with rust.

So for WASI commands it’s expected/valid that they expect _start to be called exactly once, and likely that nothing else may be called afterwards. WASI reactors expect _initialize, then you may call other exported functions.

So I do think @codefromthecrypt suggestion is a valid way forward, at least from my limited WASI understanding.

Sorry for fat fingering the close button on mobile. 😬

Go wasip1 explicitly calls proc_exit, even for a “no error” return:

./wazero/cmd/wazero/wazero run -hostlogging all main.wasm 
--> ._rt0_wasm_wasip1()
	==> wasi_snapshot_preview1.random_get(buf=810944,buf_len=32)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.random_get(buf=810736,buf_len=8)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156302238933,errno=ESUCCESS)
	==> wasi_snapshot_preview1.args_sizes_get(result.argc=830492,result.argv_len=830488)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.args_get(argv=21020680,argv_buf=21020688)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.environ_sizes_get(result.environc=830480,result.environv_len=830484)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156323231365,errno=ESUCCESS)
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156323238570,errno=ESUCCESS)
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156323332489,errno=ESUCCESS)
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156323391276,errno=ESUCCESS)
	==> wasi_snapshot_preview1.clock_time_get(id=monotonic,precision=0)
	<== (timestamp=380156323418316,errno=ESUCCESS)
	==> wasi_snapshot_preview1.proc_exit(rval=0)

Calling _start repeatedly is discouraged (because libc init runs multiple times) but wont panic on a minimal C program (built with wasi-sdk), if you return 0:

int main(int argc, char *argv[]) {
  return 0;
}
./wazero/cmd/wazero/wazero run -hostlogging all a.out               
--> ._start()
	==> wasi_snapshot_preview1.args_sizes_get(result.argc=67064,result.argv_len=67068)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.args_get(argv=67104,argv_buf=67088)
	<== errno=ESUCCESS
<--

With return 1:

int main(int argc, char *argv[]) {
  return 1;
}
./wazero/cmd/wazero/wazero run -hostlogging all a.out
--> ._start()
	==> wasi_snapshot_preview1.args_sizes_get(result.argc=67064,result.argv_len=67068)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.args_get(argv=67104,argv_buf=67088)
	<== errno=ESUCCESS
	==> wasi_snapshot_preview1.proc_exit(rval=1)

Back to Go:

This also means you wouldn’t be able to call other exported functions after main finishes, which is one (albeit wrong) way WASM/WASI is typically used. But Go doesn’t (yet) export functions, so no issue there; they’ll have to figure out an execution model once they do.

@leighmcculloch That could work… one thing we could do is just filter out any function name in the start functions. That way other init functions could not be possible to run twice as you couldn’t access them (there are others besides _start, just we default to that)