Skip to content

Commit 8fdd5cc

Browse files
xhd2015xhd2015
committed
skip trap recover()-like funcs in stdlib
1 parent 5009615 commit 8fdd5cc

File tree

10 files changed

+117
-15
lines changed

10 files changed

+117
-15
lines changed

‎cmd/xgo/main.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -299,21 +299,22 @@ func handleBuild(cmd string, args []string) error {
299299
return err
300300
}
301301
}
302-
if isDevelopment || resetInstrument || revisionChanged {
302+
resetOrRevisionChanged := resetInstrument || revisionChanged
303+
if isDevelopment || resetOrRevisionChanged {
303304
logDebug("sync goroot %s -> %s", goroot, instrumentGoroot)
304305
err = syncGoroot(goroot, instrumentGoroot, fullSyncRecord)
305306
if err != nil {
306307
return err
307308
}
308309
// patch go runtime and compiler
309310
logDebug("patch compiler at: %s", instrumentGoroot)
310-
err = patchRuntimeAndCompiler(goroot, instrumentGoroot, realXgoSrc, goVersion, syncWithLink || setupDev || buildCompiler, revisionChanged)
311+
err = patchRuntimeAndCompiler(goroot, instrumentGoroot, realXgoSrc, goVersion, syncWithLink || setupDev || buildCompiler, resetOrRevisionChanged)
311312
if err != nil {
312313
return err
313314
}
314315
}
315316

316-
if resetInstrument || revisionChanged {
317+
if resetOrRevisionChanged {
317318
logDebug("revision %s write to %s", revision, revisionFile)
318319
err := os.WriteFile(revisionFile, []byte(revision), 0755)
319320
if err != nil {

‎cmd/xgo/patch.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ func init() {
4040
// assume go 1.20
4141
// the patch should be idempotent
4242
// the origGoroot is used to generate runtime defs, see https://github.com/xhd2015/xgo/issues/4#issuecomment-2017880791
43-
func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, goVersion *goinfo.GoVersion, syncWithLink bool, revisionChanged bool) error {
43+
func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, goVersion *goinfo.GoVersion, syncWithLink bool, resetOrRevisionChanged bool) error {
4444
if goroot == "" {
4545
return fmt.Errorf("requires goroot")
4646
}
4747
if isDevelopment && xgoSrc == "" {
4848
return fmt.Errorf("requires xgoSrc")
4949
}
50-
if !isDevelopment && !revisionChanged {
50+
if !isDevelopment && !resetOrRevisionChanged {
5151
return nil
5252
}
5353

@@ -58,7 +58,7 @@ func patchRuntimeAndCompiler(origGoroot string, goroot string, xgoSrc string, go
5858
}
5959

6060
// compiler
61-
err = patchCompiler(origGoroot, goroot, goVersion, xgoSrc, revisionChanged, syncWithLink)
61+
err = patchCompiler(origGoroot, goroot, goVersion, xgoSrc, resetOrRevisionChanged, syncWithLink)
6262
if err != nil {
6363
return err
6464
}

‎cmd/xgo/runtime_gen/core/version.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
)
88

99
const VERSION = "1.0.37"
10-
const REVISION = "f52fb37283a2f6c877d110627da71df63638a916+1"
11-
const NUMBER = 243
10+
const REVISION = "ff558f48064e903570581aa9ca317c0f043ae880+1"
11+
const NUMBER = 244
1212

1313
// these fields will be filled by compiler
1414
const XGO_VERSION = ""

‎cmd/xgo/version.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ package main
33
import "fmt"
44

55
const VERSION = "1.0.37"
6-
const REVISION = "f52fb37283a2f6c877d110627da71df63638a916+1"
7-
const NUMBER = 243
6+
const REVISION = "ff558f48064e903570581aa9ca317c0f043ae880+1"
7+
const NUMBER = 244
88

99
func getRevision() string {
1010
revSuffix := ""

‎patch/syntax/rewrite.go

+26-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func replaceIdent(root syntax.Node, match string, to string) {
9494
})
9595
}
9696

97-
func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) {
97+
func rewriteFuncsSource(funcDecls []*DeclInfo, pkgPath string) {
9898
for _, fn := range funcDecls {
9999
if !fn.Kind.IsFunc() {
100100
continue
@@ -119,6 +119,13 @@ func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) {
119119
fnDecl := fn.FuncDecl
120120
pos := fn.FuncDecl.Pos()
121121

122+
// check if body contains recover(), if so
123+
// do not add interceptor
124+
// see https://github.com/xhd2015/xgo/issues/164
125+
if hasRecoverCall(fnDecl.Body) {
126+
continue
127+
}
128+
122129
fnName := fnDecl.Name.Value
123130

124131
// dump
@@ -136,6 +143,7 @@ func rewriteStdAndGenericFuncs(funcDecls []*DeclInfo, pkgPath string) {
136143
// cannot trap
137144
continue
138145
}
146+
139147
// stop if __xgo_link_generated_trap conflict with recv?
140148
preset[XgoLinkTrapForGenerated] = true
141149

@@ -414,6 +422,23 @@ func getPresetNames(node syntax.Node) map[string]bool {
414422
return preset
415423
}
416424

425+
func hasRecoverCall(node syntax.Node) bool {
426+
var found bool
427+
syntax.Inspect(node, func(n syntax.Node) bool {
428+
if n == nil {
429+
return false
430+
}
431+
if call, ok := n.(*syntax.CallExpr); ok {
432+
if idt, ok := call.Fun.(*syntax.Name); ok && idt.Value == "recover" {
433+
found = true
434+
return false
435+
}
436+
}
437+
return true
438+
})
439+
return found
440+
}
441+
417442
func copyFuncDeclWithoutBody(decl *syntax.FuncDecl) *syntax.FuncDecl {
418443
return copyFuncDecl(decl, false)
419444
}

‎patch/syntax/syntax.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,9 @@ func registerAndTrapFuncs(fileList []*syntax.File, addFile func(name string, r i
250250
// assign to global
251251
allDecls = funcDelcs
252252

253-
// std lib functions
254-
rewriteStdAndGenericFuncs(funcDelcs, pkgPath)
253+
// std lib, and generic functions
254+
// normal functions uses IR
255+
rewriteFuncsSource(funcDelcs, pkgPath)
255256

256257
if varTrap {
257258
trapVariables(pkgPath, fileList, funcDelcs)

‎runtime/core/version.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
)
88

99
const VERSION = "1.0.37"
10-
const REVISION = "f52fb37283a2f6c877d110627da71df63638a916+1"
11-
const NUMBER = 243
10+
const REVISION = "ff558f48064e903570581aa9ca317c0f043ae880+1"
11+
const NUMBER = 244
1212

1313
// these fields will be filled by compiler
1414
const XGO_VERSION = ""

‎runtime/test/recover_no_trap/recover_no_trap.go

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package recover_no_trap
2+
3+
func A() {
4+
defer B()
5+
}
6+
7+
func B() string {
8+
recover()
9+
return "B"
10+
}

‎runtime/test/recover_no_trap/recover_no_trap_test.go

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package recover_no_trap
2+
3+
import (
4+
"context"
5+
"testing"
6+
"text/template"
7+
8+
"github.com/xhd2015/xgo/runtime/core"
9+
"github.com/xhd2015/xgo/runtime/mock"
10+
)
11+
12+
// bug see https://github.com/xhd2015/xgo/issues/164
13+
// flags: --trap-stdlib
14+
func TestRecoverInStdlibShouldCapturePanic(t *testing.T) {
15+
// if the implementation is right, no panic should happen
16+
txt := `test {{define "content"}}`
17+
18+
_, err := template.New("").Parse(txt)
19+
expectMsg := "template: :1: unexpected EOF"
20+
if err == nil || err.Error() != expectMsg {
21+
t.Fatalf("expect parse err: %q, actual: %q", expectMsg, err.Error())
22+
}
23+
}
24+
25+
func TestRecoverInNonStdlibShouldBeTrapped(t *testing.T) {
26+
var haveMockedA bool
27+
mock.Mock(A, func(ctx context.Context, fn *core.FuncInfo, args, results core.Object) error {
28+
haveMockedA = true
29+
return nil
30+
})
31+
A()
32+
if !haveMockedA {
33+
t.Fatalf("expect have mocked A, actually not")
34+
}
35+
36+
var mockBSetupErr interface{}
37+
var haveMockedB bool
38+
var result string
39+
func() {
40+
defer func() {
41+
mockBSetupErr = recover()
42+
}()
43+
mock.Mock(B, func(ctx context.Context, fn *core.FuncInfo, args, results core.Object) error {
44+
haveMockedB = true
45+
return nil
46+
})
47+
result = B()
48+
}()
49+
50+
if mockBSetupErr != nil {
51+
t.Fatalf("expect setup mock B no error, actual: %v", mockBSetupErr)
52+
}
53+
if !haveMockedB {
54+
t.Fatalf("expect haveMockedB to be true, actual: false")
55+
}
56+
if result != "" {
57+
t.Fatalf("expect B() returns mocked empty string, actual: %v", result)
58+
}
59+
}

‎script/run-test/main.go

+6
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ var extraSubTests = []*TestCase{
103103
dir: "runtime/test/bugs/...",
104104
flags: []string{},
105105
},
106+
{
107+
// see https://github.com/xhd2015/xgo/issues/164
108+
name: "stdlib_recover_no_trap",
109+
dir: "runtime/test/recover_no_trap_test",
110+
flags: []string{"--trap-stdlib"},
111+
},
106112
}
107113

108114
func main() {

0 commit comments

Comments
 (0)