From e9ef931e0649563e800f0a284ad3606564a88b35 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Mon, 8 Nov 2021 13:53:55 -0500 Subject: [PATCH] cmd/compile/internal/ssa: fix debug location gen issue with zero width ops Revamp the way that buildLocationLists() handles zero-width operations, to fix a couple of problems that result in bad debug locations. The problematic scenario in this specific bug is where you have a parameter arriving in a register X, then a spill of register X to memory as the first non-zero-width instruction in the function. Example: v68 = ArgIntReg {ctx+0} [1] : BX (ctx[unsafe.Pointer]) v67 = ArgIntReg {ctx+8} [2] : CX (ctx+8[unsafe.Pointer]) ... v281 = StoreReg v67 : ctx+8[unsafe.Pointer] The existing buildLocationLists implementation effectively buffers or bundles changes from zero-width instructions until it it sees a non-zero-width instruction, but doing that in this case winds up making it look as though the parameter is live into the function in memory, not in a register. The fix for this to separate out zero-width ops into two distinct categories: those that whose lifetimes begin at block start (ex: OpArg, Phi) and those whose effects are taking place at the nearest non-zero-width instruction (ex: OpSelect0). In this patch we now handle the first category of ops in an initial pre-pass for each block, and leave the second category for the main pass through the block. See the notes on the issue below for a more detailed explanation of the failure mode. Fixes #46845. Change-Id: I27488d4c041019d5a0b897b7cf53000f63aab1cf Reviewed-on: https://go-review.googlesource.com/c/go/+/362244 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: David Chase --- src/cmd/compile/internal/ssa/debug.go | 89 +++++++++++++++++++-------- 1 file changed, 64 insertions(+), 25 deletions(-) diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go index e78eb5c0e4..fed152efba 100644 --- a/src/cmd/compile/internal/ssa/debug.go +++ b/src/cmd/compile/internal/ssa/debug.go @@ -1120,54 +1120,93 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { v.Op == OpArgIntReg || v.Op == OpArgFloatReg } + blockPrologComplete := func(v *Value) bool { + if b.ID != state.f.Entry.ID { + return !opcodeTable[v.Op].zeroWidth + } else { + return v.Op == OpInitMem + } + } + + // Examine the prolog portion of the block to process special + // zero-width ops such as Arg, Phi, LoweredGetClosurePtr (etc) + // whose lifetimes begin at the block starting point. In an + // entry block, allow for the possibility that we may see Arg + // ops that appear _after_ other non-zero-width operations. + // Example: + // + // v33 = ArgIntReg {foo+0} [0] : AX (foo) + // v34 = ArgIntReg {bar+0} [0] : BX (bar) + // ... + // v77 = StoreReg v67 : ctx+8[unsafe.Pointer] + // v78 = StoreReg v68 : ctx[unsafe.Pointer] + // v79 = Arg <*uint8> {args} : args[*uint8] (args[*uint8]) + // v80 = Arg {args} [8] : args+8[int] (args+8[int]) + // ... + // v1 = InitMem + // + // We can stop scanning the initial portion of the block when + // we either see the InitMem op (for entry blocks) or the + // first non-zero-width op (for other blocks). + for idx := 0; idx < len(b.Values); idx++ { + v := b.Values[idx] + if blockPrologComplete(v) { + break + } + // Consider only "lifetime begins at block start" ops. + if !mustBeFirst(v) && v.Op != OpArg { + continue + } + slots := state.valueNames[v.ID] + reg, _ := state.f.getHome(v.ID).(*Register) + changed := state.processValue(v, slots, reg) // changed == added to state.changedVars + if changed { + for _, varID := range state.changedVars.contents() { + state.updateVar(VarID(varID), v.Block, BlockStart) + } + state.changedVars.clear() + } + } + + // Now examine the block again, handling things other than the + // "begins at block start" lifetimes. zeroWidthPending := false - blockPrologComplete := false // set to true at first non-zero-width op - apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr + prologComplete := false // expect to see values in pattern (apc)* (zerowidth|real)* for _, v := range b.Values { + if blockPrologComplete(v) { + prologComplete = true + } slots := state.valueNames[v.ID] reg, _ := state.f.getHome(v.ID).(*Register) changed := state.processValue(v, slots, reg) // changed == added to state.changedVars if opcodeTable[v.Op].zeroWidth { + if prologComplete && mustBeFirst(v) { + panic(fmt.Errorf("Unexpected placement of op '%s' appearing after non-pseudo-op at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) + } if changed { if mustBeFirst(v) || v.Op == OpArg { - // These ranges begin at true beginning of block, not after first instruction - if blockPrologComplete && mustBeFirst(v) { - panic(fmt.Errorf("Unexpected placement of op '%s' appearing after non-pseudo-op at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) - } - apcChangedSize = len(state.changedVars.contents()) - // Other zero-width ops must wait on a "real" op. - zeroWidthPending = true + // already taken care of above continue } + zeroWidthPending = true } continue } - if !changed && !zeroWidthPending { continue } - // Not zero-width; i.e., a "real" instruction. + // Not zero-width; i.e., a "real" instruction. zeroWidthPending = false - blockPrologComplete = true - for i, varID := range state.changedVars.contents() { - if i < apcChangedSize { // buffered true start-of-block changes - state.updateVar(VarID(varID), v.Block, BlockStart) - } else { - state.updateVar(VarID(varID), v.Block, v) - } + for _, varID := range state.changedVars.contents() { + state.updateVar(VarID(varID), v.Block, v) } state.changedVars.clear() - apcChangedSize = 0 } - for i, varID := range state.changedVars.contents() { - if i < apcChangedSize { // buffered true start-of-block changes - state.updateVar(VarID(varID), b, BlockStart) - } else { - state.updateVar(VarID(varID), b, BlockEnd) - } + for _, varID := range state.changedVars.contents() { + state.updateVar(VarID(varID), b, BlockEnd) } prevBlock = b -- 2.48.1