cmd/compile: for location lists, handle case where prev block is not a pred

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>

1 files changed, 75 insertions, 17 deletions

diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go
index b6c25f6573..7407a75c41 100644
--- a/src/cmd/compile/internal/ssa/debug.go
+++ b/src/cmd/compile/internal/ssa/debug.go
@@ -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 {