From 9e902f0f3a606e1a09369e0ce8a31e8cac49c605 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 20 Oct 2015 10:00:07 -0700 Subject: [PATCH] cmd/compile: generalize racewalk to instrument (naming change) This is mechanical change that is a step toward reusing the racewalk pass for a more general instrumentation pass. The first use will be to add support for the memory sanitizer. Change-Id: I75b93b814ac60c1db1660e0b9a9a7d7977d86939 Reviewed-on: https://go-review.googlesource.com/16105 Reviewed-by: Hyang-Ah Hana Kim Reviewed-by: David Crawshaw --- src/cmd/compile/internal/gc/go.go | 4 + src/cmd/compile/internal/gc/inl.go | 8 +- src/cmd/compile/internal/gc/lex.go | 1 + src/cmd/compile/internal/gc/order.go | 6 +- src/cmd/compile/internal/gc/pgen.go | 4 +- src/cmd/compile/internal/gc/racewalk.go | 116 ++++++++++++------------ src/cmd/compile/internal/gc/range.go | 2 +- src/cmd/compile/internal/gc/subr.go | 4 +- src/cmd/compile/internal/gc/walk.go | 20 ++-- 9 files changed, 86 insertions(+), 79 deletions(-) diff --git a/src/cmd/compile/internal/gc/go.go b/src/cmd/compile/internal/gc/go.go index f250c95fb9..79b9d9f692 100644 --- a/src/cmd/compile/internal/gc/go.go +++ b/src/cmd/compile/internal/gc/go.go @@ -647,6 +647,10 @@ var flag_race int var flag_largemodel int +// Whether we are adding any sort of code instrumentation, such as +// when the race detector is enabled. +var instrumenting bool + // Pending annotations for next func declaration. var ( noescape bool diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 1f9b473f92..cb165f48f2 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -124,13 +124,13 @@ func caninl(fn *Node) { } } - // Runtime package must not be race instrumented. - // Racewalk skips runtime package. However, some runtime code can be + // Runtime package must not be instrumented. + // Instrument skips runtime package. However, some runtime code can be // inlined into other packages and instrumented there. To avoid this, - // we disable inlining of runtime functions in race mode. + // we disable inlining of runtime functions when instrumenting. // The example that we observed is inlining of LockOSThread, // which lead to false race reports on m contents. - if flag_race != 0 && myimportpath == "runtime" { + if instrumenting && myimportpath == "runtime" { return } diff --git a/src/cmd/compile/internal/gc/lex.go b/src/cmd/compile/internal/gc/lex.go index 81198f37bf..b9ce4cb010 100644 --- a/src/cmd/compile/internal/gc/lex.go +++ b/src/cmd/compile/internal/gc/lex.go @@ -249,6 +249,7 @@ func Main() { if flag_race != 0 { racepkg = mkpkg("runtime/race") racepkg.Name = "race" + instrumenting = true } // parse -d argument diff --git a/src/cmd/compile/internal/gc/order.go b/src/cmd/compile/internal/gc/order.go index c783d641a5..f996ce2aa0 100644 --- a/src/cmd/compile/internal/gc/order.go +++ b/src/cmd/compile/internal/gc/order.go @@ -434,7 +434,7 @@ func ordermapassign(n *Node, order *Order) { a = Nod(OAS, m, l.N) typecheck(&a, Etop) post = list(post, a) - } else if flag_race != 0 && n.Op == OAS2FUNC && !isblank(l.N) { + } else if instrumenting && n.Op == OAS2FUNC && !isblank(l.N) { m = l.N l.N = ordertemp(m.Type, order, false) a = Nod(OAS, m, l.N) @@ -1093,7 +1093,7 @@ func orderexpr(np **Node, order *Order, lhs *Node) { OREAL, ORECOVER: ordercall(n, order) - if lhs == nil || lhs.Op != ONAME || flag_race != 0 { + if lhs == nil || lhs.Op != ONAME || instrumenting { n = ordercopyexpr(n, n.Type, order, 0) } @@ -1153,7 +1153,7 @@ func orderexpr(np **Node, order *Order, lhs *Node) { // TODO(rsc): The Isfat is for consistency with componentgen and walkexpr. // It needs to be removed in all three places. // That would allow inlining x.(struct{*int}) the same as x.(*int). - if !isdirectiface(n.Type) || Isfat(n.Type) || flag_race != 0 { + if !isdirectiface(n.Type) || Isfat(n.Type) || instrumenting { n = ordercopyexpr(n, n.Type, order, 1) } diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index cd8e66c11a..015d2fd47e 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -399,8 +399,8 @@ func compile(fn *Node) { if nerrors != 0 { goto ret } - if flag_race != 0 { - racewalk(Curfn) + if instrumenting { + instrument(Curfn) } if nerrors != 0 { goto ret diff --git a/src/cmd/compile/internal/gc/racewalk.go b/src/cmd/compile/internal/gc/racewalk.go index 5d10fc1e9a..a2b09cdf28 100644 --- a/src/cmd/compile/internal/gc/racewalk.go +++ b/src/cmd/compile/internal/gc/racewalk.go @@ -9,7 +9,9 @@ import ( "strings" ) -// The racewalk pass modifies the code tree for the function as follows: +// The instrument pass modifies the code tree for instrumentation. +// +// For flag_race it modifies the function as follows: // // 1. It inserts a call to racefuncenter at the beginning of each function. // 2. It inserts a call to racefuncexit at the end of each function. @@ -42,16 +44,16 @@ func ispkgin(pkgs []string) bool { return false } -func racewalk(fn *Node) { +func instrument(fn *Node) { if ispkgin(omit_pkgs) || fn.Func.Norace { return } if !ispkgin(noinst_pkgs) { - racewalklist(fn.Nbody, nil) + instrumentlist(fn.Nbody, nil) // nothing interesting for race detector in fn->enter - racewalklist(fn.Func.Exit, nil) + instrumentlist(fn.Func.Exit, nil) } // nodpc is the PC of the caller as extracted by @@ -68,7 +70,7 @@ func racewalk(fn *Node) { fn.Func.Exit = list(fn.Func.Exit, nd) if Debug['W'] != 0 { - s := fmt.Sprintf("after racewalk %v", fn.Func.Nname.Sym) + s := fmt.Sprintf("after instrument %v", fn.Func.Nname.Sym) dumplist(s, fn.Nbody) s = fmt.Sprintf("enter %v", fn.Func.Nname.Sym) dumplist(s, fn.Func.Enter) @@ -77,12 +79,12 @@ func racewalk(fn *Node) { } } -func racewalklist(l *NodeList, init **NodeList) { +func instrumentlist(l *NodeList, init **NodeList) { var instr *NodeList for ; l != nil; l = l.Next { instr = nil - racewalknode(&l.N, &instr, 0, 0) + instrumentnode(&l.N, &instr, 0, 0) if init == nil { l.N.Ninit = concat(l.N.Ninit, instr) } else { @@ -94,7 +96,7 @@ func racewalklist(l *NodeList, init **NodeList) { // walkexpr and walkstmt combined // walks the tree and adds calls to the // instrumentation code to top-level (statement) nodes' init -func racewalknode(np **Node, init **NodeList, wr int, skip int) { +func instrumentnode(np **Node, init **NodeList, wr int, skip int) { n := *np if n == nil { @@ -102,35 +104,35 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { } if Debug['w'] > 1 { - Dump("racewalk-before", n) + Dump("instrument-before", n) } setlineno(n) if init == nil { - Fatalf("racewalk: bad init list") + Fatalf("instrument: bad init list") } if init == &n.Ninit { // If init == &n->ninit and n->ninit is non-nil, - // racewalknode might append it to itself. + // instrumentnode might append it to itself. // nil it out and handle it separately before putting it back. l := n.Ninit n.Ninit = nil - racewalklist(l, nil) - racewalknode(&n, &l, wr, skip) // recurse with nil n->ninit + instrumentlist(l, nil) + instrumentnode(&n, &l, wr, skip) // recurse with nil n->ninit appendinit(&n, l) *np = n return } - racewalklist(n.Ninit, nil) + instrumentlist(n.Ninit, nil) switch n.Op { default: - Fatalf("racewalk: unknown node type %v", Oconv(int(n.Op), 0)) + Fatalf("instrument: unknown node type %v", Oconv(int(n.Op), 0)) case OAS, OASWB, OAS2FUNC: - racewalknode(&n.Left, init, 1, 0) - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Left, init, 1, 0) + instrumentnode(&n.Right, init, 0, 0) goto ret // can't matter @@ -142,7 +144,7 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { for l := n.List; l != nil; l = l.Next { switch l.N.Op { case OCALLFUNC, OCALLMETH, OCALLINTER: - racewalknode(&l.N, &out, 0, 0) + instrumentnode(&l.N, &out, 0, 0) out = list(out, l.N) // Scan past OAS nodes copying results off stack. // Those must not be instrumented, because the @@ -154,7 +156,7 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { out = list(out, l.N) } default: - racewalknode(&l.N, &out, 0, 0) + instrumentnode(&l.N, &out, 0, 0) out = list(out, l.N) } } @@ -162,22 +164,22 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { goto ret case ODEFER: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret case OPROC: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret case OCALLINTER: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret // Instrument dst argument of runtime.writebarrier* calls // as we do not instrument runtime code. // typedslicecopy is instrumented in runtime. case OCALLFUNC: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret case ONOT, @@ -187,32 +189,32 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { OIMAG, OCOM, OSQRT: - racewalknode(&n.Left, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) goto ret case ODOTINTER: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret case ODOT: - racewalknode(&n.Left, init, 0, 1) + instrumentnode(&n.Left, init, 0, 1) callinstr(&n, init, wr, skip) goto ret case ODOTPTR: // dst = (*x).f with implicit *; otherwise it's ODOT+OIND - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) callinstr(&n, init, wr, skip) goto ret case OIND: // *p - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) callinstr(&n, init, wr, skip) goto ret case OSPTR, OLEN, OCAP: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) if Istype(n.Left.Type, TMAP) { n1 := Nod(OCONVNOP, n.Left, nil) n1.Type = Ptrto(Types[TUINT8]) @@ -241,18 +243,18 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { OGT, OADD, OCOMPLEX: - racewalknode(&n.Left, init, wr, 0) - racewalknode(&n.Right, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) + instrumentnode(&n.Right, init, wr, 0) goto ret case OANDAND, OOROR: - racewalknode(&n.Left, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) // walk has ensured the node has moved to a location where // side effects are safe. // n->right may not be executed, // so instrumentation goes to n->right->ninit, not init. - racewalknode(&n.Right, &n.Right.Ninit, wr, 0) + instrumentnode(&n.Right, &n.Right.Ninit, wr, 0) goto ret @@ -261,57 +263,57 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { goto ret case OCONV: - racewalknode(&n.Left, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) goto ret case OCONVNOP: - racewalknode(&n.Left, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) goto ret case ODIV, OMOD: - racewalknode(&n.Left, init, wr, 0) - racewalknode(&n.Right, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) + instrumentnode(&n.Right, init, wr, 0) goto ret case OINDEX: if !Isfixedarray(n.Left.Type) { - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) } else if !islvalue(n.Left) { // index of unaddressable array, like Map[k][i]. - racewalknode(&n.Left, init, wr, 0) + instrumentnode(&n.Left, init, wr, 0) - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Right, init, 0, 0) goto ret } - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Right, init, 0, 0) if n.Left.Type.Etype != TSTRING { callinstr(&n, init, wr, skip) } goto ret case OSLICE, OSLICEARR, OSLICE3, OSLICE3ARR, OSLICESTR: - racewalknode(&n.Left, init, 0, 0) - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) + instrumentnode(&n.Right, init, 0, 0) goto ret case OKEY: - racewalknode(&n.Left, init, 0, 0) - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) + instrumentnode(&n.Right, init, 0, 0) goto ret case OADDR: - racewalknode(&n.Left, init, 0, 1) + instrumentnode(&n.Left, init, 0, 1) goto ret // n->left is Type* which is not interesting. case OEFACE: - racewalknode(&n.Right, init, 0, 0) + instrumentnode(&n.Right, init, 0, 0) goto ret case OITAB: - racewalknode(&n.Left, init, 0, 0) + instrumentnode(&n.Left, init, 0, 0) goto ret // should not appear in AST by now @@ -355,31 +357,31 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { OAS2RECV, OAS2MAPR, OASOP: - Yyerror("racewalk: %v must be lowered by now", Oconv(int(n.Op), 0)) + Yyerror("instrument: %v must be lowered by now", Oconv(int(n.Op), 0)) goto ret // impossible nodes: only appear in backend. case ORROTC, OEXTEND: - Yyerror("racewalk: %v cannot exist now", Oconv(int(n.Op), 0)) + Yyerror("instrument: %v cannot exist now", Oconv(int(n.Op), 0)) goto ret case OGETG: - Yyerror("racewalk: OGETG can happen only in runtime which we don't instrument") + Yyerror("instrument: OGETG can happen only in runtime which we don't instrument") goto ret case OFOR: if n.Left != nil { - racewalknode(&n.Left, &n.Left.Ninit, 0, 0) + instrumentnode(&n.Left, &n.Left.Ninit, 0, 0) } if n.Right != nil { - racewalknode(&n.Right, &n.Right.Ninit, 0, 0) + instrumentnode(&n.Right, &n.Right.Ninit, 0, 0) } goto ret case OIF, OSWITCH: if n.Left != nil { - racewalknode(&n.Left, &n.Left.Ninit, 0, 0) + instrumentnode(&n.Left, &n.Left.Ninit, 0, 0) } goto ret @@ -416,10 +418,10 @@ func racewalknode(np **Node, init **NodeList, wr int, skip int) { ret: if n.Op != OBLOCK { // OBLOCK is handled above in a special way. - racewalklist(n.List, init) + instrumentlist(n.List, init) } - racewalklist(n.Nbody, nil) - racewalklist(n.Rlist, nil) + instrumentlist(n.Nbody, nil) + instrumentlist(n.Rlist, nil) *np = n } diff --git a/src/cmd/compile/internal/gc/range.go b/src/cmd/compile/internal/gc/range.go index 59f7d40c6e..0beee4e492 100644 --- a/src/cmd/compile/internal/gc/range.go +++ b/src/cmd/compile/internal/gc/range.go @@ -340,7 +340,7 @@ func walkrange(n *Node) { // // Parameters are as in walkrange: "for v1, v2 = range a". func memclrrange(n, v1, v2, a *Node) bool { - if Debug['N'] != 0 || flag_race != 0 { + if Debug['N'] != 0 || instrumenting { return false } if v1 == nil || v2 != nil { diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index df6b6f662e..c73c675884 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -1645,7 +1645,7 @@ func ullmancalc(n *Node) { // hard with race detector case OANDAND, OOROR: - if flag_race != 0 { + if instrumenting { ul = UINF goto out } @@ -2405,7 +2405,7 @@ func genwrapper(rcvr *Type, method *Type, newnam *Sym, iface int) { dot := adddot(Nod(OXDOT, this.Left, newname(method.Sym))) // generate call - if flag_race == 0 && Isptr[rcvr.Etype] && Isptr[methodrcvr.Etype] && method.Embedded != 0 && !isifacemethod(method.Type) { + if !instrumenting && Isptr[rcvr.Etype] && Isptr[methodrcvr.Etype] && method.Embedded != 0 && !isifacemethod(method.Type) { // generate tail call: adjust pointer receiver and jump to embedded method. dot = dot.Left // skip final .M if !Isptr[dotlist[0].field.Type.Etype] { diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 2afa05c66a..f0a1ddc6e4 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -237,7 +237,7 @@ func walkstmt(np **Node) { walkprintfunc(&n.Left, &n.Ninit) case OCOPY: - n.Left = copyany(n.Left, &n.Ninit, 1) + n.Left = copyany(n.Left, &n.Ninit, true) default: walkexpr(&n.Left, &n.Ninit) @@ -269,7 +269,7 @@ func walkstmt(np **Node) { walkprintfunc(&n.Left, &n.Ninit) case OCOPY: - n.Left = copyany(n.Left, &n.Ninit, 1) + n.Left = copyany(n.Left, &n.Ninit, true) default: walkexpr(&n.Left, &n.Ninit) @@ -678,7 +678,7 @@ func walkexpr(np **Node, init **NodeList) { goto ret } - if n.Right == nil || iszero(n.Right) && flag_race == 0 { + if n.Right == nil || iszero(n.Right) && !instrumenting { goto ret } @@ -690,7 +690,7 @@ func walkexpr(np **Node, init **NodeList) { // TODO(rsc): The Isfat is for consistency with componentgen and orderexpr. // It needs to be removed in all three places. // That would allow inlining x.(struct{*int}) the same as x.(*int). - if isdirectiface(n.Right.Type) && !Isfat(n.Right.Type) && flag_race == 0 { + if isdirectiface(n.Right.Type) && !Isfat(n.Right.Type) && !instrumenting { // handled directly during cgen walkexpr(&n.Right, init) break @@ -895,7 +895,7 @@ func walkexpr(np **Node, init **NodeList) { // TODO(rsc): The Isfat is for consistency with componentgen and orderexpr. // It needs to be removed in all three places. // That would allow inlining x.(struct{*int}) the same as x.(*int). - if isdirectiface(e.Type) && !Isfat(e.Type) && flag_race == 0 { + if isdirectiface(e.Type) && !Isfat(e.Type) && !instrumenting { // handled directly during gen. walkexprlistsafe(n.List, init) walkexpr(&e.Left, init) @@ -1412,7 +1412,7 @@ func walkexpr(np **Node, init **NodeList) { Fatalf("append outside assignment") case OCOPY: - n = copyany(n, init, flag_race) + n = copyany(n, init, instrumenting) goto ret // cannot use chanfn - closechan takes any, not chan any @@ -2938,7 +2938,7 @@ func appendslice(n *Node, init **NodeList) *Node { substArgTypes(fn, l1.Type, l2.Type) nt := mkcall1(fn, Types[TINT], &l, typename(l1.Type.Type), nptr1, nptr2) l = list(l, nt) - } else if flag_race != 0 { + } else if instrumenting { // rely on runtime to instrument copy. // copy(s[len(l1):len(l1)+len(l2)], l2) nptr1 := Nod(OSLICE, s, Nod(OKEY, Nod(OLEN, l1, nil), Nod(OADD, Nod(OLEN, l1, nil), Nod(OLEN, l2, nil)))) @@ -3038,7 +3038,7 @@ func walkappend(n *Node, init **NodeList, dst *Node) *Node { // General case, with no function calls left as arguments. // Leave for gen, except that race detector requires old form - if flag_race == 0 { + if !instrumenting { return n } @@ -3091,13 +3091,13 @@ func walkappend(n *Node, init **NodeList, dst *Node) *Node { // // Also works if b is a string. // -func copyany(n *Node, init **NodeList, runtimecall int) *Node { +func copyany(n *Node, init **NodeList, runtimecall bool) *Node { if haspointers(n.Left.Type.Type) { fn := writebarrierfn("typedslicecopy", n.Left.Type, n.Right.Type) return mkcall1(fn, n.Type, init, typename(n.Left.Type.Type), n.Left, n.Right) } - if runtimecall != 0 { + if runtimecall { var fn *Node if n.Right.Type.Etype == TSTRING { fn = syslook("slicestringcopy", 1) -- 2.50.0