Skip to content

Commit

Permalink
bugfix: Tighten up memory model.
Browse files Browse the repository at this point in the history
Fixes wasmerio#415

As documented in wasmerio#415,
the memory mode exposed by wasmer-go is error prone and hard to use.

Introduce a `CPtrBase[T]` helper struct to attempt to surface
errors often and early.  The `CPtrBase` is meant to store a CGo pointer.
Then, through conditional compilation, a debug implementation, enabled
via `-tags memcheck` tag, is used, which adds expensive pointer access
checks as well as forces calls to `runtime.GC()` whenever underlying
pointer accessed.  These access are very slow -- however, they cause
almost immediate crashes to surface in tests.

It's possible that some issues still remain;
The fixes were applied in order to ensure that the tests pass 5000
times.  Beyond that, there could still be remaining issues.

Release note (<category, see below>): <what> <show> <why>
  • Loading branch information
Yevgeniy Miretskiy committed Jan 14, 2025
1 parent f09913d commit 1f78846
Show file tree
Hide file tree
Showing 26 changed files with 704 additions and 408 deletions.
108 changes: 55 additions & 53 deletions wasmer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ const (

// Strings returns the CompilerKind as a string.
//
// CRANELIFT.String() // "cranelift"
// LLVM.String() // "llvm"
// CRANELIFT.String() // "cranelift"
// LLVM.String() // "llvm"
func (self CompilerKind) String() string {
switch self {
case CRANELIFT:
Expand All @@ -40,7 +40,7 @@ func (self CompilerKind) String() string {
// IsCompilerAvailable checks that the given compiler is available
// in this current version of `wasmer-go`.
//
// IsCompilerAvailable(CRANELIFT)
// IsCompilerAvailable(CRANELIFT)
func IsCompilerAvailable(compiler CompilerKind) bool {
return bool(C.wasmer_is_compiler_available(uint32(C.wasmer_compiler_t(compiler))))
}
Expand All @@ -64,8 +64,8 @@ const (

// Strings returns the EngineKind as a string.
//
// JIT.String() // "jit"
// NATIVE.String() // "native"
// JIT.String() // "jit"
// NATIVE.String() // "native"
func (self EngineKind) String() string {
switch self {
case UNIVERSAL:
Expand All @@ -80,35 +80,31 @@ func (self EngineKind) String() string {
// IsEngineAvailable checks that the given engine is available in this
// current version of `wasmer-go`.
//
// IsEngineAvailable(UNIVERSAL)
// IsEngineAvailable(UNIVERSAL)
func IsEngineAvailable(engine EngineKind) bool {
return bool(C.wasmer_is_engine_available(uint32(C.wasmer_engine_t(engine))))
}

// Config holds the compiler and the Engine used by the Store.
type Config struct {
_inner *C.wasm_config_t
CPtrBase[*C.wasm_config_t]
}

// NewConfig instantiates and returns a new Config.
//
// config := NewConfig()
// config := NewConfig()
func NewConfig() *Config {
config := C.wasm_config_new()

return &Config{
_inner: config,
}
return &Config{CPtrBase: mkPtr(C.wasm_config_new())}
}

func (self *Config) inner() *C.wasm_config_t {
return self._inner
return self.ptr()
}

// UseNativeEngine sets the engine to Universal in the configuration.
//
// config := NewConfig()
// config.UseUniversalEngine()
// config := NewConfig()
// config.UseUniversalEngine()
//
// This method might fail if the Universal engine isn't
// available. Check `IsEngineAvailable` to learn more.
Expand All @@ -124,8 +120,8 @@ func (self *Config) UseUniversalEngine() *Config {

// UseDylibEngine sets the engine to Dylib in the configuration.
//
// config := NewConfig()
// config.UseDylibEngine()
// config := NewConfig()
// config.UseDylibEngine()
//
// This method might fail if the Dylib engine isn't available. Check
// `IsEngineAvailable` to learn more.
Expand Down Expand Up @@ -153,8 +149,8 @@ func (self *Config) UseNativeEngine() *Config {

// UseCraneliftCompiler sets the compiler to Cranelift in the configuration.
//
// config := NewConfig()
// config.UseCraneliftCompiler()
// config := NewConfig()
// config.UseCraneliftCompiler()
//
// This method might fail if the Cranelift compiler isn't
// available. Check `IsCompilerAvailable` to learn more.
Expand Down Expand Up @@ -182,14 +178,17 @@ func metering_delegate(op C.wasmer_parser_operator_t) C.uint64_t {
}

// PushMeteringMiddleware allows the middleware metering to be engaged on a map of opcode to cost
// config := NewConfig()
// opmap := map[uint32]uint32{
// End: 1,
// LocalGet: 1,
// I32Add: 4,
// }
// config.PushMeteringMiddleware(7865444, opmap)
func (self *Config) PushMeteringMiddleware(maxGasUsageAllowed uint64, opMap map[Opcode]uint32) *Config {
//
// config := NewConfig()
// opmap := map[uint32]uint32{
// End: 1,
// LocalGet: 1,
// I32Add: 4,
// }
// config.PushMeteringMiddleware(7865444, opmap)
func (self *Config) PushMeteringMiddleware(
maxGasUsageAllowed uint64, opMap map[Opcode]uint32,
) *Config {
if opCodeMap == nil {
// REVIEW only allowing this to be set once
opCodeMap = opMap
Expand All @@ -200,40 +199,43 @@ func (self *Config) PushMeteringMiddleware(maxGasUsageAllowed uint64, opMap map[

// PushMeteringMiddlewarePtr allows the middleware metering to be engaged on an unsafe.Pointer
// this pointer must be a to C based function with a signature of:
// extern uint64_t cost_delegate_func(enum wasmer_parser_operator_t op);
//
// extern uint64_t cost_delegate_func(enum wasmer_parser_operator_t op);
//
// package main
//
// #include <wasmer.h>
// extern uint64_t metering_delegate_alt(enum wasmer_parser_operator_t op);
// import "C"
// import "unsafe"
//
// func getInternalCPointer() unsafe.Pointer {
// return unsafe.Pointer(C.metering_delegate_alt)
// }
// func getInternalCPointer() unsafe.Pointer {
// return unsafe.Pointer(C.metering_delegate_alt)
// }
//
// //export metering_delegate_alt
// func metering_delegate_alt(op C.wasmer_parser_operator_t) C.uint64_t {
// v, b := opCodeMap[Opcode(op)]
// if !b {
// return 0 // no value means no cost
// }
// return C.uint64_t(v)
// }
//
// void main(){
// config := NewConfig()
// config.PushMeteringMiddlewarePtr(800000000, getInternalCPointer())
// }
// func metering_delegate_alt(op C.wasmer_parser_operator_t) C.uint64_t {
// v, b := opCodeMap[Opcode(op)]
// if !b {
// return 0 // no value means no cost
// }
// return C.uint64_t(v)
// }
//
// void main(){
// config := NewConfig()
// config.PushMeteringMiddlewarePtr(800000000, getInternalCPointer())
// }
func (self *Config) PushMeteringMiddlewarePtr(maxGasUsageAllowed uint64, p unsafe.Pointer) *Config {
C.wasm_config_push_middleware(self.inner(), C.wasmer_metering_as_middleware(C.wasmer_metering_new(getPlatformLong(maxGasUsageAllowed), (*[0]byte)(p))))
return self
}

// UseLLVMCompiler sets the compiler to LLVM in the configuration.
//
// config := NewConfig()
// config.UseLLVMCompiler()
// config := NewConfig()
// config.UseLLVMCompiler()
//
// This method might fail if the LLVM compiler isn't available. Check
// `IsCompilerAvailable` to learn more.
Expand All @@ -250,8 +252,8 @@ func (self *Config) UseLLVMCompiler() *Config {
// UseSinglepassCompiler sets the compiler to Singlepass in the
// configuration.
//
// config := NewConfig()
// config.UseSinglepassCompiler()
// config := NewConfig()
// config.UseSinglepassCompiler()
//
// This method might fail if the Singlepass compiler isn't
// available. Check `IsCompilerAvailable` to learn more.
Expand All @@ -267,14 +269,14 @@ func (self *Config) UseSinglepassCompiler() *Config {

// Use a specific target for doing cross-compilation.
//
// triple, _ := NewTriple("aarch64-unknown-linux-gnu")
// cpuFeatures := NewCpuFeatures()
// target := NewTarget(triple, cpuFeatures)
// triple, _ := NewTriple("aarch64-unknown-linux-gnu")
// cpuFeatures := NewCpuFeatures()
// target := NewTarget(triple, cpuFeatures)
//
// config := NewConfig()
// config.UseTarget(target)
// config := NewConfig()
// config.UseTarget(target)
func (self *Config) UseTarget(target *Target) *Config {
C.wasm_config_set_target(self.inner(), target.inner())
C.wasm_config_set_target(self.inner(), target.release())

return self
}
13 changes: 8 additions & 5 deletions wasmer/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ func TestConfig(t *testing.T) {
instance, err := NewInstance(module, NewImportObject())
assert.NoError(t, err)

sum, err := instance.Exports.GetFunction("sum")
sum, release, err := instance.GetFunctionSafe("sum")
assert.NoError(t, err)
defer release(instance)

result, err := sum(37, 5)
assert.NoError(t, err)
Expand All @@ -57,8 +58,9 @@ func TestConfigForMetering(t *testing.T) {
instance, err := NewInstance(module, NewImportObject())
assert.NoError(t, err)

sum, err := instance.Exports.GetFunction("sum")
sum, release, err := instance.GetFunctionSafe("sum")
assert.NoError(t, err)
defer release(instance)

result, err := sum(37, 5)
assert.NoError(t, err)
Expand All @@ -69,7 +71,6 @@ func TestConfigForMetering(t *testing.T) {
}

func TestConfigForMeteringFn(t *testing.T) {

config := NewConfig().PushMeteringMiddlewarePtr(800000000, getInternalCPointer())
engine := NewEngineWithConfig(config)
store := NewStore(engine)
Expand All @@ -79,8 +80,9 @@ func TestConfigForMeteringFn(t *testing.T) {
instance, err := NewInstance(module, NewImportObject())
assert.NoError(t, err)

sum, err := instance.Exports.GetFunction("sum")
sum, release, err := instance.GetFunctionSafe("sum")
assert.NoError(t, err)
defer release(instance)

result, err := sum(37, 5)
assert.NoError(t, err)
Expand Down Expand Up @@ -149,8 +151,9 @@ func TestConfig_AllCombinations(t *testing.T) {
instance, err := NewInstance(module, NewImportObject())
assert.NoError(t, err)

sum, err := instance.Exports.GetFunction("sum")
sum, release, err := instance.GetFunctionSafe("sum")
assert.NoError(t, err)
defer release(instance)

result, err := sum(37, 5)
assert.NoError(t, err)
Expand Down
33 changes: 33 additions & 0 deletions wasmer/cptr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package wasmer

// CPtrBase is a based struct for a C pointer.
// It is intended to be embedded into any structure
// that stores a C pointer.
// While this based is parameterized on type any, using any
// type other than *C.xxx pointer should be considered an undefined behavior.
type CPtrBase[T comparable] struct {
_ptr T
maybeStack // stack of the creating goroutine (if enabled via memcheck)
maybeNil[T] // indicates if initial value of _ptr was nil (if enabled via memcheck)
}

// release returns the C pointer stored in this base and clears the finalizer.
func (b *CPtrBase[T]) release() T {
var zero T
v := b.ptr()
b._ptr = zero
b.ClearFinalizer()
return v
}

func (b *CPtrBase[T]) SetFinalizer(free func(v T)) {
doSetFinalizer(b, free)
}

func (b *CPtrBase[T]) ClearFinalizer() {
doClearFinalizer(b)
}

func mkPtr[T comparable](ptr T) CPtrBase[T] {
return doMkPtr(ptr)
}
Loading

0 comments on commit 1f78846

Please sign in to comment.