Skip to content

Commit

Permalink
strings: re-introduce noescape wrapper
Browse files Browse the repository at this point in the history
CL 573955 added internal/abi:NoEscape function, and use it in strings
builder copyCheck code.

However, internal/abi is a runtime package, which can not be built with
-d=checkptr flag yet. This causes incorrect inlining decision, since
NoEscape must not be inlined when -d=checkptr is used.

Fixing this by re-introducing noescape wrapper.

Fixes #68415

Change-Id: I776cab4c9e9e4b3e58162dcce6ec025cb366bdee
Reviewed-on: https://go-review.googlesource.com/c/go/+/598295
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Jorropo <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Auto-Submit: Cuong Manh Le <[email protected]>
  • Loading branch information
cuonglm authored and gopherbot committed Jul 15, 2024
1 parent 5d36bc1 commit 8f1ec59
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/strings/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,26 @@ 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)(abi.NoEscape(unsafe.Pointer(b)))
b.addr = (*Builder)(noescape(unsafe.Pointer(b)))
} else if b.addr != b {
panic("strings: illegal use of non-zero Builder copied by value")
}
Expand Down
15 changes: 15 additions & 0 deletions test/fixedbugs/issue68415.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run -gcflags=all=-d=checkptr

// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main

import "regexp"

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

func main() {
_ = dataFileRegexp
}

0 comments on commit 8f1ec59

Please sign in to comment.