cmd/compile: fix loclist for heap return vars without optimizations

When compiling without optimizations certain variables such as
return params end up missing location lists.

Fixes #65405

Change-Id: Id4ec6b1ab6681fd77b8fefb47a4ec05060c128ef
GitHub-Last-Rev: 5ab6a5398162119dd0cd5325f4239b4559b030bd
GitHub-Pull-Request: golang/go#74398
Reviewed-on: https://go-review.googlesource.com/c/go/+/684377
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>

5 files changed, 165 insertions, 15 deletions

diff --git a/src/cmd/compile/internal/dwarfgen/dwarf.go b/src/cmd/compile/internal/dwarfgen/dwarf.go
index fa13f07fdf..af2b191010 100644
--- a/src/cmd/compile/internal/dwarfgen/dwarf.go
+++ b/src/cmd/compile/internal/dwarfgen/dwarf.go
@@ -248,11 +248,6 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 		if n.Class == ir.PPARAM || n.Class == ir.PPARAMOUT {
 			tag = dwarf.DW_TAG_formal_parameter
 		}
-		if n.Esc() == ir.EscHeap {
-			// The variable in question has been promoted to the heap.
-			// Its address is in n.Heapaddr.
-			// TODO(thanm): generate a better location expression
-		}
 		inlIndex := 0
 		if base.Flag.GenDwarfInl > 1 {
 			if n.InlFormal() || n.InlLocal() {
@@ -263,7 +258,7 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 			}
 		}
 		declpos := base.Ctxt.InnermostPos(n.Pos())
-		vars = append(vars, &dwarf.Var{
+		dvar := &dwarf.Var{
 			Name:          n.Sym().Name,
 			IsReturnValue: isReturnValue,
 			Tag:           tag,
@@ -277,8 +272,19 @@ func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *ir.Func, apDecls []*ir
 			ChildIndex:    -1,
 			DictIndex:     n.DictIndex,
 			ClosureOffset: closureOffset(n, closureVars),
-		})
-		// Record go type of to insure that it gets emitted by the linker.
+		}
+		if n.Esc() == ir.EscHeap {
+			if n.Heapaddr == nil {
+				base.Fatalf("invalid heap allocated var without Heapaddr")
+			}
+			debug := fn.DebugInfo.(*ssa.FuncDebug)
+			list := createHeapDerefLocationList(n, fnsym, debug.EntryID, ssa.FuncEnd.ID)
+			dvar.PutLocationList = func(listSym, startPC dwarf.Sym) {
+				debug.PutLocationList(list, base.Ctxt, listSym.(*obj.LSym), startPC.(*obj.LSym))
+			}
+		}
+		vars = append(vars, dvar)
+		// Record go type to ensure that it gets emitted by the linker.
 		fnsym.Func().RecordAutoType(reflectdata.TypeLinksym(n.Type()))
 	}
 
@@ -550,6 +556,29 @@ func createComplexVar(fnsym *obj.LSym, fn *ir.Func, varID ssa.VarID, closureVars
 	return dvar
 }
 
+// createHeapDerefLocationList creates a location list for a heap-escaped variable
+// that describes "dereference pointer at stack offset"
+func createHeapDerefLocationList(n *ir.Name, fnsym *obj.LSym, entryID, prologEndID ssa.ID) []byte {
+	// Get the stack offset where the heap pointer is stored
+	heapPtrOffset := n.Heapaddr.FrameOffset()
+	if base.Ctxt.Arch.FixedFrameSize == 0 {
+		heapPtrOffset -= int64(types.PtrSize)
+	}
+	if buildcfg.FramePointerEnabled {
+		heapPtrOffset -= int64(types.PtrSize)
+	}
+
+	// Create a location expression: DW_OP_fbreg <offset> DW_OP_deref
+	var locExpr []byte
+	var sizeIdx int
+	locExpr, sizeIdx = ssa.SetupLocList(base.Ctxt, entryID, locExpr, ssa.BlockStart.ID, ssa.FuncEnd.ID)
+	locExpr = append(locExpr, dwarf.DW_OP_fbreg)
+	locExpr = dwarf.AppendSleb128(locExpr, heapPtrOffset)
+	locExpr = append(locExpr, dwarf.DW_OP_deref)
+	base.Ctxt.Arch.ByteOrder.PutUint16(locExpr[sizeIdx:], uint16(len(locExpr)-sizeIdx-2))
+	return locExpr
+}
+
 // RecordFlags records the specified command-line flags to be placed
 // in the DWARF info.
 func RecordFlags(flags ...string) {
diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go
index aa503eda87..e92b37fb7b 100644
--- a/src/cmd/compile/internal/ssa/debug.go
+++ b/src/cmd/compile/internal/ssa/debug.go
@@ -41,6 +41,9 @@ type FuncDebug struct {
 	RegOutputParams []*ir.Name
 	// Variable declarations that were removed during optimization
 	OptDcl []*ir.Name
+	// The ssa.Func.EntryID value, used to build location lists for
+	// return values promoted to heap in later DWARF generation.
+	EntryID ID
 
 	// Filled in by the user. Translates Block and Value ID to PC.
 	//
@@ -1645,13 +1648,13 @@ func readPtr(ctxt *obj.Link, buf []byte) uint64 {
 
 }
 
-// setupLocList creates the initial portion of a location list for a
+// SetupLocList creates the initial portion of a location list for a
 // user variable. It emits the encoded start/end of the range and a
 // placeholder for the size. Return value is the new list plus the
 // slot in the list holding the size (to be updated later).
-func setupLocList(ctxt *obj.Link, f *Func, list []byte, st, en ID) ([]byte, int) {
-	start, startOK := encodeValue(ctxt, f.Entry.ID, st)
-	end, endOK := encodeValue(ctxt, f.Entry.ID, en)
+func SetupLocList(ctxt *obj.Link, entryID ID, list []byte, st, en ID) ([]byte, int) {
+	start, startOK := encodeValue(ctxt, entryID, st)
+	end, endOK := encodeValue(ctxt, entryID, en)
 	if !startOK || !endOK {
 		// This could happen if someone writes a function that uses
 		// >65K values on a 32-bit platform. Hopefully a degraded debugging
@@ -1800,7 +1803,6 @@ func isNamedRegParam(p abi.ABIParamAssignment) bool {
 // appropriate for the ".closureptr" compiler-synthesized variable
 // needed by the debugger for range func bodies.
 func BuildFuncDebugNoOptimized(ctxt *obj.Link, f *Func, loggingEnabled bool, stackOffset func(LocalSlot) int32, rval *FuncDebug) {
-
 	needCloCtx := f.CloSlot != nil
 	pri := f.ABISelf.ABIAnalyzeFuncType(f.Type)
 
@@ -1911,7 +1913,7 @@ func BuildFuncDebugNoOptimized(ctxt *obj.Link, f *Func, loggingEnabled bool, sta
 		// Param is arriving in one or more registers. We need a 2-element
 		// location expression for it. First entry in location list
 		// will correspond to lifetime in input registers.
-		list, sizeIdx := setupLocList(ctxt, f, rval.LocationLists[pidx],
+		list, sizeIdx := SetupLocList(ctxt, f.Entry.ID, rval.LocationLists[pidx],
 			BlockStart.ID, afterPrologVal)
 		if list == nil {
 			pidx++
@@ -1961,7 +1963,7 @@ func BuildFuncDebugNoOptimized(ctxt *obj.Link, f *Func, loggingEnabled bool, sta
 
 		// Second entry in the location list will be the stack home
 		// of the param, once it has been spilled.  Emit that now.
-		list, sizeIdx = setupLocList(ctxt, f, list,
+		list, sizeIdx = SetupLocList(ctxt, f.Entry.ID, list,
 			afterPrologVal, FuncEnd.ID)
 		if list == nil {
 			pidx++
diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index e241e9b9bc..6ca299cac5 100644
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -6960,6 +6960,9 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
 	if base.Ctxt.Flag_locationlists {
 		var debugInfo *ssa.FuncDebug
 		debugInfo = e.curfn.DebugInfo.(*ssa.FuncDebug)
+		// Save off entry ID in case we need it later for DWARF generation
+		// for return values promoted to the heap.
+		debugInfo.EntryID = f.Entry.ID
 		if e.curfn.ABI == obj.ABIInternal && base.Flag.N != 0 {
 			ssa.BuildFuncDebugNoOptimized(base.Ctxt, f, base.Debug.LocationLists > 1, StackOffset, debugInfo)
 		} else {
diff --git a/src/cmd/link/dwarf_test.go b/src/cmd/link/dwarf_test.go
index 68849d7db9..4305354d16 100644
--- a/src/cmd/link/dwarf_test.go
+++ b/src/cmd/link/dwarf_test.go
@@ -256,3 +256,111 @@ func TestDWARFiOS(t *testing.T) {
 		testDWARF(t, "c-archive", true, cc, "CGO_ENABLED=1", "GOOS=ios", "GOARCH=arm64")
 	})
 }
+
+func TestDWARFLocationList(t *testing.T) {
+	testenv.MustHaveCGO(t)
+	testenv.MustHaveGoBuild(t)
+
+	if !platform.ExecutableHasDWARF(runtime.GOOS, runtime.GOARCH) {
+		t.Skipf("skipping on %s/%s: no DWARF symbol table in executables", runtime.GOOS, runtime.GOARCH)
+	}
+
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	exe := filepath.Join(tmpDir, "issue65405.exe")
+	dir := "./testdata/dwarf/issue65405"
+
+	cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-toolexec", os.Args[0], "-gcflags=all=-N -l", "-o", exe, dir)
+	cmd.Env = append(os.Environ(), "CGO_CFLAGS=")
+	cmd.Env = append(cmd.Env, "LINK_TEST_TOOLEXEC=1")
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("go build -o %v %v: %v\n%s", exe, dir, err, out)
+	}
+
+	f, err := objfile.Open(exe)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	d, err := f.DWARF()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Find the net.sendFile function and check its return parameter location list
+	reader := d.Reader()
+
+	for {
+		entry, err := reader.Next()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if entry == nil {
+			break
+		}
+
+		// Look for the net.sendFile subprogram
+		if entry.Tag == dwarf.TagSubprogram {
+			fnName, ok := entry.Val(dwarf.AttrName).(string)
+			if !ok {
+				continue
+			}
+			if strings.Contains(fnName, ".eq") || // Ignore autogenerated equality funcs
+				strings.HasPrefix(fnName, "internal/") || // Ignore internal/runtime package TODO(deparker): Fix these too (likely same issue as other ignored packages below).
+				strings.HasPrefix(fnName, "runtime.") || // Ignore runtime package which contain funcs implemented in assembly or exposed through linkname which seems to not generate location lists correctly (most likely linkname causing this). TODO(deparker) Fix these too.
+				strings.HasPrefix(fnName, "reflect.") || // Ignore reflect package. TODO(deparker) Fix these too.
+				strings.HasPrefix(fnName, "time.") { // Ignore funcs in time package which are exposed through linkname and seem to not generate location lists correctly TODO(deparker) Fix these too.
+				continue
+			}
+			if fnName == "syscall.compileCallback" || fnName == "maps.clone" {
+				continue // Ignore for now, possibly caused by linkname usage. TODO(deparker) Fix this too.
+			}
+			if runtime.GOOS == "windows" && strings.HasPrefix(fnName, "syscall.") {
+				continue // Ignore, caused by linkname usage. TODO(deparker) Fix these too.
+			}
+
+			for {
+				paramEntry, err := reader.Next()
+				if err != nil {
+					t.Fatal(err)
+				}
+				if paramEntry == nil || paramEntry.Tag == 0 {
+					break
+				}
+
+				if paramEntry.Tag == dwarf.TagFormalParameter {
+					paramName, hasName := paramEntry.Val(dwarf.AttrName).(string)
+					if !hasName {
+						continue
+					}
+					if paramName[0] == '~' {
+						continue
+					}
+					// Check if this parameter has a location attribute
+					if loc := paramEntry.Val(dwarf.AttrLocation); loc != nil {
+						switch locData := loc.(type) {
+						case []byte:
+							if len(locData) == 0 {
+								t.Errorf("%s return parameter %q has empty location list", fnName, paramName)
+								return
+							}
+						case int64:
+							// Location list offset - this means it has a location list
+							if locData == 0 {
+								t.Errorf("%s return parameter %q has zero location list offset", fnName, paramName)
+								return
+							}
+						default:
+							t.Errorf("%s return parameter %q has unexpected location type %T: %v", fnName, paramName, locData, locData)
+						}
+					} else {
+						t.Errorf("%s return parameter %q has no location attribute", fnName, paramName)
+					}
+				}
+			}
+		}
+	}
+}
diff --git a/src/cmd/link/testdata/dwarf/issue65405/main.go b/src/cmd/link/testdata/dwarf/issue65405/main.go
new file mode 100644
index 0000000000..f76e464b23
--- /dev/null
+++ b/src/cmd/link/testdata/dwarf/issue65405/main.go
@@ -0,0 +1,8 @@
+package main
+
+import "net/http"
+
+func main() {
+	http.Handle("/", http.StripPrefix("/static/", http.FileServer(http.Dir("./output"))))
+	http.ListenAndServe(":8000", nil)
+}