diff --git a/.github/workflows/nightly-test.yml b/.github/workflows/nightly-test.yml index d1f7b6bd..3358e392 100644 --- a/.github/workflows/nightly-test.yml +++ b/.github/workflows/nightly-test.yml @@ -71,7 +71,7 @@ jobs: run: | source util.sh cd gin-gonic/gin - xgo test -v ./... + xgo test --trap-stdlib -v ./... - uses: actions/checkout@v4 with: @@ -83,7 +83,7 @@ jobs: run: | source util.sh cd pocketbase/pocketbase - xgo test -v ./... + xgo test --trap-stdlib -v ./... - uses: actions/checkout@v4 with: @@ -95,7 +95,7 @@ jobs: run: | source util.sh cd gohugoio/hugo - xgo test -v ./... + xgo test --trap-stdlib -v ./... - uses: actions/checkout@v4 with: @@ -107,7 +107,7 @@ jobs: run: | source util.sh cd kubernetes/kubernetes - xgo test -v ./... + xgo test --trap-stdlib -v ./... - name: Summary run: | diff --git a/cmd/xgo/main.go b/cmd/xgo/main.go index d14b4a72..d34e990b 100644 --- a/cmd/xgo/main.go +++ b/cmd/xgo/main.go @@ -299,7 +299,8 @@ func handleBuild(cmd string, args []string) error { return err } } - if isDevelopment || resetInstrument || revisionChanged { + resetOrRevisionChanged := resetInstrument || revisionChanged + if isDevelopment || resetOrRevisionChanged { logDebug("sync goroot %s -> %s", goroot, instrumentGoroot) err = syncGoroot(goroot, instrumentGoroot, fullSyncRecord) if err != nil { @@ -307,13 +308,13 @@ func handleBuild(cmd string, args []string) error { } // patch go runtime and compiler logDebug("patch compiler at: %s", instrumentGoroot) - err = patchRuntimeAndCompiler(goroot, instrumentGoroot, realXgoSrc, goVersion, syncWithLink || setupDev || buildCompiler, revisionChanged) + err = patchRuntimeAndCompiler(goroot, instrumentGoroot, realXgoSrc, goVersion, syncWithLink || setupDev || buildCompiler, resetOrRevisionChanged) if err != nil { return err } } - if resetInstrument || revisionChanged { + if resetOrRevisionChanged { logDebug("revision %s write to %s", revision, revisionFile) err := os.WriteFile(revisionFile, []byte(revision), 0755) if err != nil { diff --git a/cmd/xgo/patch.go b/cmd/xgo/patch.go index b15fff29..48e12f7a 100644 --- a/cmd/xgo/patch.go +++ b/cmd/xgo/patch.go @@ -40,14 +40,14 @@ func init() { // assume go 1.20 // the patch should be idempotent // the origGoroot is used to generate runtime defs, see https://github.com/xhd2015/xgo/issues/4#issuecomment-2017880791 -func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, goVersion *goinfo.GoVersion, syncWithLink bool, revisionChanged bool) error { +func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, goVersion *goinfo.GoVersion, syncWithLink bool, resetOrRevisionChanged bool) error { if goroot == "" { return fmt.Errorf("requires goroot") } if isDevelopment && xgoSrc == "" { return fmt.Errorf("requires xgoSrc") } - if !isDevelopment && !revisionChanged { + if !isDevelopment && !resetOrRevisionChanged { return nil } @@ -58,7 +58,7 @@ func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, go } // compiler - err = patchCompiler(origGoroot, goroot, goVersion, xgoSrc, revisionChanged, syncWithLink) + err = patchCompiler(origGoroot, goroot, goVersion, xgoSrc, resetOrRevisionChanged, syncWithLink) if err != nil { return err } diff --git a/cmd/xgo/runtime_gen/core/version.go b/cmd/xgo/runtime_gen/core/version.go index ef63577b..bfd145a5 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 = "f52fb37283a2f6c877d110627da71df63638a916+1" -const NUMBER = 243 +const REVISION = "8663e050aab31ade5fb04767b355cbdd830fd926+1" +const NUMBER = 245 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/cmd/xgo/version.go b/cmd/xgo/version.go index 68e863a6..83f2bd67 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 = "f52fb37283a2f6c877d110627da71df63638a916+1" -const NUMBER = 243 +const REVISION = "8663e050aab31ade5fb04767b355cbdd830fd926+1" +const NUMBER = 245 func getRevision() string { revSuffix := "" diff --git a/patch/syntax/rewrite.go b/patch/syntax/rewrite.go index 10238a18..f2550aac 100644 --- a/patch/syntax/rewrite.go +++ b/patch/syntax/rewrite.go @@ -94,7 +94,7 @@ func replaceIdent(root syntax.Node, match string, to string) { }) } -func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) { +func rewriteFuncsSource(funcDecls []*DeclInfo, pkgPath string) { for _, fn := range funcDecls { if !fn.Kind.IsFunc() { continue @@ -119,6 +119,13 @@ func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) { fnDecl := fn.FuncDecl pos := fn.FuncDecl.Pos() + // check if body contains recover(), if so + // do not add interceptor + // see https://github.com/xhd2015/xgo/issues/164 + if hasRecoverCall(fnDecl.Body) { + continue + } + fnName := fnDecl.Name.Value // dump @@ -136,6 +143,7 @@ func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) { // cannot trap continue } + // stop if __xgo_link_generated_trap conflict with recv? preset[XgoLinkTrapForGenerated] = true @@ -414,6 +422,23 @@ func getPresetNames(node syntax.Node) map[string]bool { return preset } +func hasRecoverCall(node syntax.Node) bool { + var found bool + syntax.Inspect(node, func(n syntax.Node) bool { + if n == nil { + return false + } + if call, ok := n.(*syntax.CallExpr); ok { + if idt, ok := call.Fun.(*syntax.Name); ok && idt.Value == "recover" { + found = true + return false + } + } + return true + }) + return found +} + func copyFuncDeclWithoutBody(decl *syntax.FuncDecl) *syntax.FuncDecl { return copyFuncDecl(decl, false) } diff --git a/patch/syntax/syntax.go b/patch/syntax/syntax.go index 07f42e6e..f7c2e5ba 100644 --- a/patch/syntax/syntax.go +++ b/patch/syntax/syntax.go @@ -250,8 +250,9 @@ func registerAndTrapFuncs(fileList []*syntax.File, addFile func(name string, r i // assign to global allDecls = funcDelcs - // std lib functions - rewriteStdAndGenericFuncs(funcDelcs, pkgPath) + // std lib, and generic functions + // normal functions uses IR + rewriteFuncsSource(funcDelcs, pkgPath) if varTrap { trapVariables(pkgPath, fileList, funcDelcs) diff --git a/runtime/core/version.go b/runtime/core/version.go index ef63577b..bfd145a5 100644 --- a/runtime/core/version.go +++ b/runtime/core/version.go @@ -7,8 +7,8 @@ import ( ) const VERSION = "1.0.37" -const REVISION = "f52fb37283a2f6c877d110627da71df63638a916+1" -const NUMBER = 243 +const REVISION = "8663e050aab31ade5fb04767b355cbdd830fd926+1" +const NUMBER = 245 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/runtime/test/recover_no_trap/recover_no_trap.go b/runtime/test/recover_no_trap/recover_no_trap.go new file mode 100644 index 00000000..0254100d --- /dev/null +++ b/runtime/test/recover_no_trap/recover_no_trap.go @@ -0,0 +1,10 @@ +package recover_no_trap + +func A() { + defer B() +} + +func B() string { + recover() + return "B" +} diff --git a/runtime/test/recover_no_trap/recover_no_trap_test.go b/runtime/test/recover_no_trap/recover_no_trap_test.go new file mode 100644 index 00000000..8d07d260 --- /dev/null +++ b/runtime/test/recover_no_trap/recover_no_trap_test.go @@ -0,0 +1,59 @@ +package recover_no_trap + +import ( + "context" + "testing" + "text/template" + + "github.com/xhd2015/xgo/runtime/core" + "github.com/xhd2015/xgo/runtime/mock" +) + +// bug see https://github.com/xhd2015/xgo/issues/164 +// flags: --trap-stdlib +func TestRecoverInStdlibShouldCapturePanic(t *testing.T) { + // if the implementation is right, no panic should happen + txt := `test {{define "content"}}` + + _, err := template.New("").Parse(txt) + expectMsg := "template: :1: unexpected EOF" + if err == nil || err.Error() != expectMsg { + t.Fatalf("expect parse err: %q, actual: %q", expectMsg, err.Error()) + } +} + +func TestRecoverInNonStdlibShouldBeTrapped(t *testing.T) { + var haveMockedA bool + mock.Mock(A, func(ctx context.Context, fn *core.FuncInfo, args, results core.Object) error { + haveMockedA = true + return nil + }) + A() + if !haveMockedA { + t.Fatalf("expect have mocked A, actually not") + } + + var mockBSetupErr interface{} + var haveMockedB bool + var result string + func() { + defer func() { + mockBSetupErr = recover() + }() + mock.Mock(B, func(ctx context.Context, fn *core.FuncInfo, args, results core.Object) error { + haveMockedB = true + return nil + }) + result = B() + }() + + if mockBSetupErr != nil { + t.Fatalf("expect setup mock B no error, actual: %v", mockBSetupErr) + } + if !haveMockedB { + t.Fatalf("expect haveMockedB to be true, actual: false") + } + if result != "" { + t.Fatalf("expect B() returns mocked empty string, actual: %v", result) + } +} diff --git a/script/run-test/main.go b/script/run-test/main.go index 87f7eeef..85aefe6e 100644 --- a/script/run-test/main.go +++ b/script/run-test/main.go @@ -103,6 +103,12 @@ var extraSubTests = []*TestCase{ dir: "runtime/test/bugs/...", flags: []string{}, }, + { + // see https://github.com/xhd2015/xgo/issues/164 + name: "stdlib_recover_no_trap", + dir: "runtime/test/recover_no_trap", + flags: []string{"--trap-stdlib"}, + }, } func main() {