Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: panic: runtime error: slice bounds out of range #22102

Closed
karalabe opened this issue Oct 2, 2017 · 13 comments
Closed

os: panic: runtime error: slice bounds out of range #22102

karalabe opened this issue Oct 2, 2017 · 13 comments

Comments

@karalabe
Copy link
Contributor

@karalabe karalabe commented Oct 2, 2017

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

go version go1.9 linux/amd64

Does this issue reproduce with the latest release?

Yes (both stable as well as master).

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/work"
GORACE=""
GOROOT="/opt/google/go"
GOTOOLDIR="/opt/google/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build311345589=/tmp/go-build -gno-record-gcc-switches"
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"

What did you do?

A FUSE test from our test suite started panicking when upgrading to Go 1.9. I'm not really sure whether we're doing some bad API call, but the panic seems to originate from within the os file handling.

$ go get -v github.com/ethereum/go-ethereum/swarm/fuse
$ go test -v github.com/ethereum/go-ethereum/swarm/fuse
=== RUN   TestFUSE
=== RUN   TestFUSE/mountListAndUmount
=== RUN   TestFUSE/maxMounts
=== RUN   TestFUSE/remount
=== RUN   TestFUSE/unmount
=== RUN   TestFUSE/unmountWhenResourceBusy
=== RUN   TestFUSE/seekInMultiChunkFile
=== RUN   TestFUSE/createNewFile
=== RUN   TestFUSE/createNewFileInsideDirectory
=== RUN   TestFUSE/createNewFileInsideNewDirectory
=== RUN   TestFUSE/removeExistingFile
=== RUN   TestFUSE/removeExistingFileInsideDir
=== RUN   TestFUSE/removeNewlyAddedFile
=== RUN   TestFUSE/addNewFileAndModifyContents
panic: runtime error: slice bounds out of range [recovered]
	panic: runtime error: slice bounds out of range

goroutine 1500 [running]:
testing.tRunner.func1(0xc420e08000)
	/opt/google/go/src/testing/testing.go:713 +0x29d
panic(0x8fdfa0, 0xed51d0)
	/opt/google/go/src/runtime/panic.go:502 +0x266
internal/poll.(*FD).Write(0xc420d94b40, 0xc4206ee8f0, 0x6, 0x8, 0x0, 0x0, 0x0)
	/opt/google/go/src/internal/poll/fd_unix.go:227 +0x373
os.(*File).write(0xc420cdf0f8, 0xc4206ee8f0, 0x6, 0x8, 0xc420cdf0f8, 0x6, 0x0)
	/opt/google/go/src/os/file_unix.go:235 +0x4e
os.(*File).Write(0xc420cdf0f8, 0xc4206ee8f0, 0x6, 0x8, 0x0, 0x0, 0x0)
	/opt/google/go/src/os/file.go:140 +0x72
github.com/ethereum/go-ethereum/swarm/fuse.(*testAPI).addNewFileAndModifyContents(0xc42028e020, 0xc420e08000)
	/tmp/go/src/github.com/ethereum/go-ethereum/swarm/fuse/swarmfs_test.go:660 +0xc42
github.com/ethereum/go-ethereum/swarm/fuse.(*testAPI).(github.com/ethereum/go-ethereum/swarm/fuse.addNewFileAndModifyContents)-fm(0xc420e08000)
	/tmp/go/src/github.com/ethereum/go-ethereum/swarm/fuse/swarmfs_test.go:833 +0x34
testing.tRunner(0xc420e08000, 0xc42005be50)
	/opt/google/go/src/testing/testing.go:748 +0xc0
created by testing.(*T).Run
	/opt/google/go/src/testing/testing.go:791 +0x2b1

What did you expect to see?

A successfully passing test, as with Go 1.8.x and Go 1.7.x.

What did you see instead?

Boom.

@karalabe

This comment has been minimized.

Copy link
Contributor Author

@karalabe karalabe commented Oct 2, 2017

Tracking issue on the Ethereum repo: ethereum/go-ethereum#15216

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 3, 2017

I assume this is repeatable. I don't see any problem in the code, unless syscall.Write for some reason incorrectly returns a very large positive value.

Can you do some debugging to find out what the slice indexes are when the panic occurs? Thanks.

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 3, 2017
@opennota

This comment has been minimized.

Copy link

@opennota opennota commented Oct 3, 2017

/opt/google/go/src/os/file_unix.go:235

/opt/google/go/src/internal/poll/fd_unix.go:227

Is that really go1.9, not a git revision? Line 235 is return n, err:

   232	func (f *File) write(b []byte) (n int, err error) {
   233		n, err = f.pfd.Write(b)
   234		runtime.KeepAlive(f)
   235		return n, err
   236	}

and line 227 is continue:

   225			if err == syscall.EAGAIN && fd.pd.pollable() {
   226				if err = fd.pd.waitWrite(fd.isFile); err == nil {
   227					continue
   228				}
   229			}
@karalabe

This comment has been minimized.

Copy link
Contributor Author

@karalabe karalabe commented Oct 3, 2017

@ianlancetaylor Will do!
@opennota I may have pasted the trace from master here, didn't notice there's a diff, but 1.9 panics the same way.

@karalabe

This comment has been minimized.

Copy link
Contributor Author

@karalabe karalabe commented Oct 3, 2017

@ianlancetaylor I've added a print statement inside internal/poll/fd_unix.go, and syscall.Write indeed returns twice the size of my input array (I'm writing a slice of length 6, and syscall.Write returns 12, which blows at the next iteration).

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 3, 2017

So, you've found your problem; it must be in your FUSE driver. I think it's reasonable for Go to assume that syscall.Write returns -1 on error and the number of bytes written on success, and I think it's reasonable for Go to assume that if it passes 6 bytes to Write that no more than 6 bytes will be written. I don't think it should be necessary for the Go standard library to take precautions against other return values. I'm going to close this issue.

@karalabe

This comment has been minimized.

Copy link
Contributor Author

@karalabe karalabe commented Oct 4, 2017

@karalabe

This comment has been minimized.

Copy link
Contributor Author

@karalabe karalabe commented Oct 4, 2017

@as

This comment has been minimized.

Copy link
Contributor

@as as commented Oct 4, 2017

Note: Issue #22024 also deals with the poller introduced in go1.9 has the same high-level user experience. I.e., "it worked on go1.8 but its broken on go1.9".

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 4, 2017

@karalabe Yes, the Go code has changed, and that may well affect the behavior when using a broken FUSE driver (assuming the FUSE driver is indeed broken). But we can't promise to retain precisely the same behavior in all cases for each new Go release. If we did, we may as well not make Go releases.

Go is intended to be a safe language. That does not mean that it should never crash. Quite the opposite. It means that it should not behave unexpectedly. For example, it should not continue running after an erroneous pointer reference. If my earlier analysis is correct then the behavior here is consistent with that goal. Looking at Go 1.8, it appears that if syscall.Write returns an incorrect value that the os package would return that incorrect value back to the caller of os.(*File).Write (I didn't test this, this is just from reading the code). In other words, if syscall.Write fails to implement its API, then os.(*File).Write would fail to implement its API. In those terms, although the error handling is terrible, this is actually a step forward.

To be honest, if I were in your position, I would be pleased: a new Go release may have found a bug in the FUSE driver. That is a good thing. So I'm a little bit befuddled by your suggestion that this change in behavior is a bug in Go.

jmozah referenced this issue in ethereum/go-ethereum Oct 10, 2017
If the file already existed, the WriteResponse.Size was being set
as the length of the entire file, not just the amount that was
written to the existing file.

Fixes #15216
@protheusfr

This comment has been minimized.

Copy link

@protheusfr protheusfr commented Mar 2, 2018

Hello,

I just encountered the same issue after recompiled an existing internal software that use Minio with GoLang 1.10.
No issue with GoLang 1.9.4.

panic: runtime error: slice bounds out of range

goroutine 933 [running]:
bytes.(*Buffer).grow(0xc420170bd0, 0x200, 0xc4200aa050)
	/usr/lib/go/src/bytes/buffer.go:139 +0x297
bytes.(*Buffer).ReadFrom(0xc420170bd0, 0xda10c0, 0xc420663460, 0x7f8b73181120, 0xc420170bd0, 0x1)
	/usr/lib/go/src/bytes/buffer.go:204 +0x48
io.copyBuffer(0xda0020, 0xc420170bd0, 0xda10c0, 0xc420663460, 0x0, 0x0, 0x0, 0xbe0aa0, 0x13, 0x18)
	/usr/lib/go/src/io/io.go:386 +0x31a
io.Copy(0xda0020, 0xc420170bd0, 0xda10c0, 0xc420663460, 0x10, 0x0, 0x0)
	/usr/lib/go/src/io/io.go:362 +0x5a
io.CopyN(0xda0020, 0xc420170bd0, 0xda1720, 0xc4202e6000, 0x10, 0x10, 0x0, 0x0)
	/usr/lib/go/src/io/io.go:338 +0x8b
xxx/xxx/vendor/github.com/minio/minio-go/pkg/encrypt.(*CBCSecureMaterials).Read(0xc420376480, 0xc445120000, 0x24000000, 0x24000000, 0xca40, 0x0, 0x0)
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 2, 2018

@protheusfr This issue is closed. It is very unlikely that you are seeing the same problem: the stack trace you show is completely different. Please open a new issue, or ask on a forum; see https://golang.org/wiki/Questions. Thanks.

@protheusfr

This comment has been minimized.

Copy link

@protheusfr protheusfr commented Mar 2, 2018

Ok no pb.
Issue created : #24218

@golang golang locked and limited conversation to collaborators Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.