]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: for location lists, handle case where prev block is not a pred
authorDavid Chase <drchase@google.com>
Thu, 1 Nov 2018 19:26:02 +0000 (15:26 -0400)
committerDavid Chase <drchase@google.com>
Sun, 2 Dec 2018 22:46:27 +0000 (22:46 +0000)
Before this change, location list construction would extend
from the previous (in linear order) block, even if was not a
flow predecessor.  This can cause a debugger to tell lies.

Fix accounts for this in block merging code by (crudely)
"changing" all variables live from a previous block if it
is not also a predecessor.

Fixes #28486.

Change-Id: I11336b0b969f0cd09f40f4e5f2bdfdeb02f377a4
Reviewed-on: https://go-review.googlesource.com/c/146718
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
src/cmd/compile/internal/ssa/debug.go

index b6c25f6573dffda6f32ec860a4d29af1b95e9273..7407a75c41c334c6610519b322dd650fb4c53c06 100644 (file)
@@ -456,7 +456,7 @@ func (state *debugState) liveness() []*BlockDebug {
 
                // Build the starting state for the block from the final
                // state of its predecessors.
-               startState, startValid := state.mergePredecessors(b, blockLocs)
+               startState, startValid := state.mergePredecessors(b, blockLocs, nil)
                changed := false
                if state.loggingEnabled {
                        state.logf("Processing %v, initial state:\n%v", b, state.stateString(state.currentState))
@@ -518,9 +518,13 @@ func (state *debugState) liveness() []*BlockDebug {
 }
 
 // mergePredecessors takes the end state of each of b's predecessors and
-// intersects them to form the starting state for b. It returns that state in
-// the BlockDebug, and fills state.currentState with it.
-func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([]liveSlot, bool) {
+// intersects them to form the starting state for b. It puts that state in
+// blockLocs, and fills state.currentState with it. If convenient, it returns
+// a reused []liveSlot, true that represents the starting state.
+// If previousBlock is non-nil, it registers changes vs. that block's end
+// state in state.changedVars. Note that previousBlock will often not be a
+// predecessor.
+func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug, previousBlock *Block) ([]liveSlot, bool) {
        // Filter out back branches.
        var predsBuf [10]*Block
        preds := predsBuf[:0]
@@ -538,31 +542,68 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
                state.logf("Merging %v into %v\n", preds2, b)
        }
 
+       // TODO all the calls to this are overkill; only need to do this for slots that are not present in the merge.
+       markChangedVars := func(slots []liveSlot) {
+               for _, live := range slots {
+                       state.changedVars.add(ID(state.slotVars[live.slot]))
+               }
+       }
+
        if len(preds) == 0 {
+               if previousBlock != nil {
+                       // Mark everything in previous block as changed because it is not a predecessor.
+                       markChangedVars(blockLocs[previousBlock.ID].endState)
+               }
                state.currentState.reset(nil)
                return nil, true
        }
 
        p0 := blockLocs[preds[0].ID].endState
        if len(preds) == 1 {
+               if previousBlock != nil && preds[0].ID != previousBlock.ID {
+                       // Mark everything in previous block as changed because it is not a predecessor.
+                       markChangedVars(blockLocs[previousBlock.ID].endState)
+               }
                state.currentState.reset(p0)
                return p0, true
        }
 
+       baseID := preds[0].ID
+       baseState := p0
+
+       // If previous block is not a predecessor, its location information changes at boundary with this block.
+       previousBlockIsNotPredecessor := previousBlock != nil // If it's nil, no info to change.
+
+       if previousBlock != nil {
+               // Try to use previousBlock as the base state
+               // if possible.
+               for _, pred := range preds[1:] {
+                       if pred.ID == previousBlock.ID {
+                               baseID = pred.ID
+                               baseState = blockLocs[pred.ID].endState
+                               previousBlockIsNotPredecessor = false
+                               break
+                       }
+               }
+       }
+
        if state.loggingEnabled {
-               state.logf("Starting %v with state from %v:\n%v", b, preds[0], state.blockEndStateString(blockLocs[preds[0].ID]))
+               state.logf("Starting %v with state from b%v:\n%v", b, baseID, state.blockEndStateString(blockLocs[baseID]))
        }
 
        slotLocs := state.currentState.slots
-       for _, predSlot := range p0 {
+       for _, predSlot := range baseState {
                slotLocs[predSlot.slot] = VarLoc{predSlot.Registers, predSlot.StackOffset}
                state.liveCount[predSlot.slot] = 1
        }
-       for i := 1; i < len(preds); i++ {
+       for _, pred := range preds {
+               if pred.ID == baseID {
+                       continue
+               }
                if state.loggingEnabled {
-                       state.logf("Merging in state from %v:\n%v", preds[i], state.blockEndStateString(blockLocs[preds[i].ID]))
+                       state.logf("Merging in state from %v:\n%v", pred, state.blockEndStateString(blockLocs[pred.ID]))
                }
-               for _, predSlot := range blockLocs[preds[i].ID].endState {
+               for _, predSlot := range blockLocs[pred.ID].endState {
                        state.liveCount[predSlot.slot]++
                        liveLoc := slotLocs[predSlot.slot]
                        if !liveLoc.onStack() || !predSlot.onStack() || liveLoc.StackOffset != predSlot.StackOffset {
@@ -577,7 +618,7 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
        // final state, and reuse it if so. In principle it could match any,
        // but it's probably not worth checking more than the first.
        unchanged := true
-       for _, predSlot := range p0 {
+       for _, predSlot := range baseState {
                if state.liveCount[predSlot.slot] != len(preds) ||
                        slotLocs[predSlot.slot].Registers != predSlot.Registers ||
                        slotLocs[predSlot.slot].StackOffset != predSlot.StackOffset {
@@ -587,10 +628,14 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
        }
        if unchanged {
                if state.loggingEnabled {
-                       state.logf("After merge, %v matches %v exactly.\n", b, preds[0])
+                       state.logf("After merge, %v matches b%v exactly.\n", b, baseID)
                }
-               state.currentState.reset(p0)
-               return p0, true
+               if previousBlockIsNotPredecessor {
+                       // Mark everything in previous block as changed because it is not a predecessor.
+                       markChangedVars(blockLocs[previousBlock.ID].endState)
+               }
+               state.currentState.reset(baseState)
+               return baseState, true
        }
 
        for reg := range state.currentState.registers {
@@ -599,7 +644,7 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
 
        // A slot is live if it was seen in all predecessors, and they all had
        // some storage in common.
-       for _, predSlot := range p0 {
+       for _, predSlot := range baseState {
                slotLoc := slotLocs[predSlot.slot]
 
                if state.liveCount[predSlot.slot] != len(preds) {
@@ -616,10 +661,15 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
                        }
                        reg := uint8(bits.TrailingZeros64(mask))
                        mask &^= 1 << reg
-
                        state.currentState.registers[reg] = append(state.currentState.registers[reg], predSlot.slot)
                }
        }
+
+       if previousBlockIsNotPredecessor {
+               // Mark everything in previous block as changed because it is not a predecessor.
+               markChangedVars(blockLocs[previousBlock.ID].endState)
+
+       }
        return nil, false
 }
 
@@ -827,13 +877,19 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) {
        // Run through the function in program text order, building up location
        // lists as we go. The heavy lifting has mostly already been done.
 
+       var prevBlock *Block
        for _, b := range state.f.Blocks {
+               state.mergePredecessors(b, blockLocs, prevBlock)
+
+               // Handle any differences among predecessor blocks and previous block (perhaps not a predecessor)
+               for _, varID := range state.changedVars.contents() {
+                       state.updateVar(VarID(varID), b, BlockStart)
+               }
+
                if !blockLocs[b.ID].relevant {
                        continue
                }
 
-               state.mergePredecessors(b, blockLocs)
-
                zeroWidthPending := false
                apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr
                // expect to see values in pattern (apc)* (zerowidth|real)*
@@ -881,6 +937,8 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) {
                                state.updateVar(VarID(varID), b, BlockEnd)
                        }
                }
+
+               prevBlock = b
        }
 
        if state.loggingEnabled {