strings: prevent copyCheck from forcing Builder to escape and allocate

All credit and blame goes to Ian for this suggestion, copied from the
runtime.

Fixes #23382
Updates #7921

Change-Id: I3d5a9ee4ab730c87e0f3feff3e7fceff9bcf9e18
Reviewed-on: https://go-review.googlesource.com/86976
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/strings/builder.go b/src/strings/builder.go
index 11bcee1dfcdfb4e2c8d82760d51c27cc8ff8b336..ac58f34e1dec25048761a3047ee87a2c7941a441 100644 (file)
--- a/src/strings/builder.go
+++ b/src/strings/builder.go
@@ -17,9 +17,26 @@ type Builder struct {
        buf  []byte
 }
 
+// noescape hides a pointer from escape analysis.  noescape is
+// the identity function but escape analysis doesn't think the
+// output depends on the input. noescape is inlined and currently
+// compiles down to zero instructions.
+// USE CAREFULLY!
+// This was copied from the runtime; see issues 23382 and 7921.
+//go:nosplit
+func noescape(p unsafe.Pointer) unsafe.Pointer {
+       x := uintptr(p)
+       return unsafe.Pointer(x ^ 0)
+}
+
 func (b *Builder) copyCheck() {
        if b.addr == nil {
-               b.addr = b
+               // 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)))
        } else if b.addr != b {
                panic("strings: illegal use of non-zero Builder copied by value")
        }

diff --git a/src/strings/builder_test.go b/src/strings/builder_test.go
index c0c8fa4130fb1eb5c3e2fb1034d19d37c380e172..ecbaeaa5c17c0ced0500d3939eb2f8c0002320db 100644 (file)
--- a/src/strings/builder_test.go
+++ b/src/strings/builder_test.go
@@ -180,6 +180,18 @@ func TestBuilderAllocs(t *testing.T) {
        if allocs > 0 {
                t.Fatalf("got %d alloc(s); want 0", allocs)
        }
+
+       // Issue 23382; verify that copyCheck doesn't force the
+       // Builder to escape and be heap allocated.
+       n := testing.AllocsPerRun(10000, func() {
+               var b Builder
+               b.Grow(5)
+               b.WriteString("abcde")
+               _ = b.String()
+       })
+       if n != 1 {
+               t.Errorf("Builder allocs = %v; want 1", n)
+       }
 }
 
 func numAllocs(fn func()) uint64 {