]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile/internal/ssa: fix debug location gen issue with zero width ops
authorThan McIntosh <thanm@google.com>
Mon, 8 Nov 2021 18:53:55 +0000 (13:53 -0500)
committerThan McIntosh <thanm@google.com>
Thu, 11 Nov 2021 11:47:08 +0000 (11:47 +0000)
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 <unsafe.Pointer> {ctx+0} [1] : BX (ctx[unsafe.Pointer])
    v67 = ArgIntReg <unsafe.Pointer> {ctx+8} [2] : CX (ctx+8[unsafe.Pointer])
    ...
    v281 = StoreReg <unsafe.Pointer> 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 <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
src/cmd/compile/internal/ssa/debug.go

index e78eb5c0e4b6868010b104c4660a841082730f72..fed152efbab192e3a6084f2dd9a2e7a1af83cc7b 100644 (file)
@@ -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 <uintptr> {foo+0} [0] : AX (foo)
+               //   v34 = ArgIntReg <uintptr> {bar+0} [0] : BX (bar)
+               //   ...
+               //   v77 = StoreReg <unsafe.Pointer> v67 : ctx+8[unsafe.Pointer]
+               //   v78 = StoreReg <unsafe.Pointer> v68 : ctx[unsafe.Pointer]
+               //   v79 = Arg <*uint8> {args} : args[*uint8] (args[*uint8])
+               //   v80 = Arg <int> {args} [8] : args+8[int] (args+8[int])
+               //   ...
+               //   v1 = InitMem <mem>
+               //
+               // 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