mirror of
https://github.com/golang/go.git
synced 2025-12-08 06:10:04 +00:00
cmd/compile: better modeling of escape across loop levels
Brief background on "why heap allocate". Things can be forced to the heap for the following reasons: 1) address published, hence lifetime unknown. 2) size unknown/too large, cannot be stack allocated 3) multiplicity unknown/too large, cannot be stack allocated 4) reachable from heap (not necessarily published) The bug here is a case of failing to enforce 4) when an object Y was reachable from a heap allocation X forced because of 3). It was found in the case of a closure allocated within a loop (X) and assigned to a variable outside the loop (multiplicity unknown) where the closure also captured a map (Y) declared outside the loop (reachable from heap). Note the variable declared outside the loop (Y) is not published, has known size, and known multiplicity (one). The only reason for heap allocation is that it was reached from a heap allocated item (X), but because that was not forced by publication, it has to be tracked by loop level, but escape-loop level was not tracked and thus a bug results. The fix is that when a heap allocation is newly discovered, use its looplevel as the minimum loop level for downstream escape flooding. Every attempt to generalize this bug to X-in-loop- references-Y-outside loop succeeded, so the fix was aimed to be general. Anywhere that loop level forces heap allocation, the loop level is tracked. This is not yet tested for all possible X and Y, but it is correctness- conservative and because it caused only one trivial regression in the escape tests, it is probably also performance-conservative. The new test checks the following: 1) in the map case, that if fn escapes, so does the map. 2) in the map case, if fn does not escape, neither does the map. 3) in the &x case, that if fn escapes, so does &x. 4) in the &x case, if fn does not escape, neither does &x. Fixes #13799. Change-Id: Ie280bef2bb86ec869c7c206789d0b68f080c3fdb Reviewed-on: https://go-review.googlesource.com/18234 Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
This commit is contained in:
parent
040932869e
commit
e779bfa5d2
4 changed files with 232 additions and 18 deletions
|
|
@ -299,12 +299,13 @@ func (l Level) guaranteedDereference() int {
|
|||
}
|
||||
|
||||
type NodeEscState struct {
|
||||
Curfn *Node
|
||||
Escflowsrc *NodeList // flow(this, src)
|
||||
Escretval *NodeList // on OCALLxxx, list of dummy return values
|
||||
Escloopdepth int32 // -1: global, 0: return variables, 1:function top level, increased inside function for every loop or label to mark scopes
|
||||
Esclevel Level
|
||||
Walkgen uint32
|
||||
Curfn *Node
|
||||
Escflowsrc *NodeList // flow(this, src)
|
||||
Escretval *NodeList // on OCALLxxx, list of dummy return values
|
||||
Escloopdepth int32 // -1: global, 0: return variables, 1:function top level, increased inside function for every loop or label to mark scopes
|
||||
Esclevel Level
|
||||
Walkgen uint32
|
||||
Maxextraloopdepth int32
|
||||
}
|
||||
|
||||
func (e *EscState) nodeEscState(n *Node) *NodeEscState {
|
||||
|
|
@ -1579,7 +1580,13 @@ func funcOutputAndInput(dst, src *Node) bool {
|
|||
src.Op == ONAME && src.Class == PPARAM && src.Name.Curfn == dst.Name.Curfn
|
||||
}
|
||||
|
||||
const NOTALOOPDEPTH = -1
|
||||
|
||||
func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
||||
escwalkBody(e, level, dst, src, NOTALOOPDEPTH)
|
||||
}
|
||||
|
||||
func escwalkBody(e *EscState, level Level, dst *Node, src *Node, extraloopdepth int32) {
|
||||
if src.Op == OLITERAL {
|
||||
return
|
||||
}
|
||||
|
|
@ -1590,16 +1597,29 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
// convergence.
|
||||
level = level.min(srcE.Esclevel)
|
||||
if level == srcE.Esclevel {
|
||||
return
|
||||
// Have we been here already with an extraloopdepth,
|
||||
// or is the extraloopdepth provided no improvement on
|
||||
// what's already been seen?
|
||||
if srcE.Maxextraloopdepth >= extraloopdepth || srcE.Escloopdepth >= extraloopdepth {
|
||||
return
|
||||
}
|
||||
srcE.Maxextraloopdepth = extraloopdepth
|
||||
}
|
||||
} else { // srcE.Walkgen < e.walkgen -- first time, reset this.
|
||||
srcE.Maxextraloopdepth = NOTALOOPDEPTH
|
||||
}
|
||||
|
||||
srcE.Walkgen = e.walkgen
|
||||
srcE.Esclevel = level
|
||||
modSrcLoopdepth := srcE.Escloopdepth
|
||||
|
||||
if extraloopdepth > modSrcLoopdepth {
|
||||
modSrcLoopdepth = extraloopdepth
|
||||
}
|
||||
|
||||
if Debug['m'] > 1 {
|
||||
fmt.Printf("escwalk: level:%d depth:%d %.*s op=%v %v(%v) scope:%v[%d]\n",
|
||||
level, e.pdepth, e.pdepth, "\t\t\t\t\t\t\t\t\t\t", Oconv(int(src.Op), 0), Nconv(src, obj.FmtShort), Jconv(src, obj.FmtShort), e.curfnSym(src), srcE.Escloopdepth)
|
||||
fmt.Printf("escwalk: level:%d depth:%d %.*s op=%v %v(%v) scope:%v[%d] extraloopdepth=%v\n",
|
||||
level, e.pdepth, e.pdepth, "\t\t\t\t\t\t\t\t\t\t", Oconv(int(src.Op), 0), Nconv(src, obj.FmtShort), Jconv(src, obj.FmtShort), e.curfnSym(src), srcE.Escloopdepth, extraloopdepth)
|
||||
}
|
||||
|
||||
e.pdepth++
|
||||
|
|
@ -1638,7 +1658,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
}
|
||||
}
|
||||
|
||||
leaks = level.int() <= 0 && level.guaranteedDereference() <= 0 && dstE.Escloopdepth < srcE.Escloopdepth
|
||||
leaks = level.int() <= 0 && level.guaranteedDereference() <= 0 && dstE.Escloopdepth < modSrcLoopdepth
|
||||
|
||||
switch src.Op {
|
||||
case ONAME:
|
||||
|
|
@ -1650,7 +1670,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
Warnl(int(src.Lineno), "leaking param content: %v", Nconv(src, obj.FmtShort))
|
||||
} else {
|
||||
Warnl(int(src.Lineno), "leaking param content: %v level=%v dst.eld=%v src.eld=%v dst=%v",
|
||||
Nconv(src, obj.FmtShort), level, dstE.Escloopdepth, srcE.Escloopdepth, Nconv(dst, obj.FmtShort))
|
||||
Nconv(src, obj.FmtShort), level, dstE.Escloopdepth, modSrcLoopdepth, Nconv(dst, obj.FmtShort))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -1660,7 +1680,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
Warnl(int(src.Lineno), "leaking param: %v", Nconv(src, obj.FmtShort))
|
||||
} else {
|
||||
Warnl(int(src.Lineno), "leaking param: %v level=%v dst.eld=%v src.eld=%v dst=%v",
|
||||
Nconv(src, obj.FmtShort), level, dstE.Escloopdepth, srcE.Escloopdepth, Nconv(dst, obj.FmtShort))
|
||||
Nconv(src, obj.FmtShort), level, dstE.Escloopdepth, modSrcLoopdepth, Nconv(dst, obj.FmtShort))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1686,15 +1706,17 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
}
|
||||
if Debug['m'] > 1 {
|
||||
Warnl(int(src.Lineno), "%v escapes to heap, level=%v, dst.eld=%v, src.eld=%v",
|
||||
Nconv(p, obj.FmtShort), level, dstE.Escloopdepth, srcE.Escloopdepth)
|
||||
Nconv(p, obj.FmtShort), level, dstE.Escloopdepth, modSrcLoopdepth)
|
||||
} else {
|
||||
Warnl(int(src.Lineno), "%v escapes to heap", Nconv(p, obj.FmtShort))
|
||||
}
|
||||
}
|
||||
escwalkBody(e, level.dec(), dst, src.Left, modSrcLoopdepth)
|
||||
extraloopdepth = modSrcLoopdepth // passes to recursive case, seems likely a no-op
|
||||
} else {
|
||||
escwalk(e, level.dec(), dst, src.Left)
|
||||
}
|
||||
|
||||
escwalk(e, level.dec(), dst, src.Left)
|
||||
|
||||
case OAPPEND:
|
||||
escwalk(e, level, dst, src.List.N)
|
||||
|
||||
|
|
@ -1704,6 +1726,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
if Debug['m'] != 0 {
|
||||
Warnl(int(src.Lineno), "%v escapes to heap", Nconv(src, obj.FmtShort))
|
||||
}
|
||||
extraloopdepth = modSrcLoopdepth
|
||||
}
|
||||
// similar to a slice arraylit and its args.
|
||||
level = level.dec()
|
||||
|
|
@ -1737,6 +1760,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
if Debug['m'] != 0 {
|
||||
Warnl(int(src.Lineno), "%v escapes to heap", Nconv(src, obj.FmtShort))
|
||||
}
|
||||
extraloopdepth = modSrcLoopdepth
|
||||
}
|
||||
|
||||
case ODOT,
|
||||
|
|
@ -1778,7 +1802,7 @@ func escwalk(e *EscState, level Level, dst *Node, src *Node) {
|
|||
recurse:
|
||||
level = level.copy()
|
||||
for ll := srcE.Escflowsrc; ll != nil; ll = ll.Next {
|
||||
escwalk(e, level, dst, ll.N)
|
||||
escwalkBody(e, level, dst, ll.N, extraloopdepth)
|
||||
}
|
||||
|
||||
e.pdepth--
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue