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

spectest: reproduce segfault #50

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 7 additions & 18 deletions .github/workflows/integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ jobs:

zig:
needs: build_zig_test_binary
name: Zig (${{ matrix.os.name }}, ${{ matrix.arch }}, ${{ matrix.compiler }})
name: Zig (${{ matrix.os.name }}, ${{ matrix.arch }})
runs-on: ${{ matrix.os.version }}
strategy:
fail-fast: false # don't fail fast as sometimes failures are arch/OS specific
Expand All @@ -92,18 +92,15 @@ jobs:
name: macOS
- version: windows-2022
name: Windows
compiler: [baseline]
arch: [amd64]
include:
- os:
version: macos-14
name: macOS
compiler: optimizing
arch: "arm64"
- os:
version: ubuntu-22.04
name: Ubuntu
compiler: optimizing
arch: "amd64"
go-version: "1.21"

Expand All @@ -129,7 +126,7 @@ jobs:
- name: Run built test binaries
run: |
cd ${{ env.STDLIB_TESTS }}
go test -bench='BenchmarkZig/${{ matrix.compiler }}'
go test -bench='BenchmarkZig' -timeout=20m

build_tinygo_test_binary:
name: Build TinyGo test binary
Expand Down Expand Up @@ -168,7 +165,7 @@ jobs:

tinygo:
needs: build_tinygo_test_binary
name: TinyGo (${{ matrix.os.name }}, ${{ matrix.arch }}, ${{ matrix.compiler }})
name: TinyGo (${{ matrix.os.name }}, ${{ matrix.arch }})
runs-on: ${{ matrix.os.version }}
strategy:
fail-fast: false # don't fail fast as sometimes failures are arch/OS specific
Expand All @@ -182,18 +179,15 @@ jobs:
name: macOS
- version: windows-2022
name: Windows
compiler: [baseline]
arch: [amd64]
include:
- os:
version: macos-14
name: macOS
compiler: optimizing
arch: "arm64"
- os:
version: ubuntu-22.04
name: Ubuntu
compiler: optimizing
arch: "amd64"
go-version: "1.21"

Expand All @@ -219,7 +213,7 @@ jobs:
- name: Run test binaries
run: |
cd ${{ env.STDLIB_TESTS }}
go test -bench='BenchmarkTinyGo/${{ matrix.compiler }}'
go test -bench='BenchmarkTinyGo' -timeout=20m

wasi-testsuite:
name: wasi-testsuite
Expand Down Expand Up @@ -280,7 +274,7 @@ jobs:
go_tests:
# Due to the embedding of the GOROOT of the building env(https://github.com/golang/go/blob/3c59639b902fada0a2e5a6a35bafd10fc9183b89/src/os/os_test.go#L112),
# we have to build and cache on each OS unlike others in this file.
name: Go-${{ matrix.go-version }} (${{ matrix.os.name }}, ${{ matrix.arch }}, ${{ matrix.compiler }})
name: Go-${{ matrix.go-version }} (${{ matrix.os.name }}, ${{ matrix.arch }})
runs-on: ${{ matrix.os.version }}
strategy:
fail-fast: false # don't fail fast as sometimes failures are arch/OS specific
Expand All @@ -294,7 +288,6 @@ jobs:
name: macOS
- version: windows-2022
name: Windows
compiler: [baseline]
arch: [amd64]
go-version:
- "1.21"
Expand All @@ -303,25 +296,21 @@ jobs:
- os:
version: macos-14
name: macOS
compiler: optimizing
arch: "arm64"
go-version: "1.21"
- os:
version: macos-14
name: macOS
compiler: optimizing
arch: "arm64"
go-version: "1.22"
- os:
version: ubuntu-22.04
name: Ubuntu
compiler: optimizing
arch: "amd64"
go-version: "1.21"
- os:
version: ubuntu-22.04
name: Ubuntu
compiler: optimizing
arch: "amd64"
go-version: "1.22"

Expand Down Expand Up @@ -358,10 +347,10 @@ jobs:
- name: Run built test binaries
run: |
cd ${{ env.STDLIB_TESTS }}
go test -bench='BenchmarkWasip1/${{ matrix.compiler }}'
go test -bench='BenchmarkWasip1' -timeout=20m

libsodium:
name: libsodium (${{ matrix.os.name }}, ${{ matrix.os.arch }}, optimizing)
name: libsodium (${{ matrix.os.name }}, ${{ matrix.os.arch }})
runs-on: ${{ matrix.os.version }}
strategy:
fail-fast: false # don't fail fast as sometimes failures are arch/OS specific
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ jobs:
# windows-2022 is missing osslsigncode (no issue, yet)
- name: "Install osslsigncode, infozip; setup wix"
run: |
# Find "C:\Program Files (x86)\WiX Toolset <version>\"
WIXDIR=`ls '/c/Program Files (x86)'|grep WiX`
WIXBIN="C:\\Program Files (x86)\\$WIXDIR\\bin"
echo WIXBIN=$WIXBIN
echo $WIXBIN >> $GITHUB_PATH
choco install osslsigncode -y
choco install zip -y
echo "C:\Program Files (x86)\WiX Toolset v3.11\bin" >> $GITHUB_PATH

- name: Download Windows code signing certificate
env:
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ go_test_options ?= -timeout 300s
ensureCompilerFastest := -ldflags '-X github.com/tetratelabs/wazero/internal/integration_test/vs.ensureCompilerFastest=true'
.PHONY: bench
bench:
@go test -run=NONE -benchmem -bench=. ./internal/engine/compiler/...
@go build ./internal/integration_test/bench/...
@# Don't use -test.benchmem as it isn't accurate when comparing against CGO libs
@for d in vs/time vs/wasmedge vs/wasmtime ; do \
Expand Down
33 changes: 32 additions & 1 deletion RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,38 @@ If a module reaches this limit, an error is returned at the compilation phase.

## Compiler engine implementation

See [compiler/RATIONALE.md](internal/engine/compiler/RATIONALE.md).
### Why it's safe to execute runtime-generated machine codes against async Goroutine preemption

Goroutine preemption is the mechanism of the Go runtime to switch goroutines contexts on an OS thread.
There are two types of preemption: cooperative preemption and async preemption. The former happens, for example,
when making a function call, and it is not an issue for our runtime-generated functions as they do not make
direct function calls to Go-implemented functions. On the other hand, the latter, async preemption, can be problematic
since it tries to interrupt the execution of Goroutine at any point of function, and manipulates CPU register states.

Fortunately, our runtime-generated machine codes do not need to take the async preemption into account.
All the assembly codes are entered via the trampoline implemented as Go Assembler Function (e.g. [arch_amd64.s](./arch_amd64.s)),
and as of Go 1.20, these assembler functions are considered as _unsafe_ for async preemption:
- https://github.com/golang/go/blob/go1.20rc1/src/runtime/preempt.go#L406-L407
- https://github.com/golang/go/blob/9f0234214473dfb785a5ad84a8fc62a6a395cbc3/src/runtime/traceback.go#L227

From the Go runtime point of view, the execution of runtime-generated machine codes is considered as a part of
that trampoline function. Therefore, runtime-generated machine code is also correctly considered unsafe for async preemption.

## Why context cancellation is handled in Go code rather than native code

Since [wazero v1.0.0-pre.9](https://github.com/tetratelabs/wazero/releases/tag/v1.0.0-pre.9), the runtime
supports integration with Go contexts to interrupt execution after a timeout, or in response to explicit cancellation.
This support is internally implemented as a special opcode `builtinFunctionCheckExitCode` that triggers the execution of
a Go function (`ModuleInstance.FailIfClosed`) that atomically checks a sentinel value at strategic points in the code
(e.g. [within loops][checkexitcode_loop]).

[It _is indeed_ possible to check the sentinel value directly, without leaving the native world][native_check], thus sparing some cycles;
however, because native code never preempts (see section above), this may lead to a state where the other goroutines
never get the chance to run, and thus never get the chance to set the sentinel value; effectively preventing
cancellation from taking place.

[checkexitcode_loop]: https://github.com/tetratelabs/wazero/blob/86444c67a37dbf9e693ae5b365901f64968d9025/internal/wazeroir/compiler.go#L467-L476
[native_check]: https://github.com/tetratelabs/wazero/issues/1409

## Golang patterns

Expand Down
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ during `Runtime.CompileModule`. This means your WebAssembly functions execute
natively at runtime. Compiler is faster than Interpreter, often by order of
magnitude (10x) or more. This is done without host-specific dependencies.

If interested, check out the [RATIONALE.md][8] and help us optimize further!

### Conformance

Both runtimes pass WebAssembly Core [1.0][7] and [2.0][14] specification tests
Expand Down Expand Up @@ -123,7 +121,6 @@ wazero is a registered trademark of Tetrate.io, Inc. in the United States and/or
[5]: https://github.com/WebAssembly/WASI
[6]: https://pkg.go.dev/golang.org/x/sys/unix
[7]: https://github.com/WebAssembly/spec/tree/wg-1.0/test/core
[8]: internal/engine/compiler/RATIONALE.md
[9]: https://github.com/tetratelabs/wazero/issues/506
[10]: https://go.dev/doc/devel/release
[11]: https://github.com/actions/virtual-environments
Expand Down
4 changes: 1 addition & 3 deletions builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,7 @@ func TestNewHostModuleBuilder_Compile(t *testing.T) {
tc := tt

t.Run(tc.name, func(t *testing.T) {
cfg := NewRuntimeConfig()
cfg.(*runtimeConfig).EnableOptimizingCompiler()
b := tc.input(NewRuntimeWithConfig(testCtx, cfg)).(*hostModuleBuilder)
b := tc.input(NewRuntime(testCtx)).(*hostModuleBuilder)
compiled, err := b.Compile(testCtx)
require.NoError(t, err)
m := compiled.(*compiledModule)
Expand Down
7 changes: 0 additions & 7 deletions cmd/wazero/wazero.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/experimental"
"github.com/tetratelabs/wazero/experimental/logging"
"github.com/tetratelabs/wazero/experimental/opt"
"github.com/tetratelabs/wazero/experimental/sock"
"github.com/tetratelabs/wazero/experimental/sysfs"
"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
Expand Down Expand Up @@ -160,10 +159,6 @@ func doRun(args []string, stdOut io.Writer, stdErr logging.Writer) int {
flags.BoolVar(&useInterpreter, "interpreter", false,
"Interprets WebAssembly modules instead of compiling them into native code.")

var useOptimizingCompiler bool
flags.BoolVar(&useOptimizingCompiler, "optimizing-compiler", false,
"[Experimental] Compiles WebAssembly modules using the optimizing compiler.")

var envs sliceFlag
flags.Var(&envs, "env", "key=value pair of environment variable to expose to the binary. "+
"Can be specified multiple times.")
Expand Down Expand Up @@ -272,8 +267,6 @@ func doRun(args []string, stdOut io.Writer, stdErr logging.Writer) int {
var rtc wazero.RuntimeConfig
if useInterpreter {
rtc = wazero.NewRuntimeConfigInterpreter()
} else if useOptimizingCompiler {
rtc = opt.NewRuntimeConfigOptimizingCompiler()
} else {
rtc = wazero.NewRuntimeConfig()
}
Expand Down
10 changes: 2 additions & 8 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/tetratelabs/wazero/api"
experimentalsys "github.com/tetratelabs/wazero/experimental/sys"
"github.com/tetratelabs/wazero/internal/engine/compiler"
"github.com/tetratelabs/wazero/internal/engine/interpreter"
"github.com/tetratelabs/wazero/internal/engine/wazevo"
"github.com/tetratelabs/wazero/internal/filecache"
Expand Down Expand Up @@ -158,7 +157,7 @@ type RuntimeConfig interface {
// This is especially useful when one wants to run untrusted Wasm binaries since otherwise, any invocation of
// api.Function can potentially block the corresponding Goroutine forever. Moreover, it might block the
// entire underlying OS thread which runs the api.Function call. See "Why it's safe to execute runtime-generated
// machine codes against async Goroutine preemption" section in internal/engine/compiler/RATIONALE.md for detail.
// machine codes against async Goroutine preemption" section in RATIONALE.md for detail.
//
// Note that this comes with a bit of extra cost when enabled. The reason is that internally this forces
// interpreter and compiler runtimes to insert the periodical checks on the conditions above. For that reason,
Expand Down Expand Up @@ -191,11 +190,6 @@ type runtimeConfig struct {
ensureTermination bool
}

// EnableOptimizingCompiler implements experimental/opt/enabler.EnableOptimizingCompiler.
func (c *runtimeConfig) EnableOptimizingCompiler() {
c.newEngine = wazevo.NewEngine
}

// engineLessConfig helps avoid copy/pasting the wrong defaults.
var engineLessConfig = &runtimeConfig{
enabledFeatures: api.CoreFeaturesV2,
Expand Down Expand Up @@ -229,7 +223,7 @@ const (
func NewRuntimeConfigCompiler() RuntimeConfig {
ret := engineLessConfig.clone()
ret.engineKind = engineKindCompiler
ret.newEngine = compiler.NewEngine
ret.newEngine = wazevo.NewEngine
return ret
}

Expand Down
3 changes: 2 additions & 1 deletion experimental/checkpoint_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ type snapshotsKey struct{}
func Example_enableSnapshotterKey() {
ctx := context.Background()

rt := wazero.NewRuntime(ctx)
// TODO: currently, only the interpreter is supported for snapshotting.
rt := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfigInterpreter())
defer rt.Close(ctx) // This closes everything this Runtime created.

// Enable experimental snapshotting functionality by setting it to context. We use this
Expand Down
6 changes: 4 additions & 2 deletions experimental/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import (
func TestSnapshotNestedWasmInvocation(t *testing.T) {
ctx := context.Background()

rt := wazero.NewRuntime(ctx)
// TODO: currently, only the interpreter is supported for snapshotting.
rt := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfigInterpreter())
defer rt.Close(ctx)

sidechannel := 0
Expand Down Expand Up @@ -69,7 +70,8 @@ func TestSnapshotNestedWasmInvocation(t *testing.T) {
func TestSnapshotMultipleWasmInvocations(t *testing.T) {
ctx := context.Background()

rt := wazero.NewRuntime(ctx)
// TODO: currently, only the interpreter is supported for snapshotting.
rt := wazero.NewRuntimeWithConfig(ctx, wazero.NewRuntimeConfigInterpreter())
defer rt.Close(ctx)

_, err := rt.NewHostModuleBuilder("example").
Expand Down
16 changes: 12 additions & 4 deletions experimental/features_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"fmt"
"log"
"runtime"
"sync"
"sync/atomic"

wazero "github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/experimental"
"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
Expand All @@ -33,7 +34,9 @@ func ExampleCoreFeaturesThreads() {
ctx := context.Background()

// Threads support must be enabled explicitly in addition to standard V2 features.
cfg := wazero.NewRuntimeConfig().WithCoreFeatures(api.CoreFeaturesV2 | experimental.CoreFeaturesThreads)

// TODO: currently, only the interpreter is supported for snapshotting.
cfg := wazero.NewRuntimeConfigInterpreter().WithCoreFeatures(api.CoreFeaturesV2 | experimental.CoreFeaturesThreads)

r := wazero.NewRuntimeWithConfig(ctx, cfg)
defer r.Close(ctx)
Expand Down Expand Up @@ -102,12 +105,17 @@ type childModule struct {

var prevTID uint32

var childModuleMu sync.Mutex

// wasi sdk maintains a stack per thread within memory, so we must allocate one separately per child
// module, corresponding to a host thread, or the stack accesses would collide. wasi sdk does not
// currently plan to implement this so we must implement it ourselves. We allocate memory for a stack,
// initialize a pthread struct at the beginning of the stack, and set globals to reference it.
// https://github.com/WebAssembly/wasi-threads/issues/45
func createChildModule(rt wazero.Runtime, root api.Module, wasmCompiled wazero.CompiledModule) *childModule {
childModuleMu.Lock()
defer childModuleMu.Unlock()

ctx := context.Background()

// Not executing function so the current stack pointer is end of stack
Expand Down Expand Up @@ -145,8 +153,8 @@ func createChildModule(rt wazero.Runtime, root api.Module, wasmCompiled wazero.C
// This is relying on libc implementation details. The structure has been stable for a long time
// though it is possible it could change if compiling with a different version of wasi sdk.
tid := atomic.AddUint32(&prevTID, 1)
root.Memory().WriteUint32Le(ptr, ptr)
root.Memory().WriteUint32Le(ptr+20, tid)
child.Memory().WriteUint32Le(ptr, ptr)
child.Memory().WriteUint32Le(ptr+20, tid)
child.ExportedGlobal("__stack_pointer").(api.MutableGlobal).Set(uint64(ptr) + size)

ret := &childModule{
Expand Down
9 changes: 1 addition & 8 deletions experimental/listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import (
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/experimental"
"github.com/tetratelabs/wazero/experimental/opt"
"github.com/tetratelabs/wazero/experimental/wazerotest"
"github.com/tetratelabs/wazero/internal/platform"
"github.com/tetratelabs/wazero/internal/testing/binaryencoding"
"github.com/tetratelabs/wazero/internal/testing/require"
"github.com/tetratelabs/wazero/internal/wasm"
Expand Down Expand Up @@ -75,12 +73,7 @@ func TestFunctionListenerFactory(t *testing.T) {
},
})

var r wazero.Runtime
if platform.CompilerSupported() {
r = wazero.NewRuntimeWithConfig(ctx, opt.NewRuntimeConfigOptimizingCompiler())
} else {
r = wazero.NewRuntime(ctx)
}
r := wazero.NewRuntime(ctx)
defer r.Close(ctx) // This closes everything this Runtime created.

_, err := r.NewHostModuleBuilder("host").NewFunctionBuilder().WithFunc(func() {}).Export("").Instantiate(ctx)
Expand Down
Loading
Loading