bugfix: Tighten up memory model.

Fixes #415

As documented in #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.

Author: Yevgeniy Miretskiy

wasmer/config.go

@@ -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:
@@ -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))))
 }
@@ -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:
@@ -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.
@@ -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.
@@ -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.
@@ -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
@@ -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.
@@ -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.
@@ -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
 }

wasmer/config_test.go

@@ -29,13 +29,16 @@ func TestConfig(t *testing.T) {
 	engine := NewEngineWithConfig(config)
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
+	defer runtime.KeepAlive(module)
+
 	assert.NoError(t, err)
 
 	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)
@@ -53,12 +56,14 @@ func TestConfigForMetering(t *testing.T) {
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
 	assert.NoError(t, err)
+	defer runtime.KeepAlive(module)
 
 	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)
@@ -69,18 +74,19 @@ func TestConfigForMetering(t *testing.T) {
 }
 
 func TestConfigForMeteringFn(t *testing.T) {
-
 	config := NewConfig().PushMeteringMiddlewarePtr(800000000, getInternalCPointer())
 	engine := NewEngineWithConfig(config)
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
 	assert.NoError(t, err)
+	defer runtime.KeepAlive(module)
 
 	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)
@@ -145,12 +151,14 @@ func TestConfig_AllCombinations(t *testing.T) {
 				store := NewStore(engine)
 				module, err := NewModule(store, testGetBytes("tests.wasm"))
 				assert.NoError(t, err)
+				defer runtime.KeepAlive(module)
 
 				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)

wasmer/cptr.go

@@ -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)
+}