diff options
author | Than McIntosh <thanm@google.com> | 2021-07-01 11:22:02 -0400 |
---|---|---|
committer | Than McIntosh <thanm@google.com> | 2021-07-01 17:41:22 +0000 |
commit | ef8ae82b37657ab788f490bd757ad1b5592b952f (patch) | |
tree | f069321057b9e31ab4445c020340b49711ffe72e /src/cmd/compile/internal/ssa | |
parent | 770899f7e11e32ee8500fc033a0aad369a6a5f7b (diff) | |
download | go-ef8ae82b37657ab788f490bd757ad1b5592b952f.tar.gz go-ef8ae82b37657ab788f490bd757ad1b5592b952f.zip |
cmd/compile: fix bug in dwarf-gen var location generation
This patch fixes a bug in the SSA back end's DWARF generation code
that determines variable locations / lifetimes.
The code in question was written to handle sequences of initial
pseudo-ops (zero width instructions such as OpPhi, OpArg, etc) in a
basic block, detecting these ops at the start of a block and then
treating the values specially when emitting ranges for the variables
in those values. The logic in this code wasn't quite correct, meaning
that a flag variable wasn't being set properly to record the presence
of a block of zero-width value-bearing ops, leading to incorrect or
missing DWARF locations for register params.
Also in this patch is a tweak to some sanity-checking code intended to
catch scheduling problems with OpArg/OpPhi etc. The checks need to
allow for the possibility of an Arg op scheduled after a spill of an
incoming register param inserted by the register allocator. Example:
b1:
v13 = ArgIntReg <int> {p1+16} [2] : CX
v14 = ArgIntReg <int> {p2+16} [5] : R8
v38 = ArgIntReg <int> {p3+16} [8] : R11
v35 = ArgIntReg <int> {p1+0} [0] : AX
v15 = StoreReg <int> v35 : .autotmp_4[int]
v40 = Arg <int> {p4} [16] : p4+16[int]
v1 = InitMem <mem>
v3 = SB <uintptr> : SB
v18 = CMPQ <flags> v14 v13
NE v18 → b3 b2 (unlikely) (18)
Here the register allocator has decided to spill v35, meaning that the
OpArg v40 is no longer going to be positioned prior to all other
non-zero-width ops; this is a valid scenario and needs to be handled
properly by the debug code.
Fixes #46425.
Change-Id: I239b3ad56a9c1b8ebf68af42e1f57308293ed7e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/332269
Trust: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Diffstat (limited to 'src/cmd/compile/internal/ssa')
-rw-r--r-- | src/cmd/compile/internal/ssa/debug.go | 19 |
1 files changed, 13 insertions, 6 deletions
diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go index eaa94975ec..8e2872363b 100644 --- a/src/cmd/compile/internal/ssa/debug.go +++ b/src/cmd/compile/internal/ssa/debug.go @@ -1115,8 +1115,14 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { continue } + mustBeFirst := func(v *Value) bool { + return v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() || + v.Op == OpArgIntReg || v.Op == OpArgFloatReg + } + zeroWidthPending := false - apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr + blockPrologComplete := false // set to true at first non-zero-width op + apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr // expect to see values in pattern (apc)* (zerowidth|real)* for _, v := range b.Values { slots := state.valueNames[v.ID] @@ -1125,16 +1131,16 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { if opcodeTable[v.Op].zeroWidth { if changed { - if hasAnyArgOp(v) || v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() { + if mustBeFirst(v) || v.Op == OpArg { // These ranges begin at true beginning of block, not after first instruction - if zeroWidthPending { - panic(fmt.Errorf("Unexpected op '%s' mixed with OpArg/OpPhi/OpLoweredGetClosurePtr at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) + 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 continue } - // Other zero-width ops must wait on a "real" op. - zeroWidthPending = true } continue } @@ -1145,6 +1151,7 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { // 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) |