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: panic message clobbered #30150

Closed
cherrymui opened this issue Feb 10, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@cherrymui
Copy link
Contributor

commented Feb 10, 2019

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

tip (ca7c12d)

Does this issue reproduce with the latest release?

No, not with Go 1.11

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

linux/amd64

What did you do?

https://play.golang.org/p/HIJtff_t5Wh

package main

import "runtime"

type P string

func (p P) String() string {
	runtime.GC()
	runtime.GC()
	runtime.GC()
	zzz := "ZZZ"
	sink = zzz
	return string(p)
}

var sink interface{}

func main() {
	defer func() {
		panic(P("YYY"))
	}()
	panic(P("XXX"))
}

What did you expect to see?

With Go 1.11, this program panics with

panic: XXX
	panic: YYY

which looks correct to me.

What did you see instead?

With tip, this program panics with

panic: XXX
	panic: ZZZ

The "YYY" panic message is clobbered.

In runtime.gopanic, the _panic struct p is stack allocated and referenced from gp._panic. With stack objects, the stack variable p is dead at the point the panic message is printed (preprintpanics at https://go.googlesource.com/go/+/master/src/runtime/panic.go#563). gp._panic points to p, but stack scan doesn't look at gp. So whatever p points to may be collected and clobbered. We need to keep it alive.

Will send a CL soon.

@cherrymui cherrymui added this to the Go1.12 milestone Feb 10, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Feb 10, 2019

Change https://golang.org/cl/161778 mentions this issue: runtime: keep stack allocated _panic live

@odeke-em

This comment has been minimized.

Copy link
Member

commented Feb 10, 2019

Thank you for this report @cherrymui!

I can confirm that CL https://go-review.googlesource.com/c/134156 introduced this bug. Kindly paging @randall77 as well as @aclements

@gopherbot

This comment has been minimized.

Copy link

commented Feb 13, 2019

Change https://golang.org/cl/162358 mentions this issue: [release-branch.go1.12] runtime: scan gp._panic in stack scan

gopherbot pushed a commit that referenced this issue Feb 13, 2019

[release-branch.go1.12] runtime: scan gp._panic in stack scan
In runtime.gopanic, the _panic object p is stack allocated and
referenced from gp._panic. With stack objects, p on stack is dead
at the point preprintpanics runs. gp._panic points to p, but
stack scan doesn't look at gp. Heap scan of gp does look at
gp._panic, but it stops and ignores the pointer as it points to
the stack. So whatever p points to may be collected and clobbered.
We need to scan gp._panic explicitly during stack scan.

To test it reliably, we introduce a GODEBUG mode "clobberfree",
which clobbers the memory content when the GC frees an object.

Fixes #30150.

Change-Id: I11128298f03a89f817faa221421a9d332b41dced
Reviewed-on: https://go-review.googlesource.com/c/161778
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit af8f406)
Reviewed-on: https://go-review.googlesource.com/c/162358
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

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

runtime: scan gp._panic in stack scan
In runtime.gopanic, the _panic object p is stack allocated and
referenced from gp._panic. With stack objects, p on stack is dead
at the point preprintpanics runs. gp._panic points to p, but
stack scan doesn't look at gp. Heap scan of gp does look at
gp._panic, but it stops and ignores the pointer as it points to
the stack. So whatever p points to may be collected and clobbered.
We need to scan gp._panic explicitly during stack scan.

To test it reliably, we introduce a GODEBUG mode "clobberfree",
which clobbers the memory content when the GC frees an object.

Fixes golang#30150.

Change-Id: I11128298f03a89f817faa221421a9d332b41dced
Reviewed-on: https://go-review.googlesource.com/c/161778
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

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

runtime: scan gp._panic in stack scan
In runtime.gopanic, the _panic object p is stack allocated and
referenced from gp._panic. With stack objects, p on stack is dead
at the point preprintpanics runs. gp._panic points to p, but
stack scan doesn't look at gp. Heap scan of gp does look at
gp._panic, but it stops and ignores the pointer as it points to
the stack. So whatever p points to may be collected and clobbered.
We need to scan gp._panic explicitly during stack scan.

To test it reliably, we introduce a GODEBUG mode "clobberfree",
which clobbers the memory content when the GC frees an object.

Fixes golang#30150.

Change-Id: I11128298f03a89f817faa221421a9d332b41dced
Reviewed-on: https://go-review.googlesource.com/c/161778
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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.