Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: give clear error when a wasmexport function is called before module initialization #71240

Closed
snaffi opened this issue Jan 13, 2025 · 13 comments
Assignees
Labels
arch-wasm WebAssembly issues BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@snaffi
Copy link

snaffi commented Jan 13, 2025

Go version

go version go1.24rc1 linux/arm64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE='on'
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/.jbdevcontainer/go-build'
GODEBUG=''
GOENV='/.jbdevcontainer/config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3652802248=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='arm64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/IdeaProjects/wasm-1-24/go.mod'
GOMODCACHE='/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/.jbdevcontainer/config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_arm64'
GOVCS=''
GOVERSION='go1.24rc1'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I'm trying to call simple wasm exported Go function from wasmtime and wazero runtimes.

wasm.go

GOOS=wasip1 GOARCH=wasm go build -o out.wasm -buildmode=c-shared ./wasm/

//go:build wasm

package main

import "fmt"

//go:wasmexport SuperFunction
func SuperFunction(x, y int32) int32 {
	return x + y
} // OK

func main() {

}

wasmtime_main.go

package main

import (
	"context"
	_ "embed"
	"fmt"
	"log"
	"os"
	"unsafe"

	"github.com/bytecodealliance/wasmtime-go"
)

func memoryLimit(megabytes uint32) uint32 {
	const wazeroPageSize = 64
	kilobytes := megabytes * 1024
	return kilobytes / wazeroPageSize
}

//go:embed out.wasm
var wasmModule []byte

func main() {
	// Almost all operations in wasmtime require a contextual `store`
	// argument to share, so create that first
	engine := wasmtime.NewEngine()
	//store.SetWasi()

	// Once we have our binary `wasm` we can compile that into a `*Module`
	// which represents compiled JIT code.
	module, err := wasmtime.NewModule(engine, wasmModule)
	if err != nil {
		log.Fatal(err)
	}

	// Create a linker with WASI functions defined within it
	linker := wasmtime.NewLinker(engine)
	err = linker.DefineWasi()
	if err != nil {
		log.Fatal(err)
	}

	// Configure WASI imports to write stdout into a file, and then create
	// a `Store` using this wasi configuration.
	wasiConfig := wasmtime.NewWasiConfig()
	store := wasmtime.NewStore(engine)
	store.SetWasi(wasiConfig)
	instance, err := linker.Instantiate(store, module)
	if err != nil {
		log.Fatal(err)
	}

	// After we've instantiated we can lookup our `run` function and call
	// it.
	run := instance.GetFunc(store, "SuperFunction")
	_, err = run.Call(store, 96, 55)
	if err != nil {
		log.Fatal(err)
	}
}

wazero_main.go

package main

import (
	"context"
	_ "embed"
	"fmt"
	"log"
	"os"
	"unsafe"

	"github.com/tetratelabs/wazero"
	"github.com/tetratelabs/wazero/api"
	"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
)

func main() {
	ctx := context.Background()
	runtime := wazero.NewRuntimeWithConfig(
		ctx,
		wazero.
			NewRuntimeConfig().
			WithCoreFeatures(api.CoreFeaturesV2).
			WithMemoryLimitPages(memoryLimit(256)),
	)
	_, err := wasi_snapshot_preview1.Instantiate(ctx, runtime)
	if err != nil {
		panic(err)
	}

	builder := runtime.NewHostModuleBuilder("env")
	wasi_snapshot_preview1.NewFunctionExporter().ExportFunctions(builder)
	_, err = builder.Instantiate(ctx)
	if err != nil {
		panic(err)
	}

	compiledModule, err := runtime.CompileModule(ctx, wasmModule)
	if err != nil {
		panic(fmt.Errorf("can't compile WASM module: %w", err))
	}

	mod, err := runtime.InstantiateModule(
		ctx,
		compiledModule,
		wazero.
			NewModuleConfig().
			WithStdout(os.Stdout).
			WithStderr(os.Stderr),
	)
	if err != nil {
		panic(fmt.Errorf("can't instantiate module: %w", err))
	}

	funcName := "SuperFunction"
	targetFunc := mod.ExportedFunction(funcName)
	if targetFunc == nil {
		panic(fmt.Errorf("target function not found: %s", funcName))
	}

	mod.Memory().Grow(2)
	res, err := targetFunc.Call(ctx, 96, 55)
	//res, err := targetFunc.Call(ctx)
	fmt.Println(res, err)

	fmt.Println(targetFunc.Definition().DebugName())
	fmt.Println(targetFunc.Definition().Name())
	fmt.Println(targetFunc.Definition().ParamTypes())
	fmt.Println(targetFunc.Definition().ParamNames())

}

What did you see happen?

I got out of bounds memory access on wazero and wasmtime runtime

What did you expect to see?

I expect function execution without any error.

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 13, 2025
@mknyszek
Copy link
Contributor

CC @cherrymui

@mknyszek mknyszek added this to the Go1.24 milestone Jan 13, 2025
@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jan 13, 2025
@mknyszek
Copy link
Contributor

Marking as a release-blocker since this may be a bug in a new feature shipping in Go 1.24.

@cherrymui
Copy link
Member

The main issue is that the module must be initialized first, as documented in WASI reactor doc:

If an _initialize export is present, reactor instances may assume that it will be called by the environment at most once, and that none of their other exports are accessed before that call.

E.g. for the wazero version, applying this change

@@ -45,7 +49,8 @@ func main() {
                wazero.
                        NewModuleConfig().
                        WithStdout(os.Stdout).
-                       WithStderr(os.Stderr),
+                       WithStderr(os.Stderr).
+                       WithStartFunctions("_initialize"),
        )
        if err != nil {
                panic(fmt.Errorf("can't instantiate module: %w", err))

(along with fixing compilation errors like unused imports and missing some definitions), it works as expected.

This issue is essentially another occurrence of #65199 (comment) . Perhaps we should make it fail clearly when an exported function is called before the runtime is initialized. I'm retargeting this issue for the error detection, and perhaps documentation.

I'm also a bit surprised that the WASI runtimes don't call _initialize automatically (it does call _start if present). Perhaps they should? @golang/wasm do any of you have any insight for this?

@cherrymui cherrymui added arch-wasm WebAssembly issues compiler/runtime Issues related to the Go compiler and/or runtime. labels Jan 14, 2025
@cherrymui cherrymui changed the title wasm: simple call of wasm exported function raises an error "out of bounds memory access" runtime: give clear error when a wasmexport function is called before module initialization Jan 14, 2025
@mknyszek
Copy link
Contributor

mknyszek commented Jan 14, 2025

Based on @cherrymui's comment, removing release-blocker. It doesn't sound like this is necessarily a bug on our side?

@mknyszek mknyszek modified the milestones: Go1.24, Backlog Jan 14, 2025
@Zxilly
Copy link
Member

Zxilly commented Jan 14, 2025

Looks like it's historical?

https://github.com/tetratelabs/wazero/blob/610c202ec48f3a7c729f2bf11707330127ab3689/RATIONALE.md?plain=1#L417-L459

I may check the behavior of other wasm runtimes later.

@snaffi
Copy link
Author

snaffi commented Jan 15, 2025

@cherrymui, thank you for the response! Setting the proper startup function resolved the issue.

I have a question about WASM in Go 1.24. The documentation and compiled WASM file don’t clarify how to allocate memory for writing shared data (e.g., JSON).

In TinyGo, memory allocation is possible via the C-API (see example). It also allows exporting malloc and free to the host (example).

Similarly, AssemblyScript exports __alloc and __free for memory allocation.

@cherrymui
Copy link
Member

I have a question about WASM in Go 1.24. The documentation and compiled WASM file don’t clarify how to allocate memory for writing shared data (e.g., JSON).

The approaches I can think of are

  1. allocate memory in a Go wasmexport function (possibly fill in data), return the pointer to the host, for the host to write or read.
  2. allocate memory in Go, pass the buffer to the host via a wasmimport function (defined in the host, called from Go) for the host to read or write.

cc @golang/wasm for more suggestions, and if there is a need for a more standard interface.

@cherrymui cherrymui self-assigned this Jan 16, 2025
@Zxilly
Copy link
Member

Zxilly commented Jan 16, 2025

This may need a new issue for malloc proposal?

@cherrymui
Copy link
Member

Maybe. Or we can just do it manually, and perhaps document common best practices on a wiki page.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/643115 mentions this issue: cmd/internal/obj/wasm, runtime: detect wasmexport call before runtime initialization

@cherrymui cherrymui modified the milestones: Backlog, Go1.24 Jan 16, 2025
@dmitshur dmitshur removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 16, 2025
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 16, 2025
@johanbrandhorst
Copy link
Member

I'm also a bit surprised that the WASI runtimes don't call _initialize automatically (it does call _start if present). Perhaps they should? @golang/wasm do any of you have any insight for this?

The use of the _initialize function is documented in the wasip1 spec: https://github.com/WebAssembly/WASI/blob/5cf84da666f5f8f35834488720e970c318d35f76/legacy/application-abi.md#current-unstable-abi. I would argue that the runtime should detect that _initialize is present and call it automatically in this case, as implied by the sentence

If an _initialize export is present, reactor instances may assume that it will be called by the environment at most once, and that none of their other exports are accessed before that call.

in the above document. CC @mathetake for some context from the Wazero side.

@Zxilly
Copy link
Member

Zxilly commented Jan 20, 2025

_initialize was marked as unstable abi. I guess that explains why not all runtimes provide support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly issues BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

8 participants