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

Jump to bottom

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

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

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

mgood opened this issue Jan 31, 2019 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@mgood
Copy link

mgood 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
Copy link
Contributor

/cc @aclements @cherrymui @randall77 @ianlancetaylor

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 31, 2019
@bradfitz bradfitz added this to the Go1.12 milestone Jan 31, 2019
@randall77
Copy link
Contributor

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

@cherrymui
Copy link
Member

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
Copy link
Member

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
Copy link

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

mgood pushed a commit to shiftcars/go that referenced this issue Feb 1, 2019
@cherrymui
reflect: eliminate write barrier for copying result in callReflect
4c02fcd 
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
Copy link
Author

mgood 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.

nebulabox pushed a commit to nebulabox/go that referenced this issue Feb 18, 2019
@cherrymui @nebulabox
reflect: eliminate write barrier for copying result in callReflect
3ea418b 
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 pushed a commit to nebulabox/go that referenced this issue Feb 20, 2019
@cherrymui @nebulabox
reflect: eliminate write barrier for copying result in callReflect
3e7f550 
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>
@golang golang locked and limited conversation to collaborators Feb 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
5 participants
@bradfitz @mgood @randall77 @gopherbot @cherrymui