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: bad liveness #16095

Closed
randall77 opened this issue Jun 17, 2016 · 2 comments
Closed

cmd/compile: bad liveness #16095

randall77 opened this issue Jun 17, 2016 · 2 comments

Comments

@randall77
Copy link
Contributor

I've found a case where we don't generate live variable bitmaps correctly (using https://go-review.googlesource.com/c/23924/).

package main

import (
    "fmt"
    "runtime"
)

var sink *[20]byte

func f() (x [20]byte) {
    // Force x to be allocated on the heap.
    sink = &x
    sink = nil

    // Go to deferreturn after the panic below.
    defer func() {
        recover()
    }()

    // This call collects the heap-allocated version of x (!)
    runtime.GC()

    // Allocate that same object again and clobber it.
    y := new([20]byte)
    for i := 0; i < 20; i++ {
        y[i] = 99
    }
    // Make sure y is heap allocated.
    sink = y

    panic(nil)

    // After the recover we reach the deferreturn, which
    // copies the heap version of x back to the stack.
    // It gets the pointer to x from a stack slot that was
    // not marked as live during the call to runtime.GC().
}
func main() {
    x := f()
    for _, v := range x {
        if v != 0 {
            fmt.Printf("%v\n", x)
            panic("bad")
        }
    }
}

This program works in go1.2.2 but panics in later versions (go.1.3.3 through tip). I've labeled in 1.7maybe because it's not like we're breaking something that used to work.

See #16026.

The bug I think is a combination of:

  1. Control flow only reaches the (non-panic) return point because of a recover() in a defer.
  2. The use of &x is implicit at the return point.

The fix may be as simple as marking &v as live throughout the function for all return variables. We could probably restrict it to only functions that defer.

@aclements

@randall77 randall77 added this to the Go1.7Maybe milestone Jun 17, 2016
@randall77 randall77 self-assigned this Jun 17, 2016
@randall77 randall77 changed the title cmd/compile: bad livess cmd/compile: bad livness Jun 17, 2016
@randall77 randall77 changed the title cmd/compile: bad livness cmd/compile: bad liveness Jun 17, 2016
@gopherbot
Copy link

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

@gopherbot
Copy link

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

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 Feb 3, 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