From: David Chase Date: Thu, 1 Nov 2018 19:26:02 +0000 (-0400) Subject: cmd/compile: for location lists, handle case where prev block is not a pred X-Git-Tag: go1.12beta1~194 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b3294d9491b898396e134bad5412d85337c37b64;p=gostls13.git 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 TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- 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 {