Skip to content

cmd/compile: wrong liveness map #18860

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.

Sign up for GitHub

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
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
Labels
FrozenDueToAge
Milestone
Go1.9

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
Contributor

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

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

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

@josharian josharian mentioned this issue Feb 14, 2017
Closed
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Mar 13, 2017
@randall77
cmd/compile: zero return parameters earlier
a51e4cc 
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
gopherbot pushed a commit that referenced this issue Apr 21, 2017
@randall77
cmd/compile: experiment which clobbers all dead pointer fields
1e72bf6 
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 <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
@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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
2 participants
@randall77 @gopherbot