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.

Sign up for GitHub

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

Jump to bottom

compiler: zero-initializes moduleInstanceAddress of call engine #783

Merged
merged 3 commits into from Aug 31, 2022
Merged

compiler: zero-initializes moduleInstanceAddress of call engine #783

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98703be
fix
mathetake Aug 31, 2022
28527d1
fix
mathetake Aug 31, 2022
8804cdc
adhoc
mathetake Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 18 additions & 13 deletions internal/engine/compiler/engine.go
View file
Open in desktop
Expand Up @@ -101,6 +101,9 @@ type (
moduleContext struct {
// moduleInstanceAddress is the address of module instance from which we initialize
// the following fields. This is set whenever we enter a function or return from function calls.
//
// On the entry to the native code, this must be initialized to zero to let native code preamble know
// that this is the initial function call (which leads to moduleContext initialization pass).
moduleInstanceAddress uintptr //lint:ignore U1000 This is only used by Compiler code.

// globalElement0Address is the address of the first element in the global slice,
Expand Down Expand Up @@ -560,9 +563,7 @@ func (ce *callEngine) Call(ctx context.Context, callCtx *wasm.CallContext, param
}
// TODO: ^^ Will not fail if the function was imported from a closed module.

if v := recover(); v != nil {
err = ce.recoverOnCall(v)
}
err = ce.deferredOnCall(recover())
}()

for _, v := range params {
Expand All @@ -573,19 +574,23 @@ func (ce *callEngine) Call(ctx context.Context, callCtx *wasm.CallContext, param
return
}

// recoverOnCall takes the recovered value `recoverOnCall`, and wraps it
// with the call frame stack traces. Also, reset the state of callEngine
// so that it can be used for the subsequent calls.
func (ce *callEngine) recoverOnCall(v interface{}) (err error) {
builder := wasmdebug.NewErrorBuilder()
for i := uint64(0); i < ce.callFrameStackPointer; i++ {
def := ce.callFrameStack[ce.callFrameStackPointer-1-i].function.source.FunctionDefinition
builder.AddFrame(def.DebugName(), def.ParamTypes(), def.ResultTypes())
// deferredOnCall takes the recovered value `recovered`, and wraps it
// with the call frame stack traces if v != nil. Also, reset
mathetake marked this conversation as resolved.
Show resolved Hide resolved
// the state of callEngine so that it can be used for the subsequent calls.
//
// This is defined for testability.
func (ce *callEngine) deferredOnCall(recovered interface{}) (err error) {
if recovered != nil {
builder := wasmdebug.NewErrorBuilder()
for i := uint64(0); i < ce.callFrameStackPointer; i++ {
def := ce.callFrameStack[ce.callFrameStackPointer-1-i].function.source.FunctionDefinition
builder.AddFrame(def.DebugName(), def.ParamTypes(), def.ResultTypes())
}
err = builder.FromRecovered(recovered)
}
err = builder.FromRecovered(v)

// Allows the reuse of CallEngine.
ce.stackPointer, ce.callFrameStackPointer = 0, 0
ce.stackBasePointerInBytes, ce.stackPointer, ce.callFrameStackPointer, ce.moduleInstanceAddress = 0, 0, 0, 0
mathetake marked this conversation as resolved.
Show resolved Hide resolved
return
}

Expand Down
6 changes: 4 additions & 2 deletions internal/engine/compiler/engine_test.go
View file
Open in desktop
Expand Up @@ -305,7 +305,7 @@ func TestCallEngine_builtinFunctionTableGrow(t *testing.T) {
require.Equal(t, uintptr(0xff), table.References[0])
}

func TestCallEngine_recoverOnCall(t *testing.T) {
func TestCallEngine_deferredOnCall(t *testing.T) {
ce := &callEngine{
valueStack: make([]uint64, 100),
valueStackContext: valueStackContext{stackPointer: 3},
Expand All @@ -317,11 +317,12 @@ func TestCallEngine_recoverOnCall(t *testing.T) {
{function: &function{source: &wasm.FunctionInstance{FunctionDefinition: newMockFunctionDefinition("4")}}},
{function: &function{source: &wasm.FunctionInstance{FunctionDefinition: newMockFunctionDefinition("5")}}},
},
moduleContext: moduleContext{moduleInstanceAddress: 0xdeafbeaf},
}

beforeRecoverValueStack, beforeRecoverCallFrameStack := ce.valueStack, ce.callFrameStack

err := ce.recoverOnCall(errors.New("some error"))
err := ce.deferredOnCall(errors.New("some error"))
require.EqualError(t, err, `some error (recovered by wazero)
wasm stack trace:
5()
Expand All @@ -334,6 +335,7 @@ wasm stack trace:
// for the subsequent calls.
require.Equal(t, uint64(0), ce.stackBasePointerInBytes)
require.Equal(t, uint64(0), ce.callFrameStackPointer)
require.Equal(t, uintptr(0), ce.moduleInstanceAddress)
require.Equal(t, beforeRecoverValueStack, ce.valueStack)
require.Equal(t, beforeRecoverCallFrameStack, ce.callFrameStack)
}
Expand Down