From e937a08ba9f6a3beda0cee64d93f97d568c8f0cc Mon Sep 17 00:00:00 2001 From: xhd2015 Date: Sun, 14 Apr 2024 22:50:22 +0800 Subject: [PATCH] avoid copying locks when intercepting variables --- cmd/xgo/patch_compiler.go | 1 + cmd/xgo/patch_compiler_ast_type_check.go | 66 ++++++++++- cmd/xgo/version.go | 4 +- patch/pkgdata/pkgdata.go | 65 +++++++++- patch/syntax/syntax.go | 51 +++++++- patch/syntax/vars.go | 65 ++++++++-- runtime/core/version.go | 4 +- runtime/test/debug/debug_test.go | 34 +----- .../mock_var/mock_config_should_copy_test.go | 48 ++++++++ .../test/mock_var/mock_no_value_copy_test.go | 112 ++++++++++++++++++ runtime/test/mock_var/mock_var_test.go | 2 +- .../generic_names_go1.18_19_test.go | 7 +- 12 files changed, 397 insertions(+), 62 deletions(-) create mode 100644 runtime/test/mock_var/mock_config_should_copy_test.go create mode 100644 runtime/test/mock_var/mock_no_value_copy_test.go diff --git a/cmd/xgo/patch_compiler.go b/cmd/xgo/patch_compiler.go index 687ad128..dd5c3243 100644 --- a/cmd/xgo/patch_compiler.go +++ b/cmd/xgo/patch_compiler.go @@ -43,6 +43,7 @@ var compilerFiles = []_FilePath{ type2ExprPatch.FilePath, type2AssignmentsPatch.FilePath, syntaxWalkPatch.FilePath, + synatxParserPatch.FilePath, noderWriterPatch.FilePath, noderExprPatch.FilePath, syntaxPrinterPatch.FilePath, diff --git a/cmd/xgo/patch_compiler_ast_type_check.go b/cmd/xgo/patch_compiler_ast_type_check.go index 1c3bdb79..a25684a9 100644 --- a/cmd/xgo/patch_compiler_ast_type_check.go +++ b/cmd/xgo/patch_compiler_ast_type_check.go @@ -217,6 +217,51 @@ var syntaxWalkPatch = &FilePatch{ }, } +var synatxParserPatch = &FilePatch{ + FilePath: _FilePath{"src", "cmd", "compile", "internal", "syntax", "parser.go"}, + Patches: []*Patch{ + // { + // // import + // Mark: "syntax_parser_import_xgo_syntax", + // InsertIndex: 2, + // Anchors: []string{ + // `package syntax`, + // `import (`, + // "\n", + // }, + // Content: `xgo_syntax "cmd/compile/internal/xgo_rewrite_internal/patch/syntax"`, + // }, + { + // NOTE: dependency injection + Mark: "syntax_parser_record_comment_declare", + InsertIndex: 0, + InsertBefore: true, + Anchors: []string{ + `func (p *parser) init(file *PosBase,`, + }, + Content: `var RecordComments func(file *PosBase, line, col uint, comment string)`, + }, + { + Mark: "syntax_parser_record_comments", + InsertIndex: 6, + Anchors: []string{ + `func (p *parser) init(file *PosBase,`, + `p.scanner.init(`, + `func(line`, + `text`, + `:=`, + `commentText(msg)`, + "\n", + }, + Content: ` + if RecordComments != nil { + RecordComments(file,line,col,text) + } + `, + }, + }, +} + var noderWriterPatch = &FilePatch{ FilePath: _FilePath{"src", "cmd", "compile", "internal", "noder", "writer.go"}, Patches: []*Patch{ @@ -302,11 +347,26 @@ func patchCompilerAstTypeCheck(goroot string, goVersion *goinfo.GoVersion) error if err != nil { return err } - if goVersion.Major == 1 && goVersion.Minor < 20 { + // var mock: comments + if false { + // this does not work, because comments are turned off + err = synatxParserPatch.Apply(goroot, goVersion) + if err != nil { + return err + } + } + if goVersion.Major > 1 || goVersion.Minor >= 20 { // only go1.20 and above supports const mock - return nil + err := patchCompilerForConstTrap(goroot, goVersion) + if err != nil { + return err + } } - err = type2ExprPatch.Apply(goroot, goVersion) + return nil +} + +func patchCompilerForConstTrap(goroot string, goVersion *goinfo.GoVersion) error { + err := type2ExprPatch.Apply(goroot, goVersion) if err != nil { return err } diff --git a/cmd/xgo/version.go b/cmd/xgo/version.go index 0ff967ea..91f213eb 100644 --- a/cmd/xgo/version.go +++ b/cmd/xgo/version.go @@ -3,8 +3,8 @@ package main import "fmt" const VERSION = "1.0.24" -const REVISION = "a9cbbd937997b1473a5f70f0131927adcdbf3b79+1" -const NUMBER = 181 +const REVISION = "70a2950e60bbb075af333afa7a702fc02b9d55ed+1" +const NUMBER = 182 func getRevision() string { revSuffix := "" diff --git a/patch/pkgdata/pkgdata.go b/patch/pkgdata/pkgdata.go index f2e701d4..0118170a 100644 --- a/patch/pkgdata/pkgdata.go +++ b/patch/pkgdata/pkgdata.go @@ -11,7 +11,7 @@ import ( ) type PackageData struct { - Vars map[string]bool + Vars map[string]*VarInfo Consts map[string]*ConstInfo Funcs map[string]bool } @@ -20,6 +20,9 @@ type ConstInfo struct { Type string // t=xx Untyped bool // no 'e='(explicit) flag } +type VarInfo struct { + Trap bool // has comment trap +} var pkgDataMapping map[string]*PackageData @@ -56,11 +59,11 @@ func WritePkgData(pkgPath string, pkgData *PackageData) error { if err != nil { return err } - writeSection(w, "[var]", pkgData.Vars) + err = writeVarSection(w, "[var]", pkgData.Vars) if err != nil { return err } - writeSection(w, "[func]", pkgData.Funcs) + err = writeSection(w, "[func]", pkgData.Funcs) if err != nil { return err } @@ -120,6 +123,7 @@ func writeConstSection(w io.Writer, section string, m map[string]*ConstInfo) err } return nil } + func writeConst(w io.Writer, info *ConstInfo) error { if !info.Untyped { _, err := io.WriteString(w, " ") @@ -149,6 +153,49 @@ func writeConst(w io.Writer, info *ConstInfo) error { return nil } +func writeVarSection(w io.Writer, section string, m map[string]*VarInfo) error { + if len(m) == 0 { + return nil + } + _, err := io.WriteString(w, section) + if err != nil { + return err + } + _, err = io.WriteString(w, "\n") + if err != nil { + return err + } + for k, v := range m { + _, err := io.WriteString(w, k) + if err != nil { + return err + } + err = writeVar(w, v) + if err != nil { + return err + } + _, err = io.WriteString(w, "\n") + if err != nil { + return err + } + } + return nil +} +func writeVar(w io.Writer, info *VarInfo) error { + if info.Trap { + _, err := io.WriteString(w, " ") + if err != nil { + return err + } + _, err = io.WriteString(w, "t") + if err != nil { + return err + } + } + + return nil +} + func load(pkgPath string) (*PackageData, error) { if xgo_ctxt.XgoCompilePkgDataDir == "" { return nil, fmt.Errorf("XGO_COMPILE_PKG_DATA_DIR not set") @@ -218,9 +265,17 @@ func parsePkgData(content string) (*PackageData, error) { p.Funcs[name] = true case Section_Var: if p.Vars == nil { - p.Vars = make(map[string]bool, 1) + p.Vars = make(map[string]*VarInfo, 1) + } + varInfo := &VarInfo{} + kvs := strings.Split(extra, " ") + for _, v := range kvs { + if v == "t" { + varInfo.Trap = true + continue + } } - p.Vars[name] = true + p.Vars[name] = varInfo case Section_Const: if p.Consts == nil { p.Consts = make(map[string]*ConstInfo, 1) diff --git a/patch/syntax/syntax.go b/patch/syntax/syntax.go index 91de11a2..bdad73f9 100644 --- a/patch/syntax/syntax.go +++ b/patch/syntax/syntax.go @@ -372,9 +372,13 @@ func (c DeclKind) IsVarOrConst() bool { } type DeclInfo struct { - FuncDecl *syntax.FuncDecl - VarDecl *syntax.VarDecl - ConstDecl *syntax.ConstDecl + FuncDecl *syntax.FuncDecl + VarDecl *syntax.VarDecl + ConstDecl *syntax.ConstDecl + + // is this var decl follow a const __xgo_trap_xxx = 1? + FollowingTrapConst bool + Kind DeclKind Name string RecvTypeName string @@ -464,7 +468,46 @@ func getFuncDecls(files []*syntax.File, varTrap bool) []*DeclInfo { declFuncs = append(declFuncs, fnDecls...) } } - return declFuncs + // compute __xgo_trap_xxx + n := len(declFuncs) + if n == 0 { + return nil + } + j := n + for i := n - 1; i >= 0; i-- { + if i == 0 || !isTrapped(declFuncs, i) { + j-- + declFuncs[j] = declFuncs[i] + continue + } + // a special comment + declFuncs[i].FollowingTrapConst = true + j-- + declFuncs[j] = declFuncs[i] + // remove the comment by skipping next + i-- + } + return declFuncs[j:] +} + +func isTrapped(declFuncs []*DeclInfo, i int) bool { + fn := declFuncs[i] + if fn.Kind != Kind_Var { + return false + } + last := declFuncs[i-1] + if last.Kind != Kind_Const { + return false + } + const xgoTrapPrefix = "__xgo_trap_" + if !strings.HasPrefix(last.Name, xgoTrapPrefix) { + return false + } + subName := last.Name[len(xgoTrapPrefix):] + if !strings.EqualFold(fn.Name, subName) { + return false + } + return true } func filterFuncDecls(funcDecls []*DeclInfo, pkgPath string) []*DeclInfo { diff --git a/patch/syntax/vars.go b/patch/syntax/vars.go index 96852a03..e6441a50 100644 --- a/patch/syntax/vars.go +++ b/patch/syntax/vars.go @@ -35,10 +35,11 @@ func allowPkgVarTrap(pkgPath string) bool { func collectVarDecls(declKind DeclKind, names []*syntax.Name, typ syntax.Expr) []*DeclInfo { var decls []*DeclInfo for _, name := range names { + nameVal := name.Value line := name.Pos().Line() decls = append(decls, &DeclInfo{ Kind: declKind, - Name: name.Value, + Name: nameVal, Line: int(line), }) @@ -48,13 +49,15 @@ func collectVarDecls(declKind DeclKind, names []*syntax.Name, typ syntax.Expr) [ func trapVariables(pkgPath string, fileList []*syntax.File, funcDelcs []*DeclInfo) { names := make(map[string]*DeclInfo, len(funcDelcs)) - varNames := make(map[string]bool) + varNames := make(map[string]*pkgdata.VarInfo) constNames := make(map[string]*pkgdata.ConstInfo) for _, funcDecl := range funcDelcs { identityName := funcDecl.IdentityName() names[identityName] = funcDecl if funcDecl.Kind == Kind_Var || funcDecl.Kind == Kind_VarPtr { - varNames[identityName] = true + varNames[identityName] = &pkgdata.VarInfo{ + Trap: funcDecl.FollowingTrapConst, + } } else if funcDecl.Kind == Kind_Const { constDecl := funcDecl.ConstDecl constInfo := &pkgdata.ConstInfo{Untyped: true} @@ -141,6 +144,8 @@ type BlockContext struct { RHSAssignNoDefParent map[syntax.Node]*syntax.AssignStmt CaseClauseParent map[syntax.Node]*syntax.CaseClause ReturnStmtParent map[syntax.Node]*syntax.ReturnStmt + ParenParent map[syntax.Node]*syntax.ParenExpr + SelectorParent map[syntax.Node]*syntax.SelectorExpr // const info ConstInfo map[syntax.Node]*ConstInfo @@ -369,6 +374,25 @@ func (ctx *BlockContext) traverseCommonClause(node *syntax.CommClause, globaleNa return node } +func (c *BlockContext) recordParen(paren *syntax.ParenExpr) { + if paren == nil || paren.X == nil { + return + } + if c.ParenParent == nil { + c.ParenParent = make(map[syntax.Node]*syntax.ParenExpr, 1) + } + c.ParenParent[paren.X] = paren +} +func (c *BlockContext) recordSelectorExpr(sel *syntax.SelectorExpr) { + if sel == nil || sel.X == nil { + return + } + if c.SelectorParent == nil { + c.SelectorParent = make(map[syntax.Node]*syntax.SelectorExpr, 1) + } + c.SelectorParent[sel.X] = sel +} + func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string]*DeclInfo, imports map[string]string) syntax.Expr { if node == nil { return nil @@ -396,6 +420,7 @@ func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string] node.Body = funcCtx.traverseBlockStmt(node.Body, globaleNames, imports) return node case *syntax.ParenExpr: + ctx.recordParen(node) node.X = ctx.traverseExpr(node.X, globaleNames, imports) if xgoConv, ok := node.X.(*syntax.XgoSimpleConvert); ok { constType := getConstType(xgoConv) @@ -409,11 +434,12 @@ func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string] return newNode } case *syntax.SelectorExpr: - newNode, selIsName := ctx.trapSelector(node, node, false, globaleNames, imports) + ctx.recordSelectorExpr(node) + newNode, xIsName := ctx.trapSelector(node, node, false, globaleNames, imports) if newNode != nil { return newNode } - if !selIsName { + if !xIsName { node.X = ctx.traverseExpr(node.X, globaleNames, imports) } case *syntax.IndexExpr: @@ -439,11 +465,11 @@ func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string] case *syntax.Name: return ctx.trapAddrNode(node, x, globaleNames) case *syntax.SelectorExpr: - newNode, selIsName := ctx.trapSelector(node, x, true, globaleNames, imports) + newNode, xIsName := ctx.trapSelector(node, x, true, globaleNames, imports) if newNode != nil { return newNode } - if selIsName { + if xIsName { return node } } @@ -596,6 +622,9 @@ func (c *BlockContext) trapValueNode(node *syntax.Name, globaleNames map[string] var isCallArg bool var untypedConstType string if decl.Kind == Kind_Var || decl.Kind == Kind_VarPtr { + if !decl.FollowingTrapConst && !c.isVarOKToTrap(node) { + return node + } // good to go } else if decl.Kind == Kind_Const { // untyped const(most cases) should only be used in @@ -654,7 +683,7 @@ func (c *BlockContext) trapValueNode(node *syntax.Name, globaleNames map[string] return newName } -func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr, takeAddr bool, globaleNames map[string]*DeclInfo, imports map[string]string) (newExpr syntax.Expr, selIsName bool) { +func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr, takeAddr bool, globaleNames map[string]*DeclInfo, imports map[string]string) (newExpr syntax.Expr, xIsName bool) { // form: pkg.var nameNode, ok := sel.X.(*syntax.Name) if !ok { @@ -700,8 +729,10 @@ func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr } } - } else if pkgData.Vars[sel.Sel.Value] { - // ok to go + } else if varInfo, ok := pkgData.Vars[sel.Sel.Value]; ok { + if !varInfo.Trap && !takeAddr && !ctx.isVarOKToTrap(node) { + return nil, true + } } else { return nil, true } @@ -724,6 +755,20 @@ func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr return newName, true } +func (ctx *BlockContext) isVarOKToTrap(node syntax.Node) bool { + // a variable can only trapped when it will not + // cause an implicit pointer + _, ok := ctx.SelectorParent[node] + if ok { + return false + } + paren, ok := ctx.ParenParent[node] + if ok { + return ctx.isVarOKToTrap(paren) + } + return true +} + func (ctx *BlockContext) isConstOKToTrap(node syntax.Node) (explicitType syntax.Expr, ok bool) { if true { return nil, true diff --git a/runtime/core/version.go b/runtime/core/version.go index 55d37bd0..7e56e932 100644 --- a/runtime/core/version.go +++ b/runtime/core/version.go @@ -7,8 +7,8 @@ import ( ) const VERSION = "1.0.24" -const REVISION = "a9cbbd937997b1473a5f70f0131927adcdbf3b79+1" -const NUMBER = 181 +const REVISION = "70a2950e60bbb075af333afa7a702fc02b9d55ed+1" +const NUMBER = 182 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/runtime/test/debug/debug_test.go b/runtime/test/debug/debug_test.go index c4aad11b..d5509215 100644 --- a/runtime/test/debug/debug_test.go +++ b/runtime/test/debug/debug_test.go @@ -5,36 +5,6 @@ package debug -import ( - "context" - "os" - "testing" - "time" +const __xgo_trap_cfg2 = 1 - "github.com/xhd2015/xgo/runtime/core" - "github.com/xhd2015/xgo/runtime/trap" -) - -func TestTimeNowNestedLevel2AllowNested(t *testing.T) { - i := 0 - trap.AddInterceptor(&trap.Interceptor{ - Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { - // t.Logf("%s.%s", f.Pkg, f.IdentityName) - i++ - if i > 20 { - os.Exit(1) - } - getTime2() - return - }, - }) - time.Now() -} - -func getTime2() time.Time { - return getTime3() -} - -func getTime3() time.Time { - return time.Now() -} +var cfg2 int diff --git a/runtime/test/mock_var/mock_config_should_copy_test.go b/runtime/test/mock_var/mock_config_should_copy_test.go new file mode 100644 index 00000000..1c38397c --- /dev/null +++ b/runtime/test/mock_var/mock_config_should_copy_test.go @@ -0,0 +1,48 @@ +package mock_var + +import ( + "bytes" + "context" + "fmt" + "testing" + + "github.com/xhd2015/xgo/runtime/core" + "github.com/xhd2015/xgo/runtime/trap" +) + +type Config struct { + On bool +} + +var cfg1 Config + +const __xgo_trap_cfg2 = 1 + +var cfg2 Config + +func TestConfigAllowIntercept(t *testing.T) { + var buf bytes.Buffer + trap.AddInterceptor(&trap.Interceptor{ + Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + buf.WriteString(fmt.Sprintf("%s\n", f.IdentityName)) + return + }, + }) + readConfig1() + readConfig2() + + trapStr := buf.String() + // notice there is no lock + expectTrapStr := "readConfig1\nreadConfig2\ncfg2\n" + if trapStr != expectTrapStr { + t.Fatalf("expect tarp buf: %q, actual: %q", expectTrapStr, trapStr) + } +} + +func readConfig1() bool { + return cfg1.On +} + +func readConfig2() bool { + return cfg2.On +} diff --git a/runtime/test/mock_var/mock_no_value_copy_test.go b/runtime/test/mock_var/mock_no_value_copy_test.go new file mode 100644 index 00000000..dfc2c012 --- /dev/null +++ b/runtime/test/mock_var/mock_no_value_copy_test.go @@ -0,0 +1,112 @@ +package mock_var + +import ( + "bytes" + "context" + "fmt" + "sync" + "testing" + + "github.com/xhd2015/xgo/runtime/core" + "github.com/xhd2015/xgo/runtime/trap" +) + +var lock sync.Mutex +var count int + +func TestMockLockShouldNotCopy(t *testing.T) { + var buf bytes.Buffer + cancel := trap.AddInterceptor(&trap.Interceptor{ + Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + buf.WriteString(fmt.Sprintf("%s\n", f.IdentityName)) + return + }, + }) + incrLocked() + cancel() + if count != 1 { + t.Fatalf("expect count to be 1,actual: %d", count) + } + trapStr := buf.String() + + // notice there is no lock + expectTrapStr := "incrLocked\ncount\n" + if trapStr != expectTrapStr { + t.Fatalf("expect tarp buf: %q, actual: %q", expectTrapStr, trapStr) + } +} + +func incrLocked() { + lock.Lock() + count = count + 1 + lock.Unlock() +} + +// NOTE: this test case demonstrates a buggy case: +// +// f := lock.Lock, so lock should not be intercepted +func TestMockLockFuncShouldNotBeMistakenlyCopied(t *testing.T) { + var buf bytes.Buffer + cancel := trap.AddInterceptor(&trap.Interceptor{ + Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + buf.WriteString(fmt.Sprintf("%s\n", f.IdentityName)) + return + }, + }) + incrLocked_Fn() + cancel() + if countFn != 1 { + t.Fatalf("expect countFn to be 1,actual: %d", countFn) + } + trapStr := buf.String() + + // notice there is no lock + expectTrapStr := "incrLocked_Fn\ncountFn\n" + if trapStr != expectTrapStr { + t.Fatalf("expect tarp buf: %q, actual: %q", expectTrapStr, trapStr) + } +} + +var countFn int + +// NOTE: if lock is captured, this function will +// panic: fatal error: sync: unlock of unlocked mutex +func incrLocked_Fn() { + f := lock.Lock + f() + countFn = countFn + 1 + lock.Unlock() +} + +func TestMockLockFuncParenShouldNotBeMistakenlyCopied(t *testing.T) { + var buf bytes.Buffer + cancel := trap.AddInterceptor(&trap.Interceptor{ + Pre: func(ctx context.Context, f *core.FuncInfo, args, result core.Object) (data interface{}, err error) { + buf.WriteString(fmt.Sprintf("%s\n", f.IdentityName)) + return + }, + }) + incrLocked_Paren() + cancel() + if countParen != 1 { + t.Fatalf("expect countParen to be 1,actual: %d", countParen) + } + trapStr := buf.String() + + // notice there is no lock + expectTrapStr := "incrLocked_Paren\ncountParen\n" + if trapStr != expectTrapStr { + t.Fatalf("expect tarp buf: %q, actual: %q", expectTrapStr, trapStr) + } +} + +var countParen int + +// NOTE: if lock is captured, this function will +// panic: fatal error: sync: unlock of unlocked mutex +func incrLocked_Paren() { + f := (lock).Lock + f() + countParen = countParen + 1 + lock.Unlock() +} diff --git a/runtime/test/mock_var/mock_var_test.go b/runtime/test/mock_var/mock_var_test.go index b5dd2d83..6a024b06 100644 --- a/runtime/test/mock_var/mock_var_test.go +++ b/runtime/test/mock_var/mock_var_test.go @@ -1,4 +1,4 @@ -package main +package mock_var import ( "context" diff --git a/test/xgo_test/func_names/generic_names_go1.18_19_test.go b/test/xgo_test/func_names/generic_names_go1.18_19_test.go index f22bc27d..5437012f 100644 --- a/test/xgo_test/func_names/generic_names_go1.18_19_test.go +++ b/test/xgo_test/func_names/generic_names_go1.18_19_test.go @@ -23,11 +23,12 @@ func init() { var gi GI[int] = &gs getGenericTests = func() []*testCase { return []*testCase{ - {GF[int], "github.com/xhd2015/xgo/test/xgo_test/func_names.init.0.func1.1"}, - {(*GS[int]).F, "github.com/xhd2015/xgo/test/xgo_test/func_names.init.0.func1.2"}, + // init.2 because helper file takes some space + {GF[int], "github.com/xhd2015/xgo/test/xgo_test/func_names.init.2.func1.1"}, + {(*GS[int]).F, "github.com/xhd2015/xgo/test/xgo_test/func_names.init.2.func1.2"}, // it seems that with go1.18,go1.19 ,gs.F does not end with -fm suffix // that may cause generic mock failed. - {gs.F, "github.com/xhd2015/xgo/test/xgo_test/func_names.init.0.func1.3"}, + {gs.F, "github.com/xhd2015/xgo/test/xgo_test/func_names.init.2.func1.3"}, {GI[int].F, "github.com/xhd2015/xgo/test/xgo_test/func_names.GI[...].F"}, {gi.F, "github.com/xhd2015/xgo/test/xgo_test/func_names.GI[...].F-fm"}, }