From e6296355d5d4c54c2827becc2506d130e4a70077 Mon Sep 17 00:00:00 2001 From: xhd2015 Date: Wed, 29 May 2024 19:37:41 +0800 Subject: [PATCH] make --trap-stdlib default with running xgo test, skip unexported stdlib functions --- .github/workflows/go-compatible.yml | 29 ++++++++++ .github/workflows/go-windows.yml | 2 +- .github/workflows/go.yml | 2 +- README.md | 2 +- README_zh_cn.md | 2 +- cmd/xgo/option.go | 14 ++++- cmd/xgo/runtime_gen/core/version.go | 4 +- cmd/xgo/version.go | 4 +- patch/ctxt/stdlib.go | 33 ++++++++--- runtime/core/version.go | 4 +- runtime/mock/stdlib.md | 33 +++++++++-- .../test/trap/trap_avoid_recursive_test.go | 12 ++++ .../test/trap/trap_nested_interceptor_test.go | 15 +++++ runtime/test/trap/trap_return_err_test.go | 6 ++ runtime/test/trap_args/err_test.go | 3 + runtime/test/trap_args/trap_args_test.go | 6 ++ script/run-test/main.go | 58 ++++++++++++++----- 17 files changed, 191 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/go-compatible.yml diff --git a/.github/workflows/go-compatible.yml b/.github/workflows/go-compatible.yml new file mode 100644 index 00000000..c7a9063c --- /dev/null +++ b/.github/workflows/go-compatible.yml @@ -0,0 +1,29 @@ +# This workflow will build a golang project +# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go + +name: Go Compatible + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + +jobs: + + tests-with-older-go-versions: + strategy: + matrix: + os: [ ubuntu-latest] + go: [ '1.18' ,'1.20','1.21'] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v3 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: '${{ matrix.go }}' + + - name: Test + run: go run ./script/run-test --reset-instrument --debug -v \ No newline at end of file diff --git a/.github/workflows/go-windows.yml b/.github/workflows/go-windows.yml index 0cad36fb..7e601b21 100644 --- a/.github/workflows/go-windows.yml +++ b/.github/workflows/go-windows.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: os: [ windows-latest ] - go: [ '1.20' ] + go: [ '1.22' ] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 4efa1cfa..01e22e9f 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: os: [ ubuntu-latest] - go: [ '1.20' ] + go: [ '1.22' ] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 diff --git a/README.md b/README.md index 19077007..9a010dac 100644 --- a/README.md +++ b/README.md @@ -210,7 +210,7 @@ func TestPatchFunc(t *testing.T) { NOTE: `Patch` and `Mock`(below) supports top-level variables and consts, see [runtime/mock/MOCK_VAR_CONST.md](runtime/mock/MOCK_VAR_CONST.md). -**Notice for mocking stdlib**: due to performance and security impact, only a few packages and functions of stdlib can be mocked, the list can be found at [runtime/mock/stdlib.md](./runtime/mock/stdlib.md). If you want to mock additional stdlib functions, please file a discussion in [Issue#6](https://github.com/xhd2015/xgo/issues/6). +**Notice for mocking stdlib**: There are different modes for mocking stdlib functions,see [runtime/mock/stdlib.md](./runtime/mock/stdlib.md). ## Mock `runtime/mock` also provides another API called `Mock`, which is similar to `Patch`. diff --git a/README_zh_cn.md b/README_zh_cn.md index a8b66366..bfcb3d31 100644 --- a/README_zh_cn.md +++ b/README_zh_cn.md @@ -209,7 +209,7 @@ func TestPatchFunc(t *testing.T) { 注意: `Mock`和`Patch`也支持对包级别的变量和常量进行mock, 见[runtime/mock/MOCK_VAR_CONST.md](runtime/mock/MOCK_VAR_CONST.md). -**关于标准库Mock的注意事项**: 出于性能和安全考虑, 标准库中只有一部分包和函数能被Mock, 这个List可以在[runtime/mock/stdlib.md](./runtime/mock/stdlib.md)找到. 如果你需要Mock的标准库函数不在列表中, 可以在[Issue#6](https://github.com/xhd2015/xgo/issues/6)中进行评论。 +**关于标准库Mock的注意事项**: 针对标准库有两种不同的模式，参考[runtime/mock/stdlib.md](./runtime/mock/stdlib.md). ## Mock `runtime/mock` 还提供了名为`Mock`的API, 它与`Patch`十分类似，唯一的区别是第二个参数接受一个拦截器。 diff --git a/cmd/xgo/option.go b/cmd/xgo/option.go index 2f084143..d60cf0cc 100644 --- a/cmd/xgo/option.go +++ b/cmd/xgo/option.go @@ -188,6 +188,10 @@ func parseOptions(cmd string, args []string) (*options, error) { }) } + if cmd == "test" { + trapStdlib = true + } + for i := 0; i < nArg; i++ { arg := args[i] if !strings.HasPrefix(arg, "-") { @@ -266,8 +270,14 @@ func parseOptions(cmd string, args []string) (*options, error) { continue } - if arg == "--trap-stdlib" { - trapStdlib = true + // supported flag: --trap-stdlib, --trap-stdlib=false, --trap-stdlib=true + trapStdlibFlag, trapStdlibVal := flag.TrySingleFlag([]string{"--trap-stdlib"}, arg) + if trapStdlibFlag != "" { + if trapStdlibVal == "" || trapStdlibVal == "true" { + trapStdlib = true + } else { + trapStdlib = false + } continue } diff --git a/cmd/xgo/runtime_gen/core/version.go b/cmd/xgo/runtime_gen/core/version.go index 728ef860..f5b453fe 100755 --- a/cmd/xgo/runtime_gen/core/version.go +++ b/cmd/xgo/runtime_gen/core/version.go @@ -7,8 +7,8 @@ import ( ) const VERSION = "1.0.37" -const REVISION = "2e709f7b1620aea78fdc1dba282bcf6778dd6d51+1" -const NUMBER = 245 +const REVISION = "1d56b338a2c930297d1285877665201ebc0e1077+1" +const NUMBER = 246 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/cmd/xgo/version.go b/cmd/xgo/version.go index b5830f9d..cff1290c 100644 --- a/cmd/xgo/version.go +++ b/cmd/xgo/version.go @@ -3,8 +3,8 @@ package main import "fmt" const VERSION = "1.0.37" -const REVISION = "2e709f7b1620aea78fdc1dba282bcf6778dd6d51+1" -const NUMBER = 245 +const REVISION = "1d56b338a2c930297d1285877665201ebc0e1077+1" +const NUMBER = 246 func getRevision() string { revSuffix := "" diff --git a/patch/ctxt/stdlib.go b/patch/ctxt/stdlib.go index c23f641f..d1832fad 100644 --- a/patch/ctxt/stdlib.go +++ b/patch/ctxt/stdlib.go @@ -1,6 +1,9 @@ package ctxt -import "strings" +import ( + "cmd/compile/internal/types" + "strings" +) var stdWhitelist = map[string]map[string]bool{ // "runtime": map[string]bool{ @@ -73,21 +76,35 @@ var stdBlocklist = map[string]map[string]bool{ "*": true, }, // testing is not harmful - // "testing": map[string]bool{ - // "*": true, - // }, + // but may cause infinite loop? + // we may dig later or just add somce whitelist + "testing": map[string]bool{ + "*": true, + }, "unsafe": map[string]bool{ "*": true, }, } func allowStdFunc(pkgPath string, funcName string) bool { - if XgoStdTrapDefaultAllow { - if stdBlocklist[pkgPath]["*"] || stdBlocklist[pkgPath][funcName] { - return false - } + if isWhitelistStdFunc(pkgPath, funcName) { return true } + if !XgoStdTrapDefaultAllow { + return false + } + + if stdBlocklist[pkgPath]["*"] || stdBlocklist[pkgPath][funcName] { + return false + } + if !types.IsExported(funcName) { + // unexported func + return false + } + return true +} + +func isWhitelistStdFunc(pkgPath string, funcName string) bool { if stdWhitelist[pkgPath][funcName] { return true } diff --git a/runtime/core/version.go b/runtime/core/version.go index 728ef860..f5b453fe 100644 --- a/runtime/core/version.go +++ b/runtime/core/version.go @@ -7,8 +7,8 @@ import ( ) const VERSION = "1.0.37" -const REVISION = "2e709f7b1620aea78fdc1dba282bcf6778dd6d51+1" -const NUMBER = 245 +const REVISION = "1d56b338a2c930297d1285877665201ebc0e1077+1" +const NUMBER = 246 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/runtime/mock/stdlib.md b/runtime/mock/stdlib.md index 28233107..4430abaa 100644 --- a/runtime/mock/stdlib.md +++ b/runtime/mock/stdlib.md @@ -1,11 +1,34 @@ -# Limitation On Stdlib Functions -Stdlib functions like `os.ReadFile`, `io.Read` are widely used by go code. So installing a trap on these functions may have big impact on performance and security. +# Support Of Mocking Stdlib Functions +There are two modes for mocking stdlib functions, +1. Default-Allow Mode +2. Default-Disallow Mode -And as compiler treats stdlib from ordinary module differently, current implementation to support stdlib function is based on source code injection, which may causes build time to slow down. +These two modes can be switched by `--trap-stdlib` and `--trap-stdlib=false`. -So only a limited list of stdlib functions can be mocked. However, if there lacks some functions you may want to use, you can leave a comment in [Issue#6](https://github.com/xhd2015/xgo/issues/6) or fire an issue to let us know and add it. +When running `xgo test`, `--trap-stdlib` is assumed, you can turn it off with `--trap-stdlib=false`. -# Supported List +When running `xgo run` and `xgo build`, `--trap-stdlib=false` is assumed, you can turn it on with `--trap-stdlib`. + +# `--trap-stdlib`: Default-Allow Mode +In this mode, most stdlib packages can be mocked, except `runtime`, `syscall`, `reflect`, `sync`, `sync/atomic`, `testing`, `unsafe`. + +# `--trap-stdlib=false`: Default-Disallow Mode +In this mode, only a small list of stdlib functions can be mocked due to performance and security impact. + +Rational: stdlib functions like `os.ReadFile`, `io.Read` are widely used by go code, installing a trap on these functions may have big impact on performance and security. + +So in this mode only a limited list of stdlib functions can be mocked. However, if there lacks some functions you may want to use, you can leave a comment in [Issue#6](https://github.com/xhd2015/xgo/issues/6) or fire an issue to let us know and add it. + +# Functions In Stdlib Calling `recover()` Cannot Be Mocked +When a function calls `recover()`, it will capture panic when used in defer. + +However, since compiler treats stdlib from ordinary module differently, current implementation to support stdlib function is based on source code injection, which may causes build time to slow down, and also causes functions containing `recover()` to be invalid if rewritten, see https://github.com/xhd2015/xgo/issues/164. + +This will be fixed in the long run, but before that, such functions in stdlib cannot be mocked. + +NOTE: functions outside stdlib, even with calling `recover()`, are not affected since they are rewritten with IR, not source code. + +# Supported List When `--trap-stdlib=false` ## `os` - `Getenv` - `Getwd` diff --git a/runtime/test/trap/trap_avoid_recursive_test.go b/runtime/test/trap/trap_avoid_recursive_test.go index e7240d77..5877ac2d 100644 --- a/runtime/test/trap/trap_avoid_recursive_test.go +++ b/runtime/test/trap/trap_avoid_recursive_test.go @@ -17,10 +17,16 @@ func TestNakedTrapShouldAvoidRecursiveInterceptor(t *testing.T) { var recurseBuf bytes.Buffer trap.AddInterceptor(&trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } fmt.Fprintf(&recurseBuf, "pre\n") return nil, nil }, Post: func(ctx context.Context, f *core.FuncInfo, args, result core.Object, data interface{}) (err error) { + if f.Stdlib { + return nil + } fmt.Fprintf(&recurseBuf, "post\n") return nil }, @@ -41,10 +47,16 @@ func TestDeferredFuncShouldBeExecutedWhenAbort(t *testing.T) { var recurseBuf bytes.Buffer trap.AddInterceptor(&trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } fmt.Fprintf(&recurseBuf, "pre\n") return nil, trap.ErrAbort }, Post: func(ctx context.Context, f *core.FuncInfo, args, result core.Object, data interface{}) (err error) { + if f.Stdlib { + return nil + } fmt.Fprintf(&recurseBuf, "post\n") return nil }, diff --git a/runtime/test/trap/trap_nested_interceptor_test.go b/runtime/test/trap/trap_nested_interceptor_test.go index 8725831c..9ee1c217 100644 --- a/runtime/test/trap/trap_nested_interceptor_test.go +++ b/runtime/test/trap/trap_nested_interceptor_test.go @@ -18,6 +18,9 @@ func TestNestedTrapShouldBeAllowedBySpecifyingMapping(t *testing.T) { // list the names that can be nested trap.AddInterceptor(&trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } traceRecords.WriteString(fmt.Sprintf("call %s\n", f.IdentityName)) if f.IdentityName == "A0" { // call A1 inside the interceptor @@ -29,12 +32,18 @@ func TestNestedTrapShouldBeAllowedBySpecifyingMapping(t *testing.T) { }) trap.AddFuncInterceptor(A1, &trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } traceRecords.WriteString(fmt.Sprintf("call %s\n", f.IdentityName)) return }, }) trap.AddFuncInterceptor(A2, &trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } traceRecords.WriteString(fmt.Sprintf("call %s\n", f.IdentityName)) return }, @@ -68,6 +77,9 @@ func TestNestedTrapPartialAllowShouldTakeEffect(t *testing.T) { var traceRecords bytes.Buffer trap.AddInterceptor(&trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } traceRecords.WriteString(fmt.Sprintf("call %s\n", f.IdentityName)) if f.IdentityName == "B0" { // call A1 inside the interceptor @@ -80,6 +92,9 @@ func TestNestedTrapPartialAllowShouldTakeEffect(t *testing.T) { trap.AddFuncInterceptor(B2, &trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } traceRecords.WriteString(fmt.Sprintf("call %s\n", f.IdentityName)) return }, diff --git a/runtime/test/trap/trap_return_err_test.go b/runtime/test/trap/trap_return_err_test.go index ee18d226..b586a18e 100644 --- a/runtime/test/trap/trap_return_err_test.go +++ b/runtime/test/trap/trap_return_err_test.go @@ -14,6 +14,9 @@ func TestTrapReturnedErrorShouldBeSet(t *testing.T) { mockErr := errors.New("must not be an odd") trap.AddInterceptor(&trap.Interceptor{ Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } return nil, mockErr }, }) @@ -30,6 +33,9 @@ func TestTrapPosReturnedErrorShouldBeSet(t *testing.T) { mockErr := errors.New("must not be an odd") trap.AddInterceptor(&trap.Interceptor{ Post: func(ctx context.Context, f *core.FuncInfo, args, result core.Object, data interface{}) error { + if f.Stdlib { + return nil + } return mockErr }, }) diff --git a/runtime/test/trap_args/err_test.go b/runtime/test/trap_args/err_test.go index 988e3f96..2ebe6ff3 100644 --- a/runtime/test/trap_args/err_test.go +++ b/runtime/test/trap_args/err_test.go @@ -47,6 +47,9 @@ func TestSubErrShouldNotSetErrRes(t *testing.T) { callAndCheck(func() { err = subErr() }, func(trapCtx context.Context, f *core.FuncInfo, args, result core.Object) error { + if f.Stdlib { + return nil + } if f.LastResultErr { t.Fatalf("expect f.LastResultErr to be false, actual: true") } diff --git a/runtime/test/trap_args/trap_args_test.go b/runtime/test/trap_args/trap_args_test.go index 12a119d1..3ede652a 100644 --- a/runtime/test/trap_args/trap_args_test.go +++ b/runtime/test/trap_args/trap_args_test.go @@ -48,6 +48,9 @@ func TestCtxArgWithRecvCanBeRecognized(t *testing.T) { var callCount int trap.AddInterceptor(&trap.Interceptor{ Pre: func(trapCtx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } callCount++ if !f.FirstArgCtx { t.Fatalf("expect first arg to be context") @@ -80,6 +83,9 @@ func callAndCheck(fn func(), check func(trapCtx context.Context, f *core.FuncInf var callCount int trap.AddInterceptor(&trap.Interceptor{ Pre: func(trapCtx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + if f.Stdlib { + return nil, nil + } if callCount == 0 { callCount++ if pcName != f.FullName { diff --git a/script/run-test/main.go b/script/run-test/main.go index 85aefe6e..70dd0c90 100644 --- a/script/run-test/main.go +++ b/script/run-test/main.go @@ -42,15 +42,14 @@ import ( // run specific test for all go versions // go run ./script/run-test/ --include go1.17.13 --include go1.18.10 --include go1.19.13 --include go1.20.14 --include go1.21.8 --include go1.22.1 -count=1 --xgo-default-test-only -run TestFuncNameSliceShouldWorkWithDebug -v +var globalFlags = []string{"-timeout=60s"} + // TODO: remove duplicate test between xgo test and runtime test var runtimeSubTests = []string{ "func_list", "trap", "trap_inspect_func", "trap_args", - "trace", - "trace_marshal", - "trace_panic_peek", "mock_func", "mock_method", "mock_by_name", @@ -64,9 +63,10 @@ var runtimeSubTests = []string{ } type TestCase struct { - name string - dir string - flags []string + name string + dir string + flags []string + windowsFlags []string } var extraSubTests = []*TestCase{ @@ -74,17 +74,21 @@ var extraSubTests = []*TestCase{ name: "trace_without_dep", dir: "runtime/test/trace_without_dep", flags: []string{"--strace"}, + // see https://github.com/xhd2015/xgo/issues/144#issuecomment-2138565532 + windowsFlags: []string{"--trap-stdlib=false", "--strace"}, }, { - name: "trace_without_dep_vendor", - dir: "runtime/test/trace_without_dep_vendor", - flags: []string{"--strace"}, + name: "trace_without_dep_vendor", + dir: "runtime/test/trace_without_dep_vendor", + flags: []string{"--strace"}, + windowsFlags: []string{"--trap-stdlib=false", "--strace"}, }, { // see https://github.com/xhd2015/xgo/issues/87 - name: "trace_without_dep_vendor_replace", - dir: "runtime/test/trace_without_dep_vendor_replace", - flags: []string{"--strace"}, + name: "trace_without_dep_vendor_replace", + dir: "runtime/test/trace_without_dep_vendor_replace", + flags: []string{"--strace"}, + windowsFlags: []string{"--trap-stdlib=false", "--strace"}, }, { name: "trap_with_overlay", @@ -103,6 +107,26 @@ var extraSubTests = []*TestCase{ dir: "runtime/test/bugs/...", flags: []string{}, }, + { + name: "trace_marshal", + dir: "runtime/test/trace_marshal/...", + flags: []string{}, + windowsFlags: []string{"--trap-stdlib=false"}, + }, + { + // see https://github.com/xhd2015/xgo/issues/142 + name: "trace", + dir: "runtime/test/trace/...", + flags: []string{}, + windowsFlags: []string{"--trap-stdlib=false"}, + }, + { + // see https://github.com/xhd2015/xgo/issues/142 + name: "trace_panic_peek", + dir: "runtime/test/trace_panic_peek/...", + flags: []string{}, + windowsFlags: []string{"--trap-stdlib=false"}, + }, { // see https://github.com/xhd2015/xgo/issues/164 name: "stdlib_recover_no_trap", @@ -487,7 +511,11 @@ func runRuntimeSubTest(goroot string, args []string, tests []string, names []str } else { testArgDir = "./..." } - err := doRunTest(goroot, testKind_xgoAny, amendArgs([]string{"--project-dir", dotDir}, tt.flags), []string{testArgDir}) + testFlags := tt.flags + if runtime.GOOS == "windows" && tt.windowsFlags != nil { + testFlags = tt.windowsFlags + } + err := doRunTest(goroot, testKind_xgoAny, amendArgs([]string{"--project-dir", dotDir}, testFlags), []string{testArgDir}) if err != nil { return err } @@ -527,6 +555,7 @@ func doRunTest(goroot string, kind testKind, args []string, tests []string) erro switch kind { case testKind_default: testArgs = []string{"test"} + testArgs = append(testArgs, globalFlags...) testArgs = append(testArgs, args...) if len(tests) > 0 { testArgs = append(testArgs, tests...) @@ -546,6 +575,7 @@ func doRunTest(goroot string, kind testKind, args []string, tests []string) erro } case testKind_xgoTest: testArgs = []string{"run", "-tags", "dev", "./cmd/xgo", "test"} + testArgs = append(testArgs, globalFlags...) testArgs = append(testArgs, args...) if len(tests) > 0 { testArgs = append(testArgs, tests...) @@ -569,6 +599,7 @@ func doRunTest(goroot string, kind testKind, args []string, tests []string) erro } case testKind_runtimeSubTest: testArgs = []string{"run", "-tags", "dev", "./cmd/xgo", "test", "--project-dir", "runtime/test"} + testArgs = append(testArgs, globalFlags...) testArgs = append(testArgs, args...) if len(tests) > 0 { testArgs = append(testArgs, tests...) @@ -579,6 +610,7 @@ func doRunTest(goroot string, kind testKind, args []string, tests []string) erro } case testKind_xgoAny: testArgs = []string{"run", "-tags", "dev", "./cmd/xgo", "test"} + testArgs = append(testArgs, globalFlags...) testArgs = append(testArgs, args...) testArgs = append(testArgs, tests...) }