bytes: copying Buffer struct corrupts content

[icza]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/icza/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/icza/gows"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build608842240=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run the following app https://play.golang.org/p/8XAMVW4UZd7

package main

import (
	"bytes"
	"fmt"
)

func main() {
	a := bytes.Buffer{}
	a.WriteString("a-")
	fmt.Printf("a: %q\n", a.String())

	b := a
	fmt.Printf("b: %q\n", b.String())
	b.WriteString("b")
	fmt.Printf("b: %q\n", b.String())

	a = b
	fmt.Printf("a: %q\n", a.String())
	fmt.Printf("b: %q\n", b.String())
}

What did you expect to see?

Since bytes.Buffer is a struct (holding a slice buffer and some other fields), assigning the struct value should copy all the fields (including the slice buffer - the slice header), so content should not change, and the result struct should give the same content when String() is called on it.

a: "a-"
b: "a-"
b: "a-b"
a: "a-b"
b: "a-b"

What did you see instead?

a: "a-"
b: "a-"
b: "a-b"
a: "a-\x00"
b: "a-\x00"

[gopherbot]

Change https://golang.org/cl/122595 mentions this issue: cmd/go: add test for tests with no tests

[ysmolski]

Is it safe to assume that bytes.Buffer struct can be copied just like? I do not see a test doing that in buffer_test.go.

I see that there is a function NewBuffer which creates new buffer and transfers ownership to the new buffer: https://golang.org/pkg/bytes/#NewBuffer

Your example using that function: https://play.golang.org/p/sByMPpES47e

[mvdan]

If copying buffers wasn't supported by design, it should probably be caught by vet via the noCopy trick.

[icza]

I know the bytes.Buffer struct should not be copied, but I can't explain what I experience. The buffer's content changes just because I assigned the struct to another variable. Both the assigned and the assignee changes.

If we add more print statements to the example (https://play.golang.org/p/Tt1p4CyhsPp):

a := bytes.Buffer{}
a.WriteString("a-")
fmt.Printf("a: %q\n", a.String())

b := a
fmt.Printf("b: %q\n", b.String())
b.WriteString("b")
fmt.Printf("b: %q\n", b.String())

fmt.Println("BEFORE")
fmt.Printf("a: %q\n", a.String())
fmt.Printf("b: %q\n", b.String())

a = b
fmt.Println("AFTER")
fmt.Printf("a: %q\n", a.String())
fmt.Printf("b: %q\n", b.String())

Output is:

a: "a-"
b: "a-"
b: "a-b"
BEFORE
a: "a-"
b: "a-b"
AFTER
a: "a-\x00"
b: "a-\x00"

Between the BEFORE and AFTER sections there is only an assignment:

a = b

Nothing else, yet the content of the buffers change.

[ghost]

@icza

When you do b := a, b.buf is a slice of a.bootstrap.
When you do b.WriteString("b"), a.bootstrap gets modified, not b.bootstrap
When you do a = b, a.boostrap gets written over by b.bootstrap, of which b.bootstrap[2] == 0x00, and that is a.buf's backing array.

[ysmolski]

Maybe documentation should be improved for such a case? Or vet rule added?

[icza]

@bontibon I think that explains everything, thanks.

@ysmolsky Vet rule would be definitely useful for such cases.

[bcmills]

See previously #25907, and I could have sworn that @alandonovan filed a similar issue (for a vet or lint check) but I can't find it now.

[jckasto]

@bontibon

When you do b := a, b.buf is a slice of a.bootstrap.

Can you explain the principle to me in detail?

[alandonovan]

A bytes.Buffer is an example of a data structure that contains a pointer to its own interior. (In the specific case of Buffer, its buf field is a slice that initially points to a small array called bootstrap.)

In general it is not safe to copy such a value without additional logic. (This is the purpose of a "copy constructor" in C++, but Go does not have that concept.) Otherwise, the copy and the original become entangled as both point into the interior of the original; operations on one variable may be observed by the other variable, or have unpredictable effects.

Go programmers should defensively assume that it is not safe to make a copy of any value of type T whose methods are associated with the pointer, *T.

[ghost]

Buffer.bootstrap is now gone: 9c2be4c.

[ianlancetaylor]

Fixed, so closing. Thanks.