Skip to content

Commit

Permalink
cmd/compile: only escape unsafe.Pointer conversions when -d=checkptr=2
Browse files Browse the repository at this point in the history
Escaping all unsafe.Pointer conversions for -d=checkptr seems like it
might be a little too aggressive to enable for -race/-msan mode, since
at least some tests are written to expect unsafe.Pointer conversions
to not affect escape analysis.

So instead only enable that functionality behind -d=checkptr=2.

Updates #22218.
Updates #34959.

Change-Id: I2f0a774ea5961dabec29bc5b8ebe387a1b90d27b
Reviewed-on: https://go-review.googlesource.com/c/go/+/201840
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
mdempsky committed Oct 18, 2019
1 parent 5375c71 commit 46aa835
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/gc/escape.go
Expand Up @@ -471,8 +471,8 @@ func (e *Escape) exprSkipInit(k EscHole, n *Node) {
e.discard(max)

case OCONV, OCONVNOP:
if checkPtr(e.curfn) && n.Type.Etype == TUNSAFEPTR && n.Left.Type.IsPtr() {
// When -d=checkptr is enabled, treat
if checkPtr(e.curfn, 2) && n.Type.Etype == TUNSAFEPTR && n.Left.Type.IsPtr() {
// When -d=checkptr=2 is enabled, treat
// conversions to unsafe.Pointer as an
// escaping operation. This allows better
// runtime instrumentation, since we can more
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/compile/internal/gc/main.go
Expand Up @@ -94,6 +94,11 @@ const debugHelpHeader = `usage: -d arg[,arg]* and arg is <key>[=<value>]
const debugHelpFooter = `
<value> is key-specific.
Key "checkptr" supports values:
"0": instrumentation disabled
"1": conversions involving unsafe.Pointer are instrumented
"2": conversions to unsafe.Pointer force heap allocation
Key "pctab" supports values:
"pctospadj", "pctofile", "pctoline", "pctoinline", "pctopcdata"
`
Expand Down
9 changes: 5 additions & 4 deletions src/cmd/compile/internal/gc/walk.go
Expand Up @@ -951,7 +951,7 @@ opswitch:

case OCONV, OCONVNOP:
n.Left = walkexpr(n.Left, init)
if n.Op == OCONVNOP && checkPtr(Curfn) {
if n.Op == OCONVNOP && checkPtr(Curfn, 1) {
if n.Type.IsPtr() && n.Left.Type.Etype == TUNSAFEPTR { // unsafe.Pointer to *T
n = walkCheckPtrAlignment(n, init)
break
Expand Down Expand Up @@ -3976,7 +3976,8 @@ func walkCheckPtrArithmetic(n *Node, init *Nodes) *Node {
}

// checkPtr reports whether pointer checking should be enabled for
// function fn.
func checkPtr(fn *Node) bool {
return Debug_checkptr != 0 && fn.Func.Pragma&NoCheckPtr == 0
// function fn at a given level. See debugHelpFooter for defined
// levels.
func checkPtr(fn *Node, level int) bool {
return Debug_checkptr >= level && fn.Func.Pragma&NoCheckPtr == 0
}

0 comments on commit 46aa835

Please sign in to comment.