diff options
author | Brad Fitzpatrick <bradfitz@golang.org> | 2018-01-09 18:08:43 +0000 |
---|---|---|
committer | Brad Fitzpatrick <bradfitz@golang.org> | 2018-01-09 22:01:28 +0000 |
commit | 484586c81a0196e42ac52f651bc56017ca454280 (patch) | |
tree | 69983a55b34bb5354421f3ace5a16848dc769fe7 | |
parent | b26c88db2f338d6529e1021db59f0faa75af5934 (diff) | |
download | go-484586c81a0196e42ac52f651bc56017ca454280.tar.gz go-484586c81a0196e42ac52f651bc56017ca454280.zip |
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>
-rw-r--r-- | src/strings/builder.go | 19 | ||||
-rw-r--r-- | src/strings/builder_test.go | 12 |
2 files changed, 30 insertions, 1 deletions
diff --git a/src/strings/builder.go b/src/strings/builder.go index 11bcee1dfc..ac58f34e1d 100644 --- 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 c0c8fa4130..ecbaeaa5c1 100644 --- 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 { |