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

cmd/compile: wrong liveness map #18860

Closed
randall77 opened this issue Jan 30, 2017 · 4 comments
Closed

cmd/compile: wrong liveness map #18860

randall77 opened this issue Jan 30, 2017 · 4 comments
Milestone

Comments

@randall77
Copy link
Contributor

func f(p, q *int) (r *int) {
	r = p
	defer func() {
		recover()
	}()
	g()
	r = q
	return
}
func g()

At the call to g, r should be marked as live. It isn't.

> go tool compile -live ~/go/tmp3.go
tmp3.go:3: live at entry to f: p q
tmp3.go:7: live at call to deferproc: q r
tmp3.go:8: live at call to g: q                   <- r is live here
tmp3.go:10: live at call to deferreturn: r
tmp3.go:7: live at call to deferreturn: r

Any defer makes all of the output args live at any (possibly panicing) call.
Found using the dead-slot clobberer, https://go-review.googlesource.com/c/23924/
Seems to be around for a while, at least 1.6.
I'm not sure why the dead-slot clobberer didn't find this before.

(Note to self: original detection from encoding/gob/encoder_test.go:encodeAndRecover.)

@randall77 randall77 added this to the Go1.8Maybe milestone Jan 30, 2017
@randall77 randall77 self-assigned this Jan 30, 2017
@randall77
Copy link
Contributor Author

@aclements

@gopherbot
Copy link

CL https://golang.org/cl/36030 mentions this issue.

@randall77 randall77 modified the milestones: Go1.9, Go1.8Maybe Jan 31, 2017
@gopherbot
Copy link

CL https://golang.org/cl/23924 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/38133 mentions this issue.

gopherbot pushed a commit that referenced this issue Mar 13, 2017
Move the zeroing of results earlier.  In particular, they need to
come before any move-to-heap operations, as those require allocation.
Those allocations are points at which the GC can see the uninitialized
result slots.

For the function:

func f() (x, y, z *int) {
  defer(){}()
  escape(&y)
  return
}

We used to generate code like this:

x = nil
y = nil
&y = new(int)
z = nil

Now we will generate:

x = nil
y = nil
z = nil
&y = new(int)

Since the fix for #18860, the return slots are always live if there
is a defer, so the former ordering allowed the GC to see junk
in the z slot.

Fixes #19078

Change-Id: I71554ae437549725bb79e13b2c100b2911d47ed4
Reviewed-on: https://go-review.googlesource.com/38133
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Apr 21, 2017
The experiment "clobberdead" clobbers all pointer fields that the
compiler thinks are dead, just before and after every safepoint.
Useful for debugging the generation of live pointer bitmaps.

Helped find the following issues:
Update #15936
Update #16026
Update #16095
Update #18860

Change-Id: Id1d12f86845e3d93bae903d968b1eac61fc461f9
Reviewed-on: https://go-review.googlesource.com/23924
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Mar 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants