Skip to content

Commit

Permalink
cmd/compile: don't inline runtime functions in -d=checkptr build
Browse files Browse the repository at this point in the history
Runtime functions, e.g. internal/abi.NoEscape, should not be
instrumented with checkptr. But if they are inlined into a
checkptr-enabled function, they will be instrumented, and may
result in a check failure.

Let the compiler not inline runtime functions into checkptr-
enabled functions.

Also undo the change in the strings package in CL 598295, as the
compiler handles it now.

Fixes golang#68511.
Updates golang#68415.

Change-Id: I78eb380855ac9dd53c1a1a628ec0da75c3e5a1a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/599435
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
  • Loading branch information
cherrymui committed Jul 22, 2024
1 parent 3959d54 commit f0de94f
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 14 deletions.
9 changes: 9 additions & 0 deletions src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,15 @@ func canInlineCallExpr(callerfn *ir.Func, n *ir.CallExpr, callee *ir.Func, bigCa
return false, 0, false
}

if base.Debug.Checkptr != 0 && types.IsRuntimePkg(callee.Sym().Pkg) {
// We don't intrument runtime packages for checkptr (see base/flag.go).
if log && logopt.Enabled() {
logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(callerfn),
fmt.Sprintf(`call to into runtime package function %s in -d=checkptr build`, ir.PkgFuncName(callee)))
}
return false, 0, false
}

// Check if we've already inlined this function at this particular
// call site, in order to stop inlining when we reach the beginning
// of a recursion cycle again. We don't inline immediately recursive
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/compile/internal/types/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,11 @@ func IsNoRacePkg(p *Pkg) bool {
return objabi.LookupPkgSpecial(p.Path).NoRaceFunc
}

// IsRuntimePkg reports whether p is a runtime package.
func IsRuntimePkg(p *Pkg) bool {
return objabi.LookupPkgSpecial(p.Path).Runtime
}

// ReceiverBaseType returns the underlying type, if any,
// that owns methods with receiver parameter t.
// The result is either a named type or an anonymous struct.
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/internal/objabi/pkgspecial.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ type PkgSpecial struct {
//
// - Optimizations are always enabled.
//
// - Checkptr is always disabled.
//
// This should be set for runtime and all packages it imports, and may be
// set for additional packages.
Runtime bool
Expand Down
14 changes: 1 addition & 13 deletions src/strings/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,14 @@ type Builder struct {
buf []byte
}

// This is just a wrapper around abi.NoEscape.
//
// This wrapper is necessary because internal/abi is a runtime package,
// so it can not be built with -d=checkptr, causing incorrect inlining
// decision when building with checkptr enabled, see issue #68415.
//
//go:nosplit
//go:nocheckptr
func noescape(p unsafe.Pointer) unsafe.Pointer {
return abi.NoEscape(p)
}

func (b *Builder) copyCheck() {
if b.addr == nil {
// This hack works around a failing of Go's escape analysis
// that was causing b to escape and be heap allocated.
// See issue 23382.
// TODO: once issue 7921 is fixed, this should be reverted to
// just "b.addr = b".
b.addr = (*Builder)(noescape(unsafe.Pointer(b)))
b.addr = (*Builder)(abi.NoEscape(unsafe.Pointer(b)))
} else if b.addr != b {
panic("strings: illegal use of non-zero Builder copied by value")
}
Expand Down
6 changes: 5 additions & 1 deletion test/fixedbugs/issue68415.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@

package main

import "regexp"
import (
"regexp"
"unique"
)

var dataFileRegexp = regexp.MustCompile(`^data\.\d+\.bin$`)

func main() {
_ = dataFileRegexp
unique.Make("")
}

0 comments on commit f0de94f

Please sign in to comment.