diff --git a/experimental/listener.go b/experimental/listener.go index c1193b59..ecf89bd5 100644 --- a/experimental/listener.go +++ b/experimental/listener.go @@ -55,14 +55,14 @@ type FunctionListener interface { // instance or parent of the result. // - mod: the calling module. // - def: the function definition. - // - paramValues: api.ValueType encoded parameters. + // - params: api.ValueType encoded parameters. // - stackIterator: iterator on the call stack. At least one entry is // guaranteed (the called function), whose Args() will be equal to - // paramValues. The iterator will be reused between calls to Before. + // params. The iterator will be reused between calls to Before. // // Note: api.Memory is meant for inspection, not modification. // mod can be cast to InternalModule to read non-exported globals. - Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator StackIterator) context.Context + Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator StackIterator) context.Context // After is invoked after a function is called. // @@ -71,11 +71,10 @@ type FunctionListener interface { // - ctx: the context returned by Before. // - mod: the calling module. // - def: the function definition. - // - err: nil if the function didn't err - // - resultValues: api.ValueType encoded results. + // - results: api.ValueType encoded results. // // Note: api.Memory is meant for inspection, not modification. - After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) + After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) } // FunctionListenerFunc is a function type implementing the FunctionListener @@ -89,14 +88,14 @@ type FunctionListener interface { type FunctionListenerFunc func(context.Context, api.Module, api.FunctionDefinition, []uint64, StackIterator) // Before satisfies the FunctionListener interface, calls f. -func (f FunctionListenerFunc) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator StackIterator) context.Context { - f(ctx, mod, def, paramValues, stackIterator) +func (f FunctionListenerFunc) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator StackIterator) context.Context { + f(ctx, mod, def, params, stackIterator) return ctx } // After is declared to satisfy the FunctionListener interface, but it does // nothing. -func (f FunctionListenerFunc) After(context.Context, api.Module, api.FunctionDefinition, error, []uint64) { +func (f FunctionListenerFunc) After(context.Context, api.Module, api.FunctionDefinition, []uint64) { } // FunctionListenerFactoryFunc is a function type implementing the @@ -159,9 +158,9 @@ func (multi *multiFunctionListener) Before(ctx context.Context, mod api.Module, return ctx } -func (multi *multiFunctionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, results []uint64) { +func (multi *multiFunctionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) { for _, lstn := range multi.lstns { - lstn.After(ctx, mod, def, err, results) + lstn.After(ctx, mod, def, results) } } @@ -312,16 +311,16 @@ func BenchmarkFunctionListener(n int, module api.Module, stack []StackFrame, lis panic("cannot benchmark function listener with an empty stack") } - functionDefinition := stack[0].Function.Definition() - functionParams := stack[0].Params - functionResults := stack[0].Results - stackIterator := &stackIterator{base: NewStackIterator(stack...)} ctx := context.Background() + def := stack[0].Function.Definition() + params := stack[0].Params + results := stack[0].Results + stackIterator := &stackIterator{base: NewStackIterator(stack...)} for i := 0; i < n; i++ { stackIterator.index = -1 - callContext := listener.Before(ctx, module, functionDefinition, functionParams, stackIterator) - listener.After(callContext, module, functionDefinition, nil, functionResults) + callContext := listener.Before(ctx, module, def, params, stackIterator) + listener.After(callContext, module, def, results) } } @@ -329,8 +328,6 @@ func BenchmarkFunctionListener(n int, module api.Module, stack []StackFrame, lis // indirect in terms of depth and breadth. The test could show a tree 3 calls deep where the there are a couple calls at // each depth under the root. The main thing this can help prevent is accidentally swapping the context internally. -// TODO: Errors aren't handled, and the After hook should accept one along with the result values. - // TODO: The context parameter of the After hook is not the same as the Before hook. This means interceptor patterns // are awkward. e.g. something like timing is difficult as it requires propagating a stack. Otherwise, nested calls will // overwrite each other's "since" time. Propagating a stack is further awkward as the After hook needs to know the diff --git a/experimental/listener_example_test.go b/experimental/listener_example_test.go index 38f528e7..6fc48844 100644 --- a/experimental/listener_example_test.go +++ b/experimental/listener_example_test.go @@ -50,7 +50,7 @@ func (u uniqGoFuncs) Before(ctx context.Context, _ api.Module, def api.FunctionD } // After implements FunctionListener.After -func (u uniqGoFuncs) After(context.Context, api.Module, api.FunctionDefinition, error, []uint64) {} +func (u uniqGoFuncs) After(context.Context, api.Module, api.FunctionDefinition, []uint64) {} // This shows how to make a listener that counts go function calls. func Example_customListenerFactory() { diff --git a/experimental/listener_test.go b/experimental/listener_test.go index 8555f6f1..55a8aa5c 100644 --- a/experimental/listener_test.go +++ b/experimental/listener_test.go @@ -27,7 +27,7 @@ func (r *recorder) Before(ctx context.Context, _ api.Module, def api.FunctionDef return ctx } -func (r *recorder) After(_ context.Context, _ api.Module, def api.FunctionDefinition, _ error, _ []uint64) { +func (r *recorder) After(_ context.Context, _ api.Module, def api.FunctionDefinition, _ []uint64) { r.afterNames = append(r.afterNames, def.DebugName()) } @@ -116,7 +116,7 @@ func TestMultiFunctionListenerFactory(t *testing.T) { } n := 0 - f := func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) { + f := func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) { n++ i := 0 for stackIterator.Next() { @@ -180,12 +180,12 @@ func BenchmarkMultiFunctionListener(b *testing.B) { }{ { scenario: "simple function listener", - function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) { + function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) { }, }, { scenario: "stack iterator", - function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) { + function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) { for stackIterator.Next() { } }, diff --git a/experimental/logging/log_listener.go b/experimental/logging/log_listener.go index 51452f86..7e2aecc7 100644 --- a/experimental/logging/log_listener.go +++ b/experimental/logging/log_listener.go @@ -203,7 +203,9 @@ func (l *loggingListener) Before(ctx context.Context, mod api.Module, _ api.Func // We're starting to log: increase the indentation level. nestLevel++ - l.logIndented(ctx, mod, nestLevel, true, params, nil, nil) + l.logIndented(nestLevel, l.beforePrefix, func() { + l.logParams(ctx, mod, params) + }) // We need to propagate this invocation's parameters to the after callback. state = &logState{w: l.w, nestLevel: nestLevel} @@ -220,34 +222,27 @@ func (l *loggingListener) Before(ctx context.Context, mod api.Module, _ api.Func // After logs to stdout the module and function name, prefixed with '<--' and // indented based on the call nesting level. -func (l *loggingListener) After(ctx context.Context, mod api.Module, _ api.FunctionDefinition, err error, results []uint64) { +func (l *loggingListener) After(ctx context.Context, mod api.Module, _ api.FunctionDefinition, results []uint64) { // Note: We use the nest level directly even though it is the "next" nesting level. // This works because our indent of zero nesting is one tab. if state, ok := ctx.Value(logging.LoggerKey{}).(*logState); ok { if state == unsampledLogState { return } - l.logIndented(ctx, mod, state.nestLevel, false, state.params, err, results) + l.logIndented(state.nestLevel, l.afterPrefix, func() { + l.logResults(ctx, mod, state.params, results) + }) } } -// logIndented logs an indented l.w like this: "-->\t\t\t$nestLevel$funcName\n" -func (l *loggingListener) logIndented(ctx context.Context, mod api.Module, nestLevel int, isBefore bool, params []uint64, err error, results []uint64) { +// logIndented writes an indentation level and prefix prior to calling log to +// output the log line. +func (l *loggingListener) logIndented(nestLevel int, prefix string, log func()) { for i := 1; i < nestLevel; i++ { l.w.WriteByte('\t') //nolint } - if isBefore { // before - l.w.WriteString(l.beforePrefix) //nolint - l.logParams(ctx, mod, params) - } else { // after - l.w.WriteString(l.afterPrefix) //nolint - if err != nil { - l.w.WriteString(" error: ") //nolint - l.w.WriteString(err.Error()) //nolint - } else { - l.logResults(ctx, mod, params, results) - } - } + l.w.WriteString(prefix) //nolint + log() l.w.WriteByte('\n') //nolint if f, ok := l.w.(flusher); ok { diff --git a/experimental/logging/log_listener_test.go b/experimental/logging/log_listener_test.go index 8417f56f..81580bef 100644 --- a/experimental/logging/log_listener_test.go +++ b/experimental/logging/log_listener_test.go @@ -3,7 +3,6 @@ package logging_test import ( "bytes" "context" - "io" "math" "testing" @@ -36,7 +35,6 @@ func Test_loggingListener(t *testing.T) { isHostFunc bool paramNames, resultNames []string params, results []uint64 - err error expected string }{ { @@ -44,14 +42,6 @@ func Test_loggingListener(t *testing.T) { functype: wasm.FunctionType{}, expected: `--> test.fn() <-- -`, - }, - { - name: "error", - functype: wasm.FunctionType{}, - err: io.EOF, - expected: `--> test.fn() -<-- error: EOF `, }, { @@ -88,20 +78,6 @@ func Test_loggingListener(t *testing.T) { results: []uint64{uint64(wasi.ErrnoFault)}, expected: `==> wasi_snapshot_preview1.random_get(buf=0,buf_len=8) <== errno=EFAULT -`, - }, - { - name: "wasi error", - functype: wasiFuncType, - moduleName: wasi.InternalModuleName, - funcName: wasiFuncName, - paramNames: wasiParamNames, - resultNames: wasiResultNames, - isHostFunc: true, - params: wasiParams, - err: io.EOF, // not possible as we coerce errors to numbers, but test anyway! - expected: `==> wasi_snapshot_preview1.random_get(buf=0,buf_len=8) -<== error: EOF `, }, { @@ -304,7 +280,7 @@ func Test_loggingListener(t *testing.T) { out.Reset() ctx := l.Before(testCtx, nil, def, tc.params, nil) - l.After(ctx, nil, def, tc.err, tc.results) + l.After(ctx, nil, def, tc.results) require.Equal(t, tc.expected, out.String()) }) } @@ -341,8 +317,8 @@ func Test_loggingListener_indentation(t *testing.T) { ctx := l1.Before(testCtx, nil, def1, []uint64{}, nil) ctx1 := l2.Before(ctx, nil, def2, []uint64{}, nil) - l2.After(ctx1, nil, def2, nil, []uint64{}) - l1.After(ctx, nil, def1, nil, []uint64{}) + l2.After(ctx1, nil, def2, []uint64{}) + l1.After(ctx, nil, def1, []uint64{}) require.Equal(t, `--> test.fn1() --> test.fn2() <-- diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index c848c2c0..a6ec03f9 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -1183,7 +1183,7 @@ func (ce *callEngine) builtinFunctionFunctionListenerBefore(ctx context.Context, func (ce *callEngine) builtinFunctionFunctionListenerAfter(ctx context.Context, mod api.Module, fn *function) { base := int(ce.stackBasePointerInBytes >> 3) - fn.parent.listener.After(ctx, mod, fn.definition(), nil, ce.stack[base:base+fn.funcType.ResultNumInUint64]) + fn.parent.listener.After(ctx, mod, fn.definition(), ce.stack[base:base+fn.funcType.ResultNumInUint64]) i := len(ce.contextStack) - 1 ce.ctx = ce.contextStack[i] diff --git a/internal/engine/compiler/engine_bench_test.go b/internal/engine/compiler/engine_bench_test.go index 23ca605a..c3b6fc06 100644 --- a/internal/engine/compiler/engine_bench_test.go +++ b/internal/engine/compiler/engine_bench_test.go @@ -17,7 +17,7 @@ func BenchmarkCallEngine_builtinFunctionFunctionListener(b *testing.B) { before: func(context.Context, api.Module, api.FunctionDefinition, []uint64, experimental.StackIterator) context.Context { return context.Background() }, - after: func(context.Context, api.Module, api.FunctionDefinition, error, []uint64) { + after: func(context.Context, api.Module, api.FunctionDefinition, []uint64) { }, }, index: 0, diff --git a/internal/engine/compiler/engine_test.go b/internal/engine/compiler/engine_test.go index cb015753..3b081e74 100644 --- a/internal/engine/compiler/engine_test.go +++ b/internal/engine/compiler/engine_test.go @@ -589,9 +589,9 @@ func TestCallEngine_builtinFunctionFunctionListenerBefore(t *testing.T) { funcType: &wasm.FunctionType{ParamNumInUint64: 3}, parent: &compiledFunction{ listener: mockListener{ - before: func(ctx context.Context, _ api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context { + before: func(ctx context.Context, _ api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context { require.Equal(t, currentContext, ctx) - require.Equal(t, []uint64{2, 3, 4}, paramValues) + require.Equal(t, []uint64{2, 3, 4}, params) assertStackIterator(t, stackIterator, []stackEntry{{def: def, args: []uint64{2, 3, 4}}}) return nextContext }, @@ -620,9 +620,9 @@ func TestCallEngine_builtinFunctionFunctionListenerAfter(t *testing.T) { funcType: &wasm.FunctionType{ResultNumInUint64: 1}, parent: &compiledFunction{ listener: mockListener{ - after: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) { + after: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) { require.Equal(t, currentContext, ctx) - require.Equal(t, []uint64{5}, resultValues) + require.Equal(t, []uint64{5}, results) }, }, index: 0, @@ -645,16 +645,16 @@ func TestCallEngine_builtinFunctionFunctionListenerAfter(t *testing.T) { } type mockListener struct { - before func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context - after func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) + before func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context + after func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) } -func (m mockListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context { - return m.before(ctx, mod, def, paramValues, stackIterator) +func (m mockListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context { + return m.before(ctx, mod, def, params, stackIterator) } -func (m mockListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) { - m.after(ctx, mod, def, err, resultValues) +func (m mockListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) { + m.after(ctx, mod, def, results) } func TestFunction_getSourceOffsetInWasmBinary(t *testing.T) { diff --git a/internal/engine/interpreter/interpreter.go b/internal/engine/interpreter/interpreter.go index fd2233c2..c25bcc9a 100644 --- a/internal/engine/interpreter/interpreter.go +++ b/internal/engine/interpreter/interpreter.go @@ -608,7 +608,7 @@ func (ce *callEngine) callGoFunc(ctx context.Context, m *wasm.ModuleInstance, f if lsn != nil { // TODO: This doesn't get the error due to use of panic to propagate them. results := stack[:typ.ResultNumInUint64] - lsn.After(ctx, m, f.definition(), nil, results) + lsn.After(ctx, m, f.definition(), results) } } @@ -4087,7 +4087,7 @@ func (ce *callEngine) callNativeFuncWithListener(ctx context.Context, m *wasm.Mo ctx = fnl.Before(ctx, m, def, ce.peekValues(len(typ.Params)), &ce.stackIterator) ce.stackIterator.clear() ce.callNativeFunc(ctx, m, f) - fnl.After(ctx, m, def, nil, ce.peekValues(len(typ.Results))) + fnl.After(ctx, m, def, ce.peekValues(len(typ.Results))) return ctx } diff --git a/internal/testing/enginetest/enginetest.go b/internal/testing/enginetest/enginetest.go index 1a103a42..12e39860 100644 --- a/internal/testing/enginetest/enginetest.go +++ b/internal/testing/enginetest/enginetest.go @@ -556,7 +556,7 @@ func RunTestModuleEngineBeforeListenerStackIterator(t *testing.T, et EngineTeste } fnListener := &fnListener{ - beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context { + beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context { require.True(t, len(expectedCallstacks) > 0) expectedCallstack := expectedCallstacks[0] for si.Next() { @@ -693,7 +693,7 @@ func RunTestModuleEngineBeforeListenerGlobals(t *testing.T, et EngineTester) { } fnListener := &fnListener{ - beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context { + beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context { require.True(t, len(expectedGlobals) > 0) imod := mod.(experimental.InternalModule) @@ -803,24 +803,24 @@ func RunTestModuleEngineBeforeListenerGlobals(t *testing.T, et EngineTester) { } type fnListener struct { - beforeFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context - afterFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) + beforeFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context + afterFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) } func (f *fnListener) NewFunctionListener(api.FunctionDefinition) experimental.FunctionListener { return f } -func (f *fnListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context { +func (f *fnListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context { if f.beforeFn != nil { - return f.beforeFn(ctx, mod, def, paramValues, stackIterator) + return f.beforeFn(ctx, mod, def, params, stackIterator) } return ctx } -func (f *fnListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) { +func (f *fnListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) { if f.afterFn != nil { - f.afterFn(ctx, mod, def, err, resultValues) + f.afterFn(ctx, mod, def, results) } } @@ -835,7 +835,7 @@ func RunTestModuleEngineStackIteratorOffset(t *testing.T, et EngineTester) { var tape [][]frame fnListener := &fnListener{ - beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context { + beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context { var stack []frame for si.Next() { fn := si.Function()