dynamically check trap mark to avoid nested trap issue

cmd/xgo/version.go

@@ -2,9 +2,9 @@ package main
 
 import "fmt"
 
-const VERSION = "1.0.0"
-const REVISION = "37d37c71b68249a79d6cb3347d41945295ecde57+1"
-const NUMBER = 79
+const VERSION = "1.0.1"
+const REVISION = "4a87957ab7175717205880ca95086e259f342c25+1"
+const NUMBER = 80
 
 func getRevision() string {
 	return fmt.Sprintf("%s %s BUILD_%d", VERSION, REVISION, NUMBER)

patch/adapter_go1.17.go

@@ -0,0 +1,7 @@
+//go:build go1.17 && !go1.18
+// +build go1.17,!go1.18
+
+package patch
+
+const goMajor = 1
+const goMinor = 17

patch/adapter_go1.18.go

@@ -0,0 +1,7 @@
+//go:build go1.18 && !go1.19
+// +build go1.18,!go1.19
+
+package patch
+
+const goMajor = 1
+const goMinor = 18

patch/adapter_go1.19.go

@@ -9,6 +9,9 @@ import (
 	xgo_ctxt "cmd/compile/internal/xgo_rewrite_internal/patch/ctxt"
 )
 
+const goMajor = 1
+const goMinor = 19
+
 const genericTrapNeedsWorkaround = true
 
 func NewSignature(pkg *types.Pkg, recv *types.Field, tparams, params, results []*types.Field) *types.Type {

patch/adapter_go1.20.go

@@ -10,6 +10,9 @@ import (
 	"cmd/compile/internal/types"
 )
 
+const goMajor = 1
+const goMinor = 20
+
 const genericTrapNeedsWorkaround = false
 
 // different with go1.20

patch/adapter_go1.21.go

@@ -10,6 +10,9 @@ import (
 	"cmd/compile/internal/types"
 )
 
+const goMajor = 1
+const goMinor = 21
+
 const genericTrapNeedsWorkaround = false
 
 // different with go1.20

patch/adapter_go1.22.go

@@ -14,6 +14,9 @@ import (
 	"cmd/internal/src"
 )
 
+const goMajor = 1
+const goMinor = 22
+
 const genericTrapNeedsWorkaround = true
 
 func forEachFunc(callback func(fn *ir.Func) bool) {

patch/trap.go

@@ -24,6 +24,13 @@ func init() {
 	if sig_gen__xgo_trap != sig_expected__xgo_trap {
 		panic(fmt.Errorf("__xgo_trap signature changed, run go generate and update sig_expected__xgo_trap correspondly"))
 	}
+	if goMajor != 1 {
+		panic(fmt.Errorf("expect goMajor to be 1, actual:%d", goMajor))
+	}
+
+	if goMinor < 17 || goMinor > 22 {
+		panic(fmt.Errorf("expect goMinor 17~22, actual:%d", goMinor))
+	}
 }
 
 const disableXgoLink bool = false
@@ -204,22 +211,30 @@ func CanInsertTrapOrLink(fn *ir.Func) (string, bool) {
 	}
 */
 
+// for go1.17,go1.18
+//
+//	fn.Sym().Pkg.Path
+//
+// returns empty
+const hasFuncPkgPath = goMajor > 1 || (goMajor == 1 && goMinor >= 19)
+
 func InsertTrapForFunc(fn *ir.Func, forGeneric bool) bool {
 	ensureInit()
 
-	curPkgPath := xgo_ctxt.GetPkgPath()
-
-	var fnPkg string
-	fnSym := fn.Sym()
-	if fnSym != nil {
-		fnPkg = fnSym.Pkg.Path
-	}
-	// when package are not the same,
-	// do not insert trap points
-	// TODO: solve the generic package issues by
-	// using generic metadata
-	if fnPkg == "" || fnPkg != curPkgPath {
-		return false
+	if hasFuncPkgPath {
+		curPkgPath := xgo_ctxt.GetPkgPath()
+		var fnPkg string
+		fnSym := fn.Sym()
+		if fnSym != nil {
+			fnPkg = fnSym.Pkg.Path
+		}
+		// when package are not the same,
+		// do not insert trap points
+		// TODO: solve the generic package issues by
+		// using generic metadata
+		if fnPkg == "" || fnPkg != curPkgPath {
+			return false
+		}
 	}
 
 	pos := base.Ctxt.PosTable.Pos(fn.Pos())

runtime/core/version.go

@@ -1,5 +1,5 @@
 package core
 
-const VERSION = "1.0.0"
-const REVISION = "37d37c71b68249a79d6cb3347d41945295ecde57+1"
-const NUMBER = 79
+const VERSION = "1.0.1"
+const REVISION = "4a87957ab7175717205880ca95086e259f342c25+1"
+const NUMBER = 80

runtime/trap/interceptor.go

@@ -38,7 +38,7 @@ func init() {
 				panic(e)
 			}
 		}()
-		__xgo_link_on_goexit(clearLocalInterceptors)
+		__xgo_link_on_goexit(clearLocalInterceptorsAndMark)
 	}()
 }
 
@@ -84,7 +84,7 @@ func GetLocalInterceptors() []*Interceptor {
 }
 
 func ClearLocalInterceptors() {
-	clearLocalInterceptors()
+	clearLocalInterceptorsAndMark()
 }
 
 func GetAllInterceptors() []*Interceptor {
@@ -145,7 +145,9 @@ type interceptorList struct {
 	interceptors []*Interceptor
 }
 
-func clearLocalInterceptors() {
+func clearLocalInterceptorsAndMark() {
 	key := __xgo_link_getcurg()
 	localInterceptors.Delete(key)
+
+	clearTrappingMark()
 }

runtime/trap/trap.go

@@ -22,17 +22,24 @@ func __xgo_link_set_trap(trapImpl func(pkgPath string, identityName string, gene
 	panic("failed to link __xgo_link_set_trap")
 }
 
-func Skip() {
-	// this is intenionally leave empty
-	// as trap.Skip() is a function used
-	// to mark the caller should not be trapped.
-	// one can also use trap.Skip() in
-	// the non-interceptor context
-}
+// Skip serves as mark to tell xgo not insert
+// trap instructions for the function that
+// calls Skip()
+// NOTE: the function body is intenionally leave empty
+// as trap.Skip() is just a mark that makes
+// sense at compile time.
+func Skip() {}
+
+var trappingMark sync.Map // <goroutine key> -> struct{}{}
 
 // link to runtime
 // xgo:notrap
 func trapImpl(pkgPath string, identityName string, generic bool, pc uintptr, recv interface{}, args []interface{}, results []interface{}) (func(), bool) {
+	dispose := setTrappingMark()
+	if dispose == nil {
+		return nil, false
+	}
+	defer dispose()
 	type intf struct {
 		_  uintptr
 		pc *uintptr
@@ -174,6 +181,11 @@ func trapImpl(pkgPath string, identityName string, generic bool, pc uintptr, rec
 	}
 
 	return func() {
+		dispose := setTrappingMark()
+		if dispose == nil {
+			return
+		}
+		defer dispose()
 		for i := 0; i < n; i++ {
 			interceptor := interceptors[i]
 			if interceptor.Post == nil {
@@ -194,3 +206,19 @@ func trapImpl(pkgPath string, identityName string, generic bool, pc uintptr, rec
 		}
 	}, false
 }
+
+func setTrappingMark() func() {
+	key := uintptr(__xgo_link_getcurg())
+	_, trapping := trappingMark.LoadOrStore(key, struct{}{})
+	if trapping {
+		return nil
+	}
+	return func() {
+		trappingMark.Delete(key)
+	}
+}
+
+func clearTrappingMark() {
+	key := uintptr(__xgo_link_getcurg())
+	trappingMark.Delete(key)
+}

script/run-test/main.go

@@ -12,7 +12,7 @@ import (
 //
 //	go run ./script/run-test/ --include go1.19.13
 //	go run ./script/run-test/ --include go1.19.13 -run TestHelloWorld -v
-//	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 --reset-instrument
+//	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
 func main() {
 	args := os.Args[1:]
 	var excludes []string

test/testdata/trap_nested/main.go

@@ -0,0 +1,39 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"github.com/xhd2015/xgo/runtime/core"
+	"github.com/xhd2015/xgo/runtime/trap"
+)
+
+// Prior to xgo v1.0.1, explicit trap.Skip()
+// must be made to avoid stack overflow.
+// This is called 'the nested trap' problem.
+//
+// After xgo v1.0.1,xgo can check dynamically
+// whether current goroutine is inside trap, if so
+// skip it
+
+func main() {
+	if os.Getenv("XGO_TEST_HAS_INSTRUMENT") != "false" {
+		trap.AddInterceptor(&trap.Interceptor{
+			Pre: func(ctx context.Context, f *core.FuncInfo, args core.Object, results core.Object) (interface{}, error) {
+				fmt.Printf("trap pre: %s\n", f.Name)
+				callFromTrap()
+				return nil, nil
+			},
+		})
+	}
+	hello()
+}
+
+func hello() {
+	fmt.Printf("hello world\n")
+}
+
+func callFromTrap() {
+	fmt.Printf("call from trap\n")
+}

test/trap_local_test.go

@@ -19,3 +19,11 @@ func TestTrapGoroutineLocal(t *testing.T) {
 	expectOut := "call: goroutineStr\nprintHello: goroutine\nmain: goroutine\n"
 	testTrap(t, "./testdata/goroutine_trap", origExpect, expectOut)
 }
+
+// go test -run TestTrapNestedShouldAvoidRecursive -v ./test
+func TestTrapNestedShouldAvoidRecursive(t *testing.T) {
+	t.Parallel()
+	origExpect := "hello world\n"
+	expectOut := "trap pre: hello\ncall from trap\nhello world\n"
+	testTrap(t, "./testdata/trap_nested", origExpect, expectOut)
+}