Skip to content

Commit b3294d9

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]>
1 parent 9e277f7 commit b3294d9

File tree

1 file changed

+75
-17
lines changed

1 file changed

+75
-17
lines changed

src/cmd/compile/internal/ssa/debug.go

Lines changed: 75 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ func (state *debugState) liveness() []*BlockDebug {
456456

457457
// Build the starting state for the block from the final
458458
// state of its predecessors.
459-
startState, startValid := state.mergePredecessors(b, blockLocs)
459+
startState, startValid := state.mergePredecessors(b, blockLocs, nil)
460460
changed := false
461461
if state.loggingEnabled {
462462
state.logf("Processing %v, initial state:\n%v", b, state.stateString(state.currentState))
@@ -518,9 +518,13 @@ func (state *debugState) liveness() []*BlockDebug {
518518
}
519519

520520
// mergePredecessors takes the end state of each of b's predecessors and
521-
// intersects them to form the starting state for b. It returns that state in
522-
// the BlockDebug, and fills state.currentState with it.
523-
func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([]liveSlot, bool) {
521+
// intersects them to form the starting state for b. It puts that state in
522+
// blockLocs, and fills state.currentState with it. If convenient, it returns
523+
// a reused []liveSlot, true that represents the starting state.
524+
// If previousBlock is non-nil, it registers changes vs. that block's end
525+
// state in state.changedVars. Note that previousBlock will often not be a
526+
// predecessor.
527+
func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug, previousBlock *Block) ([]liveSlot, bool) {
524528
// Filter out back branches.
525529
var predsBuf [10]*Block
526530
preds := predsBuf[:0]
@@ -538,31 +542,68 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
538542
state.logf("Merging %v into %v\n", preds2, b)
539543
}
540544

545+
// TODO all the calls to this are overkill; only need to do this for slots that are not present in the merge.
546+
markChangedVars := func(slots []liveSlot) {
547+
for _, live := range slots {
548+
state.changedVars.add(ID(state.slotVars[live.slot]))
549+
}
550+
}
551+
541552
if len(preds) == 0 {
553+
if previousBlock != nil {
554+
// Mark everything in previous block as changed because it is not a predecessor.
555+
markChangedVars(blockLocs[previousBlock.ID].endState)
556+
}
542557
state.currentState.reset(nil)
543558
return nil, true
544559
}
545560

546561
p0 := blockLocs[preds[0].ID].endState
547562
if len(preds) == 1 {
563+
if previousBlock != nil && preds[0].ID != previousBlock.ID {
564+
// Mark everything in previous block as changed because it is not a predecessor.
565+
markChangedVars(blockLocs[previousBlock.ID].endState)
566+
}
548567
state.currentState.reset(p0)
549568
return p0, true
550569
}
551570

571+
baseID := preds[0].ID
572+
baseState := p0
573+
574+
// If previous block is not a predecessor, its location information changes at boundary with this block.
575+
previousBlockIsNotPredecessor := previousBlock != nil // If it's nil, no info to change.
576+
577+
if previousBlock != nil {
578+
// Try to use previousBlock as the base state
579+
// if possible.
580+
for _, pred := range preds[1:] {
581+
if pred.ID == previousBlock.ID {
582+
baseID = pred.ID
583+
baseState = blockLocs[pred.ID].endState
584+
previousBlockIsNotPredecessor = false
585+
break
586+
}
587+
}
588+
}
589+
552590
if state.loggingEnabled {
553-
state.logf("Starting %v with state from %v:\n%v", b, preds[0], state.blockEndStateString(blockLocs[preds[0].ID]))
591+
state.logf("Starting %v with state from b%v:\n%v", b, baseID, state.blockEndStateString(blockLocs[baseID]))
554592
}
555593

556594
slotLocs := state.currentState.slots
557-
for _, predSlot := range p0 {
595+
for _, predSlot := range baseState {
558596
slotLocs[predSlot.slot] = VarLoc{predSlot.Registers, predSlot.StackOffset}
559597
state.liveCount[predSlot.slot] = 1
560598
}
561-
for i := 1; i < len(preds); i++ {
599+
for _, pred := range preds {
600+
if pred.ID == baseID {
601+
continue
602+
}
562603
if state.loggingEnabled {
563-
state.logf("Merging in state from %v:\n%v", preds[i], state.blockEndStateString(blockLocs[preds[i].ID]))
604+
state.logf("Merging in state from %v:\n%v", pred, state.blockEndStateString(blockLocs[pred.ID]))
564605
}
565-
for _, predSlot := range blockLocs[preds[i].ID].endState {
606+
for _, predSlot := range blockLocs[pred.ID].endState {
566607
state.liveCount[predSlot.slot]++
567608
liveLoc := slotLocs[predSlot.slot]
568609
if !liveLoc.onStack() || !predSlot.onStack() || liveLoc.StackOffset != predSlot.StackOffset {
@@ -577,7 +618,7 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
577618
// final state, and reuse it if so. In principle it could match any,
578619
// but it's probably not worth checking more than the first.
579620
unchanged := true
580-
for _, predSlot := range p0 {
621+
for _, predSlot := range baseState {
581622
if state.liveCount[predSlot.slot] != len(preds) ||
582623
slotLocs[predSlot.slot].Registers != predSlot.Registers ||
583624
slotLocs[predSlot.slot].StackOffset != predSlot.StackOffset {
@@ -587,10 +628,14 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
587628
}
588629
if unchanged {
589630
if state.loggingEnabled {
590-
state.logf("After merge, %v matches %v exactly.\n", b, preds[0])
631+
state.logf("After merge, %v matches b%v exactly.\n", b, baseID)
591632
}
592-
state.currentState.reset(p0)
593-
return p0, true
633+
if previousBlockIsNotPredecessor {
634+
// Mark everything in previous block as changed because it is not a predecessor.
635+
markChangedVars(blockLocs[previousBlock.ID].endState)
636+
}
637+
state.currentState.reset(baseState)
638+
return baseState, true
594639
}
595640

596641
for reg := range state.currentState.registers {
@@ -599,7 +644,7 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
599644

600645
// A slot is live if it was seen in all predecessors, and they all had
601646
// some storage in common.
602-
for _, predSlot := range p0 {
647+
for _, predSlot := range baseState {
603648
slotLoc := slotLocs[predSlot.slot]
604649

605650
if state.liveCount[predSlot.slot] != len(preds) {
@@ -616,10 +661,15 @@ func (state *debugState) mergePredecessors(b *Block, blockLocs []*BlockDebug) ([
616661
}
617662
reg := uint8(bits.TrailingZeros64(mask))
618663
mask &^= 1 << reg
619-
620664
state.currentState.registers[reg] = append(state.currentState.registers[reg], predSlot.slot)
621665
}
622666
}
667+
668+
if previousBlockIsNotPredecessor {
669+
// Mark everything in previous block as changed because it is not a predecessor.
670+
markChangedVars(blockLocs[previousBlock.ID].endState)
671+
672+
}
623673
return nil, false
624674
}
625675

@@ -827,13 +877,19 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) {
827877
// Run through the function in program text order, building up location
828878
// lists as we go. The heavy lifting has mostly already been done.
829879

880+
var prevBlock *Block
830881
for _, b := range state.f.Blocks {
882+
state.mergePredecessors(b, blockLocs, prevBlock)
883+
884+
// Handle any differences among predecessor blocks and previous block (perhaps not a predecessor)
885+
for _, varID := range state.changedVars.contents() {
886+
state.updateVar(VarID(varID), b, BlockStart)
887+
}
888+
831889
if !blockLocs[b.ID].relevant {
832890
continue
833891
}
834892

835-
state.mergePredecessors(b, blockLocs)
836-
837893
zeroWidthPending := false
838894
apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr
839895
// expect to see values in pattern (apc)* (zerowidth|real)*
@@ -881,6 +937,8 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) {
881937
state.updateVar(VarID(varID), b, BlockEnd)
882938
}
883939
}
940+
941+
prevBlock = b
884942
}
885943

886944
if state.loggingEnabled {

0 commit comments

Comments
 (0)