aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrad Fitzpatrick <bradfitz@golang.org>2018-01-09 18:08:43 +0000
committerBrad Fitzpatrick <bradfitz@golang.org>2018-01-09 22:01:28 +0000
commit484586c81a0196e42ac52f651bc56017ca454280 (patch)
tree69983a55b34bb5354421f3ace5a16848dc769fe7
parentb26c88db2f338d6529e1021db59f0faa75af5934 (diff)
downloadgo-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.go19
-rw-r--r--src/strings/builder_test.go12
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 {