Commit 9a8372f8 authored by Keith Randall's avatar Keith Randall Committed by Keith Randall

cmd/compile,runtime: remove ambiguously live logic

The previous CL introduced stack objects. This CL removes the old
ambiguously live liveness analysis. After this CL we're relying
on stack objects exclusively.

Update a bunch of liveness tests to reflect the new world.

Fixes #22350

Change-Id: I739b26e015882231011ce6bc1a7f426049e59f31
Reviewed-on: https://go-review.googlesource.com/c/134156Reviewed-by: 's avatarAustin Clements <austin@google.com>
Reviewed-by: 's avatarCherry Zhang <cherryyz@google.com>
parent cbafcc55
......@@ -4,9 +4,9 @@ package gc
import "strconv"
const _Op_name = "XXXNAMENONAMETYPEPACKLITERALADDSUBORXORADDSTRADDRANDANDAPPENDARRAYBYTESTRARRAYBYTESTRTMPARRAYRUNESTRSTRARRAYBYTESTRARRAYBYTETMPSTRARRAYRUNEASAS2AS2FUNCAS2RECVAS2MAPRAS2DOTTYPEASOPCALLCALLFUNCCALLMETHCALLINTERCALLPARTCAPCLOSECLOSURECMPIFACECMPSTRCOMPLITMAPLITSTRUCTLITARRAYLITSLICELITPTRLITCONVCONVIFACECONVNOPCOPYDCLDCLFUNCDCLFIELDDCLCONSTDCLTYPEDELETEDOTDOTPTRDOTMETHDOTINTERXDOTDOTTYPEDOTTYPE2EQNELTLEGEGTINDINDEXINDEXMAPKEYSTRUCTKEYLENMAKEMAKECHANMAKEMAPMAKESLICEMULDIVMODLSHRSHANDANDNOTNEWNOTCOMPLUSMINUSORORPANICPRINTPRINTNPARENSENDSLICESLICEARRSLICESTRSLICE3SLICE3ARRRECOVERRECVRUNESTRSELRECVSELRECV2IOTAREALIMAGCOMPLEXALIGNOFOFFSETOFSIZEOFBLOCKBREAKCASEXCASECONTINUEDEFEREMPTYFALLFORFORUNTILGOTOIFLABELPROCRANGERETURNSELECTSWITCHTYPESWTCHANTMAPTSTRUCTTINTERTFUNCTARRAYDDDDDDARGINLCALLEFACEITABIDATASPTRCLOSUREVARCFUNCCHECKNILVARKILLVARLIVEINDREGSPRETJMPGETGEND"
const _Op_name = "XXXNAMENONAMETYPEPACKLITERALADDSUBORXORADDSTRADDRANDANDAPPENDARRAYBYTESTRARRAYBYTESTRTMPARRAYRUNESTRSTRARRAYBYTESTRARRAYBYTETMPSTRARRAYRUNEASAS2AS2FUNCAS2RECVAS2MAPRAS2DOTTYPEASOPCALLCALLFUNCCALLMETHCALLINTERCALLPARTCAPCLOSECLOSURECMPIFACECMPSTRCOMPLITMAPLITSTRUCTLITARRAYLITSLICELITPTRLITCONVCONVIFACECONVNOPCOPYDCLDCLFUNCDCLFIELDDCLCONSTDCLTYPEDELETEDOTDOTPTRDOTMETHDOTINTERXDOTDOTTYPEDOTTYPE2EQNELTLEGEGTINDINDEXINDEXMAPKEYSTRUCTKEYLENMAKEMAKECHANMAKEMAPMAKESLICEMULDIVMODLSHRSHANDANDNOTNEWNOTCOMPLUSMINUSORORPANICPRINTPRINTNPARENSENDSLICESLICEARRSLICESTRSLICE3SLICE3ARRRECOVERRECVRUNESTRSELRECVSELRECV2IOTAREALIMAGCOMPLEXALIGNOFOFFSETOFSIZEOFBLOCKBREAKCASEXCASECONTINUEDEFEREMPTYFALLFORFORUNTILGOTOIFLABELPROCRANGERETURNSELECTSWITCHTYPESWTCHANTMAPTSTRUCTTINTERTFUNCTARRAYDDDDDDARGINLCALLEFACEITABIDATASPTRCLOSUREVARCFUNCCHECKNILVARDEFVARKILLVARLIVEINDREGSPRETJMPGETGEND"
var _Op_index = [...]uint16{0, 3, 7, 13, 17, 21, 28, 31, 34, 36, 39, 45, 49, 55, 61, 73, 88, 100, 112, 127, 139, 141, 144, 151, 158, 165, 175, 179, 183, 191, 199, 208, 216, 219, 224, 231, 239, 245, 252, 258, 267, 275, 283, 289, 293, 302, 309, 313, 316, 323, 331, 339, 346, 352, 355, 361, 368, 376, 380, 387, 395, 397, 399, 401, 403, 405, 407, 410, 415, 423, 426, 435, 438, 442, 450, 457, 466, 469, 472, 475, 478, 481, 484, 490, 493, 496, 499, 503, 508, 512, 517, 522, 528, 533, 537, 542, 550, 558, 564, 573, 580, 584, 591, 598, 606, 610, 614, 618, 625, 632, 640, 646, 651, 656, 660, 665, 673, 678, 683, 687, 690, 698, 702, 704, 709, 713, 718, 724, 730, 736, 742, 747, 751, 758, 764, 769, 775, 778, 784, 791, 796, 800, 805, 809, 819, 824, 832, 839, 846, 854, 860, 864, 867}
var _Op_index = [...]uint16{0, 3, 7, 13, 17, 21, 28, 31, 34, 36, 39, 45, 49, 55, 61, 73, 88, 100, 112, 127, 139, 141, 144, 151, 158, 165, 175, 179, 183, 191, 199, 208, 216, 219, 224, 231, 239, 245, 252, 258, 267, 275, 283, 289, 293, 302, 309, 313, 316, 323, 331, 339, 346, 352, 355, 361, 368, 376, 380, 387, 395, 397, 399, 401, 403, 405, 407, 410, 415, 423, 426, 435, 438, 442, 450, 457, 466, 469, 472, 475, 478, 481, 484, 490, 493, 496, 499, 503, 508, 512, 517, 522, 528, 533, 537, 542, 550, 558, 564, 573, 580, 584, 591, 598, 606, 610, 614, 618, 625, 632, 640, 646, 651, 656, 660, 665, 673, 678, 683, 687, 690, 698, 702, 704, 709, 713, 718, 724, 730, 736, 742, 747, 751, 758, 764, 769, 775, 778, 784, 791, 796, 800, 805, 809, 819, 824, 832, 838, 845, 852, 860, 866, 870, 873}
func (i Op) String() string {
if i >= Op(len(_Op_index)-1) {
......
This diff is collapsed.
......@@ -839,6 +839,10 @@ func slicelit(ctxt initContext, n *Node, var_ *Node, init *Nodes) {
a = nod(OAS, x, nil)
a = typecheck(a, Etop)
init.Append(a) // zero new temp
} else {
// Declare that we're about to initialize all of x.
// (Which happens at the *vauto = vstat below.)
init.Append(nod(OVARDEF, x, nil))
}
a = nod(OADDR, x, nil)
......@@ -849,6 +853,8 @@ func slicelit(ctxt initContext, n *Node, var_ *Node, init *Nodes) {
a = typecheck(a, Etop)
init.Append(a) // zero new temp
a = a.Left
} else {
init.Append(nod(OVARDEF, a, nil))
}
a = nod(OADDR, a, nil)
......
......@@ -755,8 +755,8 @@ func (s *state) stmtList(l Nodes) {
// stmt converts the statement n to SSA and adds it to s.
func (s *state) stmt(n *Node) {
if !(n.Op == OVARKILL || n.Op == OVARLIVE) {
// OVARKILL and OVARLIVE are invisible to the programmer, so we don't use their line numbers to avoid confusion in debugging.
if !(n.Op == OVARKILL || n.Op == OVARLIVE || n.Op == OVARDEF) {
// OVARKILL, OVARLIVE, and OVARDEF are invisible to the programmer, so we don't use their line numbers to avoid confusion in debugging.
s.pushLine(n.Pos)
defer s.popLine()
}
......@@ -1170,6 +1170,10 @@ func (s *state) stmt(n *Node) {
}
s.startBlock(bEnd)
case OVARDEF:
if !s.canSSA(n.Left) {
s.vars[&memVar] = s.newValue1Apos(ssa.OpVarDef, types.TypeMem, n.Left, s.mem(), false)
}
case OVARKILL:
// Insert a varkill op to record that a variable is no longer live.
// We only care about liveness info at call sites, so putting the
......@@ -4977,6 +4981,12 @@ func emitStackObjects(e *ssafn, pp *Progs) {
p.To.Type = obj.TYPE_MEM
p.To.Name = obj.NAME_EXTERN
p.To.Sym = x
if debuglive != 0 {
for _, v := range vars {
Warnl(v.Pos, "stack object %v %s", v, v.Type.String())
}
}
}
// genssa appends entries to pp for each instruction in f.
......@@ -5056,24 +5066,8 @@ func genssa(f *ssa.Func, pp *Progs) {
case ssa.OpGetG:
// nothing to do when there's a g register,
// and checkLower complains if there's not
case ssa.OpVarDef, ssa.OpVarLive, ssa.OpKeepAlive:
case ssa.OpVarDef, ssa.OpVarLive, ssa.OpKeepAlive, ssa.OpVarKill:
// nothing to do; already used by liveness
case ssa.OpVarKill:
// Zero variable if it is ambiguously live.
// After the VARKILL anything this variable references
// might be collected. If it were to become live again later,
// the GC will see references to already-collected objects.
// See issue 20029.
n := v.Aux.(*Node)
if n.Name.Needzero() {
if n.Class() != PAUTO {
v.Fatalf("zero of variable which isn't PAUTO %v", n)
}
if n.Type.Size()%int64(Widthptr) != 0 {
v.Fatalf("zero of variable not a multiple of ptr size %v", n)
}
thearch.ZeroAuto(s.pp, n)
}
case ssa.OpPhi:
CheckLoweredPhi(v)
case ssa.OpConvert:
......
......@@ -739,6 +739,7 @@ const (
OCLOSUREVAR // variable reference at beginning of closure function
OCFUNC // reference to c function pointer (not go func value)
OCHECKNIL // emit code to ensure pointer/interface not nil
OVARDEF // variable is about to be fully initialized
OVARKILL // variable is dead
OVARLIVE // variable is alive
OINDREGSP // offset plus indirect of REGSP, such as 8(SP).
......
......@@ -470,8 +470,9 @@ var genericOps = []opData{
{name: "VarDef", argLength: 1, aux: "Sym", typ: "Mem", symEffect: "None", zeroWidth: true}, // aux is a *gc.Node of a variable that is about to be initialized. arg0=mem, returns mem
{name: "VarKill", argLength: 1, aux: "Sym", symEffect: "None"}, // aux is a *gc.Node of a variable that is known to be dead. arg0=mem, returns mem
{name: "VarLive", argLength: 1, aux: "Sym", symEffect: "Read", zeroWidth: true}, // aux is a *gc.Node of a variable that must be kept live. arg0=mem, returns mem
{name: "KeepAlive", argLength: 2, typ: "Mem", zeroWidth: true}, // arg[0] is a value that must be kept alive until this mark. arg[1]=mem, returns mem
// TODO: what's the difference betweeen VarLive and KeepAlive?
{name: "VarLive", argLength: 1, aux: "Sym", symEffect: "Read", zeroWidth: true}, // aux is a *gc.Node of a variable that must be kept live. arg0=mem, returns mem
{name: "KeepAlive", argLength: 2, typ: "Mem", zeroWidth: true}, // arg[0] is a value that must be kept alive until this mark. arg[1]=mem, returns mem
// Ops for breaking 64-bit operations on 32-bit architectures
{name: "Int64Make", argLength: 2, typ: "UInt64"}, // arg0=hi, arg1=lo
......
......@@ -232,6 +232,11 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
gp.param = nil
c.sendq.enqueue(mysg)
goparkunlock(&c.lock, waitReasonChanSend, traceEvGoBlockSend, 3)
// Ensure the value being sent is kept alive until the
// receiver copies it out. The sudog has a pointer to the
// stack object, but sudogs aren't considered as roots of the
// stack tracer.
KeepAlive(ep)
// someone woke us up.
if mysg != gp.waiting {
......
......@@ -35,14 +35,46 @@ func TestGCInfo(t *testing.T) {
verifyGCInfo(t, "data eface", &dataEface, infoEface)
verifyGCInfo(t, "data iface", &dataIface, infoIface)
verifyGCInfo(t, "stack Ptr", new(Ptr), infoPtr)
verifyGCInfo(t, "stack ScalarPtr", new(ScalarPtr), infoScalarPtr)
verifyGCInfo(t, "stack PtrScalar", new(PtrScalar), infoPtrScalar)
verifyGCInfo(t, "stack BigStruct", new(BigStruct), infoBigStruct())
verifyGCInfo(t, "stack string", new(string), infoString)
verifyGCInfo(t, "stack slice", new([]string), infoSlice)
verifyGCInfo(t, "stack eface", new(interface{}), infoEface)
verifyGCInfo(t, "stack iface", new(Iface), infoIface)
{
var x Ptr
verifyGCInfo(t, "stack Ptr", &x, infoPtr)
runtime.KeepAlive(x)
}
{
var x ScalarPtr
verifyGCInfo(t, "stack ScalarPtr", &x, infoScalarPtr)
runtime.KeepAlive(x)
}
{
var x PtrScalar
verifyGCInfo(t, "stack PtrScalar", &x, infoPtrScalar)
runtime.KeepAlive(x)
}
{
var x BigStruct
verifyGCInfo(t, "stack BigStruct", &x, infoBigStruct())
runtime.KeepAlive(x)
}
{
var x string
verifyGCInfo(t, "stack string", &x, infoString)
runtime.KeepAlive(x)
}
{
var x []string
verifyGCInfo(t, "stack slice", &x, infoSlice)
runtime.KeepAlive(x)
}
{
var x interface{}
verifyGCInfo(t, "stack eface", &x, infoEface)
runtime.KeepAlive(x)
}
{
var x Iface
verifyGCInfo(t, "stack iface", &x, infoIface)
runtime.KeepAlive(x)
}
for i := 0; i < 10; i++ {
verifyGCInfo(t, "heap Ptr", escape(new(Ptr)), trimDead(padDead(infoPtr)))
......
......@@ -1994,7 +1994,9 @@ func reflect_gcbits(x interface{}) []byte {
return ret
}
// Returns GC type info for object p for testing.
// Returns GC type info for the pointer stored in ep for testing.
// If ep points to the stack, only static live information will be returned
// (i.e. not for objects which are only dynamically live stack objects).
func getgcmask(ep interface{}) (mask []byte) {
e := *efaceOf(&ep)
p := e.data
......@@ -2051,11 +2053,6 @@ func getgcmask(ep interface{}) (mask []byte) {
_g_ := getg()
gentraceback(_g_.m.curg.sched.pc, _g_.m.curg.sched.sp, 0, _g_.m.curg, 0, nil, 1000, getgcmaskcb, noescape(unsafe.Pointer(&frame)), 0)
if frame.fn.valid() {
// TODO: once stack objects are enabled (and their pointers
// are no longer described by the stack pointermap directly),
// tests using this will probably need fixing. We might need
// to loop through the stackobjects and if we're inside one,
// use the pointermap from that object.
locals, _, _ := getStackMap(&frame, nil, false)
if locals.n == 0 {
return
......
......@@ -15,10 +15,10 @@ type T struct {
}
func f(a T) { // ERROR "live at entry to f: a"
var e interface{}
func() { // ERROR "live at entry to f.func1: a &e"
e = a.s // ERROR "live at call to convT2Estring: a &e"
}() // ERROR "live at call to f.func1: e$"
var e interface{} // ERROR "stack object e interface \{\}$"
func() { // ERROR "live at entry to f.func1: a &e"
e = a.s // ERROR "live at call to convT2Estring: &e" "stack object a T$"
}()
// Before the fix, both a and e were live at the previous line.
_ = e
}
This diff is collapsed.
......@@ -10,7 +10,6 @@
package main
// issue 8142: lost 'addrtaken' bit on inlined variables.
// no inlining in this test, so just checking that non-inlined works.
func printnl()
......@@ -28,15 +27,15 @@ func newT40() *T40 {
}
func bad40() {
t := newT40() // ERROR "live at call to makemap: .autotmp_[0-9]+ ret$"
printnl() // ERROR "live at call to printnl: .autotmp_[0-9]+ ret$"
useT40(t) // ERROR "live at call to useT40: .autotmp_[0-9]+ ret$"
t := newT40() // ERROR "live at call to makemap: ret$" "stack object ret T40$" "stack object .autotmp_[0-9]+ map.hdr\[int\]int$"
printnl() // ERROR "live at call to printnl: ret$"
useT40(t)
}
func good40() {
ret := T40{}
ret.m = make(map[int]int, 42) // ERROR "live at call to makemap: .autotmp_[0-9]+ ret$"
ret := T40{} // ERROR "stack object ret T40$"
ret.m = make(map[int]int, 42) // ERROR "live at call to makemap: ret$" "stack object .autotmp_[0-9]+ map.hdr\[int\]int$"
t := &ret
printnl() // ERROR "live at call to printnl: .autotmp_[0-9]+ ret$"
useT40(t) // ERROR "live at call to useT40: .autotmp_[0-9]+ ret$"
printnl() // ERROR "live at call to printnl: ret$"
useT40(t)
}
......@@ -19,22 +19,22 @@ func f(uintptr) // ERROR "f assuming arg#1 is unsafe uintptr"
func g() {
var t int
f(uintptr(unsafe.Pointer(&t))) // ERROR "live at call to f: .?autotmp" "g &t does not escape"
f(uintptr(unsafe.Pointer(&t))) // ERROR "live at call to f: .?autotmp" "g &t does not escape" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
func h() {
var v int
syscall.Syscall(0, 1, uintptr(unsafe.Pointer(&v)), 2) // ERROR "live at call to Syscall: .?autotmp" "h &v does not escape"
syscall.Syscall(0, 1, uintptr(unsafe.Pointer(&v)), 2) // ERROR "live at call to Syscall: .?autotmp" "h &v does not escape" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
func i() {
var t int
p := unsafe.Pointer(&t) // ERROR "i &t does not escape"
f(uintptr(p)) // ERROR "live at call to f: .?autotmp"
f(uintptr(p)) // ERROR "live at call to f: .?autotmp" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
func j() {
var v int
p := unsafe.Pointer(&v) // ERROR "j &v does not escape"
syscall.Syscall(0, 1, uintptr(p), 2) // ERROR "live at call to Syscall: .?autotmp"
syscall.Syscall(0, 1, uintptr(p), 2) // ERROR "live at call to Syscall: .?autotmp" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
// run
// Copyright 2018 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package main
import (
"fmt"
"runtime"
)
type HeapObj [8]int64
type StkObj struct {
h *HeapObj
}
var n int
var c int = -1
func gc() {
// encourage heap object to be collected, and have its finalizer run.
runtime.GC()
runtime.GC()
runtime.GC()
n++
}
func main() {
f()
gc() // prior to stack objects, heap object is not collected until here
if c < 0 {
panic("heap object never collected")
}
if c != 1 {
panic(fmt.Sprintf("expected collection at phase 1, got phase %d", c))
}
}
func f() {
var s StkObj
s.h = new(HeapObj)
runtime.SetFinalizer(s.h, func(h *HeapObj) {
// Remember at what phase the heap object was collected.
c = n
})
g(&s)
gc()
}
func g(s *StkObj) {
gc() // heap object is still live here
runtime.KeepAlive(s)
gc() // heap object should be collected here
}
// run
// Copyright 2018 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// This test makes sure that ambiguously live arguments work correctly.
package main
import (
"runtime"
)
type HeapObj [8]int64
type StkObj struct {
h *HeapObj
}
var n int
var c int = -1
func gc() {
// encourage heap object to be collected, and have its finalizer run.
runtime.GC()
runtime.GC()
runtime.GC()
n++
}
var null StkObj
var sink *HeapObj
//go:noinline
func use(p *StkObj) {
}
//go:noinline
func f(s StkObj, b bool) {
var p *StkObj
if b {
p = &s
} else {
p = &null
}
// use is required here to prevent the conditional
// code above from being executed after the first gc() call.
use(p)
// If b==false, h should be collected here.
gc() // 0
sink = p.h
gc() // 1
sink = nil
// If b==true, h should be collected here.
gc() // 2
}
func fTrue() {
var s StkObj
s.h = new(HeapObj)
c = -1
n = 0
runtime.SetFinalizer(s.h, func(h *HeapObj) {
// Remember at what phase the heap object was collected.
c = n
})
f(s, true)
if c != 2 {
panic("bad liveness")
}
}
func fFalse() {
var s StkObj
s.h = new(HeapObj)
c = -1
n = 0
runtime.SetFinalizer(s.h, func(h *HeapObj) {
// Remember at what phase the heap object was collected.
c = n
})
f(s, false)
if c != 0 {
panic("bad liveness")
}
}
func main() {
fTrue()
fFalse()
}
......@@ -30,14 +30,14 @@ func F4(...uintptr) {} // ERROR "escaping ...uintptr"
func G() {
var t int // ERROR "moved to heap"
F1(uintptr(unsafe.Pointer(&t))) // ERROR "live at call to F1: .?autotmp" "&t escapes to heap"
F1(uintptr(unsafe.Pointer(&t))) // ERROR "live at call to F1: .?autotmp" "&t escapes to heap" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
var t2 int // ERROR "moved to heap"
F3(uintptr(unsafe.Pointer(&t2))) // ERROR "live at call to F3: .?autotmp" "&t2 escapes to heap"
F3(uintptr(unsafe.Pointer(&t2))) // ERROR "live at call to F3: .?autotmp" "&t2 escapes to heap" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
func H() {
var v int // ERROR "moved to heap"
F2(0, 1, uintptr(unsafe.Pointer(&v)), 2) // ERROR "live at call to newobject: .?autotmp" "live at call to F2: .?autotmp" "escapes to heap"
F2(0, 1, uintptr(unsafe.Pointer(&v)), 2) // ERROR "live at call to newobject: .?autotmp" "live at call to F2: .?autotmp" "escapes to heap" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
var v2 int // ERROR "moved to heap"
F4(0, 1, uintptr(unsafe.Pointer(&v2)), 2) // ERROR "live at call to newobject: .?autotmp" "live at call to F4: .?autotmp" "escapes to heap"
F4(0, 1, uintptr(unsafe.Pointer(&v2)), 2) // ERROR "live at call to newobject: .?autotmp" "live at call to F4: .?autotmp" "escapes to heap" "stack object .autotmp_[0-9]+ unsafe.Pointer$"
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment