cmd/compile: add loclist for removed DCL nodes

Certain return parameters which live in registers and end up pruned by
earlier SSA passes will end up with a DWARF entry but no location list.
This patch fixes this by ensuring those params have proper location
lists generated for them.

Change-Id: I4fff074e62c3010abdee85934fb286510b21c707
Reviewed-on: https://go-review.googlesource.com/c/go/+/696575
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Derek Parker <parkerderek86@gmail.com>

src/cmd/compile/internal/dwarfgen/dwarf.go

@@ -236,6 +236,8 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 	// reliably report its contents."
 	// For non-SSA-able arguments, however, the correct information
 	// is known -- they have a single home on the stack.
+	var outidx int
+	debug, _ := fn.DebugInfo.(*ssa.FuncDebug)
 	for _, n := range dcl {
 		if selected.Has(n) {
 			continue
@@ -244,6 +246,11 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 		if c == '.' || n.Type().IsUntyped() {
 			continue
 		}
+
+		// Occasionally the dcl list will contain duplicates. Add here
+		// so the `selected.Has` check above filters them out.
+		selected.Add(n)
+
 		if n.Class == ir.PPARAM && !ssa.CanSSA(n.Type()) {
 			// SSA-able args get location lists, and may move in and
 			// out of registers, so those are handled elsewhere.
@@ -256,6 +263,7 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 			decls = append(decls, n)
 			continue
 		}
+
 		typename := dwarf.InfoPrefix + types.TypeSymName(n.Type())
 		decls = append(decls, n)
 		tag := dwarf.DW_TAG_variable
@@ -288,11 +296,30 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 			DictIndex:     n.DictIndex,
 			ClosureOffset: closureOffset(n, closureVars),
 		}
-		if n.Esc() == ir.EscHeap {
+		if debug != nil && isReturnValue && n.IsOutputParamInRegisters() {
+			// Create register based location list
+			// This assumes that the register return params in the dcl list is sorted correctly and
+			// params appear in order they are defined in the function signature.
+
+			startIDs := debug.RegOutputParamStartIDs
+			if !strings.HasPrefix(n.Sym().Name, "~") {
+				// If this is not an unnamed return parameter do not pass ranges through and assume
+				// this value is live throughout the entirety of the function.
+				// TODO(derekparker): there is still work to be done here to better track the movement
+				// of these params through the body of the function, e.g. it may move registers at certain address
+				// ranges or spill to the stack.
+				startIDs = nil
+			}
+			list := createRegisterReturnParamLocList(debug.RegOutputParamRegList[outidx], debug.EntryID, startIDs)
+			outidx++
+			dvar.PutLocationList = func(listSym, startPC dwarf.Sym) {
+				debug.PutLocationList(list, base.Ctxt, listSym.(*obj.LSym), startPC.(*obj.LSym))
+			}
+		}
+		if debug != nil && n.Esc() == ir.EscHeap {
 			if n.Heapaddr == nil {
 				base.Fatalf("invalid heap allocated var without Heapaddr")
 			}
-			debug := fn.DebugInfo.(*ssa.FuncDebug)
 			list := createHeapDerefLocationList(n, debug.EntryID)
 			dvar.PutLocationList = func(listSym, startPC dwarf.Sym) {
 				debug.PutLocationList(list, base.Ctxt, listSym.(*obj.LSym), startPC.(*obj.LSym))
@@ -309,6 +336,54 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 	return decls, vars
 }
 
+// createRegisterReturnParamLocList creates a location list that specifies
+// a value is in a register for the given block ranges.
+// startIDs contains a list of SSA IDs where the value is live, and it is assumed by this function
+// that the value is live until the nearest RET instruction, or the end of the function in the case
+// of the last / only start location.
+func createRegisterReturnParamLocList(regs []int8, entryID ssa.ID, startIDs []ssa.ID) []byte {
+	var list []byte
+	ctxt := base.Ctxt
+
+	if len(startIDs) == 0 {
+		// No ranges specified, fall back to full function
+		startIDs = []ssa.ID{ssa.BlockStart.ID}
+	}
+
+	stardIDLenIdx := len(startIDs) - 1
+	// Create a location list entry for each range
+	for i, start := range startIDs {
+		var sizeIdx int
+
+		end := ssa.FuncLocalEnd.ID
+		if i == stardIDLenIdx {
+			// If this is the last entry always extend to end of function.
+			end = ssa.FuncEnd.ID
+		}
+		list, sizeIdx = ssa.SetupLocList(base.Ctxt, entryID, list, start, end)
+
+		// The actual location expression
+		for _, reg := range regs {
+			if reg < 32 {
+				list = append(list, dwarf.DW_OP_reg0+byte(reg))
+			} else {
+				list = append(list, dwarf.DW_OP_regx)
+				list = dwarf.AppendUleb128(list, uint64(reg))
+			}
+			if len(regs) > 1 {
+				list = append(list, dwarf.DW_OP_piece)
+				list = dwarf.AppendUleb128(list, uint64(ctxt.Arch.RegSize))
+			}
+		}
+
+		// Update the size for this entry
+		locSize := len(list) - sizeIdx - 2
+		ctxt.Arch.ByteOrder.PutUint16(list[sizeIdx:], uint16(locSize))
+	}
+
+	return list
+}
+
 // sortDeclsAndVars sorts the decl and dwarf var lists according to
 // parameter declaration order, so as to insure that when a subprogram
 // DIE is emitted, its parameter children appear in declaration order.

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

@@ -397,6 +397,10 @@ func NewConfig(arch string, types Types, ctxt *obj.Link, optimize, softfloat boo
 
 func (c *Config) Ctxt() *obj.Link { return c.ctxt }
 
+func (c *Config) Reg(i int8) int16                  { return c.registers[i].objNum }
+func (c *Config) IntParamReg(i abi.RegIndex) int8   { return c.intParamRegs[i] }
+func (c *Config) FloatParamReg(i abi.RegIndex) int8 { return c.floatParamRegs[i] }
+
 func (c *Config) haveByteSwap(size int64) bool {
 	switch size {
 	case 8:

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

@@ -38,7 +38,12 @@ type FuncDebug struct {
 	LocationLists [][]byte
 	// Register-resident output parameters for the function. This is filled in at
 	// SSA generation time.
-	RegOutputParams []*ir.Name
+	RegOutputParams       []*ir.Name
+	RegOutputParamRegList [][]int8
+	// RegOutputParamStartIDs contains a list of SSA IDs which represent a location where
+	// the register based return param is known to be live. It is assumed that the value is
+	// live from the ID to the closest function return location.
+	RegOutputParamStartIDs []ID
 	// Variable declarations that were removed during optimization
 	OptDcl []*ir.Name
 	// The ssa.Func.EntryID value, used to build location lists for
@@ -189,6 +194,12 @@ var FuncEnd = &Value{
 	Aux: StringToAux("FuncEnd"),
 }
 
+var FuncLocalEnd = &Value{
+	ID:  -40000,
+	Op:  OpInvalid,
+	Aux: StringToAux("FuncLocalEnd"),
+}
+
 // RegisterSet is a bitmap of registers, indexed by Register.num.
 type RegisterSet uint64
 

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

@@ -461,6 +461,15 @@ func buildssa(fn *ir.Func, worker int, isPgoHot bool) *ssa.Func {
 	var params *abi.ABIParamResultInfo
 	params = s.f.ABISelf.ABIAnalyze(fn.Type(), true)
 
+	abiRegIndexToRegister := func(reg abi.RegIndex) int8 {
+		i := s.f.ABISelf.FloatIndexFor(reg)
+		if i >= 0 { // float PR
+			return s.f.Config.FloatParamReg(abi.RegIndex(i))
+		} else {
+			return s.f.Config.IntParamReg(reg)
+		}
+	}
+
 	// The backend's stackframe pass prunes away entries from the fn's
 	// Dcl list, including PARAMOUT nodes that correspond to output
 	// params passed in registers. Walk the Dcl list and capture these
@@ -470,6 +479,16 @@ func buildssa(fn *ir.Func, worker int, isPgoHot bool) *ssa.Func {
 	for _, n := range fn.Dcl {
 		if n.Class == ir.PPARAMOUT && n.IsOutputParamInRegisters() {
 			debugInfo.RegOutputParams = append(debugInfo.RegOutputParams, n)
+			op := params.OutParam(len(debugInfo.RegOutputParamRegList))
+			debugInfo.RegOutputParamRegList = append(debugInfo.RegOutputParamRegList, make([]int8, len(op.Registers)))
+			for i, reg := range op.Registers {
+				idx := len(debugInfo.RegOutputParamRegList) - 1
+				// TODO(deparker) This is a rather large amount of conversions to get from
+				// an abi.RegIndex to a Dwarf register number. Can this be simplified?
+				abiReg := s.f.Config.Reg(abiRegIndexToRegister(reg))
+				dwarfReg := base.Ctxt.Arch.DWARFRegisters[abiReg]
+				debugInfo.RegOutputParamRegList[idx][i] = int8(dwarfReg)
+			}
 		}
 	}
 	fn.DebugInfo = &debugInfo
@@ -7282,6 +7301,7 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
 			ssa.BuildFuncDebugNoOptimized(base.Ctxt, f, base.Debug.LocationLists > 1, StackOffset, debugInfo)
 		} else {
 			ssa.BuildFuncDebug(base.Ctxt, f, base.Debug.LocationLists, StackOffset, debugInfo)
+			populateReturnValueBlockRanges(f, debugInfo)
 		}
 		bstart := s.bstart
 		idToIdx := make([]int, f.NumBlocks())
@@ -7305,6 +7325,20 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
 				return valueToProgAfter[blk.Values[nv-1].ID].Pc
 			case ssa.FuncEnd.ID:
 				return e.curfn.LSym.Size
+			case ssa.FuncLocalEnd.ID:
+				// Find the closest RET instruction to this block.
+				// This ensures that location lists are correct for functions
+				// with multiple returns.
+				blk := f.Blocks[idToIdx[b]]
+				nv := len(blk.Values)
+				pa := valueToProgAfter[blk.Values[nv-1].ID]
+				for {
+					if pa.Link == nil || pa.As == obj.ARET {
+						break
+					}
+					pa = pa.Link
+				}
+				return pa.Pc + 1
 			default:
 				return valueToProgAfter[v].Pc
 			}
@@ -8084,4 +8118,31 @@ func isStructNotSIMD(t *types.Type) bool {
 	return t.IsStruct() && !t.IsSIMD()
 }
 
+// populateReturnValueBlockRanges analyzes the SSA to find when return values
+// are assigned and creates precise block ranges for their liveness.
+func populateReturnValueBlockRanges(f *ssa.Func, debugInfo *ssa.FuncDebug) {
+	if debugInfo == nil || len(debugInfo.RegOutputParams) == 0 {
+		return
+	}
+
+	// Find assignment points for each return parameter.
+	for _, b := range f.Blocks {
+		// Check if this is a return block
+		if b.Kind != ssa.BlockRet && b.Kind != ssa.BlockRetJmp {
+			continue
+		}
+		val := b.Values[0]
+		for i := range b.Values {
+			// Not skipping these causes a panic when using the value to lookup within `valueToProgAfter`.
+			op := b.Values[i].Op
+			if op == ssa.OpArgIntReg || op == ssa.OpArgFloatReg {
+				continue
+			}
+			val = b.Values[i]
+			break
+		}
+		debugInfo.RegOutputParamStartIDs = append(debugInfo.RegOutputParamStartIDs, val.ID)
+	}
+}
+
 var BoundsCheckFunc [ssa.BoundsKindCount]*obj.LSym

src/cmd/link/dwarf_test.go

@@ -16,6 +16,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -300,16 +301,13 @@ func TestDWARFiOS(t *testing.T) {
 
 // This test ensures that variables promoted to the heap, specifically
 // function return parameters, have correct location lists generated.
-//
-// TODO(deparker): This test is intentionally limited to GOOS=="linux"
-// and scoped to net.sendFile, which was the function reported originally in
-// issue #65405. There is relevant discussion in https://go-review.googlesource.com/c/go/+/684377
-// pertaining to these limitations. There are other missing location lists which must be fixed
-// particularly in functions where `linkname` is involved.
 func TestDWARFLocationList(t *testing.T) {
-	if runtime.GOOS != "linux" {
-		t.Skip("skipping test on non-linux OS")
+	const dwarfGoLanguage = 22
+
+	if runtime.GOARCH == "386" {
+		t.Skipf("skipping on %s/386: not all unnamed return parameters have lists yet", runtime.GOOS)
 	}
+
 	testenv.MustHaveCGO(t)
 	testenv.MustHaveGoBuild(t)
 
@@ -344,6 +342,11 @@ func TestDWARFLocationList(t *testing.T) {
 	// Find the net.sendFile function and check its return parameter location list
 	reader := d.Reader()
 
+	pattern := `\w+(\.\w+)*\.func\d+`
+	re := regexp.MustCompile(pattern)
+
+	var cu *dwarf.Entry
+	var lr *dwarf.LineReader
 	for {
 		entry, err := reader.Next()
 		if err != nil {
@@ -353,14 +356,37 @@ func TestDWARFLocationList(t *testing.T) {
 			break
 		}
 
-		// Look for the net.sendFile subprogram
-		if entry.Tag == dwarf.TagSubprogram {
-			fnName, ok := entry.Val(dwarf.AttrName).(string)
-			if !ok || fnName != "net.sendFile" {
+		if entry.Tag == dwarf.TagCompileUnit {
+			if lang, _ := entry.Val(dwarf.AttrLanguage).(int64); lang != dwarfGoLanguage {
 				reader.SkipChildren()
+				continue // Skip non-Go compile units.
+			}
+			cu = entry
+			lr, err = d.LineReader(cu)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		if cu != nil && entry.Tag == dwarf.TagSubprogram {
+			fnName, ok := entry.Val(dwarf.AttrName).(string)
+			if !ok || re.MatchString(fnName) {
+				continue // Skip if no name or an anonymous function. TODO(deparker): fix loclists for anonymous functions (possible unused param).
+			}
+
+			if strings.HasPrefix(fnName, "runtime.") {
+				// Ignore runtime for now, there are a lot of runtime functions which use
+				// certain pragmas that seemingly cause the location lists to be empty such as
+				// cgo_unsafe_args and nosplit.
+				// TODO(deparker): fix loclists for runtime functions.
 				continue
 			}
 
+			fi, ok := entry.Val(dwarf.AttrDeclFile).(int64)
+			if !ok || lr.Files()[fi].Name == "<autogenerated>" {
+				continue // Function may be implemented in assembly, skip it.
+			}
+
 			for {
 				paramEntry, err := reader.Next()
 				if err != nil {
@@ -373,6 +399,15 @@ func TestDWARFLocationList(t *testing.T) {
 				if paramEntry.Tag == dwarf.TagFormalParameter {
 					paramName, _ := paramEntry.Val(dwarf.AttrName).(string)
 
+					if paramName == ".dict" {
+						continue
+					}
+
+					// Skip anonymous / blank (unused) params.
+					if strings.HasPrefix(paramName, "~p") {
+						continue
+					}
+
 					// Check if this parameter has a location attribute
 					if loc := paramEntry.Val(dwarf.AttrLocation); loc != nil {
 						switch locData := loc.(type) {