cmd/compile: use getcallerpc for racefuncentry

cherrymui · cherrymui · commit a22bd3dc73bf · 2021-03-05T22:42:10.000Z
Currently, when instrumenting for the race detector, the compiler inserts racefuncentry/racefuncentryfp at the entry of instrumented functions. racefuncentry takes the caller's PC. On AMD64, we synthesize a node which points to -8(FP) which is where the return address is stored. Later this node turns to a special Arg in SSA that is not really an argument. This causes problems in the new ABI work so that special node has to be special-cased. This CL changes the special node to a call to getcallerpc, which lowers to an intrinsic in SSA. This also unifies AMD64 code path and LR machine code path, as getcallerpc works on all platforms. Change-Id: I1377e140b91e0473cfcadfda221f26870c1b124d Reviewed-on: https://go-review.googlesource.com/c/go/+/297929 Trust: Cherry Zhang <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
diff --git a/src/cmd/compile/internal/base/base.go b/src/cmd/compile/internal/base/base.go
@@ -70,6 +70,6 @@ var NoInstrumentPkgs = []string{
 	"internal/cpu",
 }
 
-// Don't insert racefuncenterfp/racefuncexit into the following packages.
+// Don't insert racefuncenter/racefuncexit into the following packages.
 // Memory accesses in the packages are either uninteresting or will cause false positives.
 var NoRacePkgs = []string{"sync", "sync/atomic"}
diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go
@@ -1612,18 +1612,18 @@ func needRaceCleanup(sym *AuxCall, v *Value) bool {
 	if !f.Config.Race {
 		return false
 	}
-	if !isSameCall(sym, "runtime.racefuncenter") && !isSameCall(sym, "runtime.racefuncenterfp") && !isSameCall(sym, "runtime.racefuncexit") {
+	if !isSameCall(sym, "runtime.racefuncenter") && !isSameCall(sym, "runtime.racefuncexit") {
 		return false
 	}
 	for _, b := range f.Blocks {
 		for _, v := range b.Values {
 			switch v.Op {
 			case OpStaticCall, OpStaticLECall:
-				// Check for racefuncenter/racefuncenterfp will encounter racefuncexit and vice versa.
+				// Check for racefuncenter will encounter racefuncexit and vice versa.
 				// Allow calls to panic*
 				s := v.Aux.(*AuxCall).Fn.String()
 				switch s {
-				case "runtime.racefuncenter", "runtime.racefuncenterfp", "runtime.racefuncexit",
+				case "runtime.racefuncenter", "runtime.racefuncexit",
 					"runtime.panicdivide", "runtime.panicwrap",
 					"runtime.panicshift":
 					continue
diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
@@ -4564,6 +4564,9 @@ func findIntrinsic(sym *types.Sym) intrinsicBuilder {
 	if sym.Pkg == types.LocalPkg {
 		pkg = base.Ctxt.Pkgpath
 	}
+	if sym.Pkg == ir.Pkgs.Runtime {
+		pkg = "runtime"
+	}
 	if base.Flag.Race && pkg == "sync/atomic" {
 		// The race detector needs to be able to intercept these calls.
 		// We can't intrinsify them.
diff --git a/src/cmd/compile/internal/typecheck/builtin.go b/src/cmd/compile/internal/typecheck/builtin.go
diff --git a/src/cmd/compile/internal/typecheck/builtin/runtime.go b/src/cmd/compile/internal/typecheck/builtin/runtime.go
@@ -6,6 +6,7 @@
 // to update builtin.go. This is not done automatically
 // to avoid depending on having a working compiler binary.
 
+//go:build ignore
 // +build ignore
 
 package runtime
@@ -224,9 +225,10 @@ func uint32tofloat64(uint32) float64
 
 func complex128div(num complex128, den complex128) (quo complex128)
 
+func getcallerpc() uintptr
+
 // race detection
 func racefuncenter(uintptr)
-func racefuncenterfp()
 func racefuncexit()
 func raceread(uintptr)
 func racewrite(uintptr)
diff --git a/src/cmd/compile/internal/walk/race.go b/src/cmd/compile/internal/walk/race.go
@@ -7,11 +7,8 @@ package walk
 import (
 	"cmd/compile/internal/base"
 	"cmd/compile/internal/ir"
-	"cmd/compile/internal/ssagen"
-	"cmd/compile/internal/typecheck"
 	"cmd/compile/internal/types"
 	"cmd/internal/src"
-	"cmd/internal/sys"
 )
 
 func instrument(fn *ir.Func) {
@@ -26,26 +23,12 @@ func instrument(fn *ir.Func) {
 	if base.Flag.Race {
 		lno := base.Pos
 		base.Pos = src.NoXPos
-		if ssagen.Arch.LinkArch.Arch.Family != sys.AMD64 {
-			fn.Enter.Prepend(mkcallstmt("racefuncenterfp"))
-			fn.Exit.Append(mkcallstmt("racefuncexit"))
-		} else {
-
-			// nodpc is the PC of the caller as extracted by
-			// getcallerpc. We use -widthptr(FP) for x86.
-			// This only works for amd64. This will not
-			// work on arm or others that might support
-			// race in the future.
-
-			nodpc := ir.NewNameAt(src.NoXPos, typecheck.Lookup(".fp"))
-			nodpc.Class = ir.PPARAM
-			nodpc.SetUsed(true)
-			nodpc.SetType(types.Types[types.TUINTPTR])
-			nodpc.SetFrameOffset(int64(-types.PtrSize))
-			fn.Dcl = append(fn.Dcl, nodpc)
-			fn.Enter.Prepend(mkcallstmt("racefuncenter", nodpc))
-			fn.Exit.Append(mkcallstmt("racefuncexit"))
+		var init ir.Nodes
+		fn.Enter.Prepend(mkcallstmt("racefuncenter", mkcall("getcallerpc", types.Types[types.TUINTPTR], &init)))
+		if len(init) != 0 {
+			base.Fatalf("race walk: unexpected init for getcallerpc")
 		}
+		fn.Exit.Append(mkcallstmt("racefuncexit"))
 		base.Pos = lno
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,6 @@ var NoInstrumentPkgs = []string{`
`70`	`70`	`"internal/cpu",`
`71`	`71`	`}`
`72`	`72`
`73`		`-// Don't insert racefuncenterfp/racefuncexit into the following packages.`
	`73`	`+// Don't insert racefuncenter/racefuncexit into the following packages.`
`74`	`74`	`// Memory accesses in the packages are either uninteresting or will cause false positives.`
`75`	`75`	`var NoRacePkgs = []string{"sync", "sync/atomic"}`