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

runtime: go1.11.5 GC crash when calling reflect.MakeFunc function #30041

Closed
mgood opened this issue Jan 31, 2019 · 6 comments

Comments

Projects
None yet
5 participants
@mgood
Copy link

commented Jan 31, 2019

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

$ go version
go version go1.11.5 linux/amd64
$ go version
go version go1.11.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build773613817=/tmp/go-build -gno-record-gcc-switches"

What did you do?

go run main.go from this example https://gist.github.com/mgood/2f92cbfa67bfb15fef5b59617a466d30

What did you expect to see?

Should keep running until stopped

What did you see instead?

runtime: pointer 0xc0006057d0 to unallocated span span.base()=0xc0005f8000 span.limit=0xc000605fe0 span.state=3
fatal error: found bad pointer in Go heap (incorrect use of unsafe or cgo?)

In production we also saw other runtime crashes such as:

fatal error: sweep increased allocation count

This seems similar to #18635 which was fixed in Go 1.8rc2, however the example in that issue keeps running for me until it hits the stack depth limit.

When running with -race no errors are reported, but it also doesn't reproduce the crash so the race detector may be imposing some synchronization that mitigates the problem.

The body of the loop in GetSomeFlag doesn't contain any concurrency or modify any external state, so our suspicion is that the issue is related to reflect.MakeFunc since that's the only part that seems unusual. If I replace the reflect func with a regular func I haven't been able to reproduce the crash.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

@bradfitz bradfitz added this to the Go1.12 milestone Jan 31, 2019

@randall77

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

Sounds like #28750 also, but that should have been fixed in 1.11.4.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

https://go.googlesource.com/go/+/go1.11.5/src/reflect/value.go#567

If I understand correctly, this stores to the stack, which is uninitialized at this point, but has a write barrier.

But even I disable this write barrier, the program can still crash. I no longer see the crash with bad pointer found in write barrier buffer flush, but still see bad pointer found in gcDrain. There must be some other problem...

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

Apparently my local change didn't take effect in the first time. I think by removing that write barrier it indeed fix the problem.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 1, 2019

Change https://golang.org/cl/160737 mentions this issue: reflect: eliminate write barrier for copying result in callReflect

mgood added a commit to shiftcars/go that referenced this issue Feb 1, 2019

reflect: eliminate write barrier for copying result in callReflect
We are copying the results to uninitialized stack space. Write
barrier is not needed.

TODO: add a test. I don't know how to test this in a reliable
way.

Fixes golang#30041.

Change-Id: Ia91d74dbafd96dc2bd92de0cb479808991dda03e
@mgood

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

Thanks, I did some local testing and it no longer reproduces the crash. I've applied the patch to our toolchain and going to do some more testing on the real code.

@gopherbot gopherbot closed this in 7e987b7 Feb 1, 2019

nebulabox added a commit to nebulabox/go that referenced this issue Feb 18, 2019

reflect: eliminate write barrier for copying result in callReflect
We are copying the results to uninitialized stack space. Write
barrier is not needed.

Fixes golang#30041.

Change-Id: Ia91d74dbafd96dc2bd92de0cb479808991dda03e
Reviewed-on: https://go-review.googlesource.com/c/160737
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>

nebulabox added a commit to nebulabox/go that referenced this issue Feb 20, 2019

reflect: eliminate write barrier for copying result in callReflect
We are copying the results to uninitialized stack space. Write
barrier is not needed.

Fixes golang#30041.

Change-Id: Ia91d74dbafd96dc2bd92de0cb479808991dda03e
Reviewed-on: https://go-review.googlesource.com/c/160737
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.