dwtest: add more loclist tests Adds tests to verify changes in CL 696575: https://go-review.googlesource.com/c/go/+/696575 Updates the dwdumploc program to search for locations with liveness in mind. For example, for return parameters (especially unnamed) they will not be live until the function return (e.g. OpMakeResult) and so trying to find the location list with a PC value at the function entry will not yield anything. Change-Id: Ia7c1455e7c875bdf29fca5e2fb540cd771555635 Reviewed-on: https://go-review.googlesource.com/c/debug/+/714340 Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Keith Randall <khr@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/dwtest/dwloc_test.go b/dwtest/dwloc_test.go index 16ae923..3f79d17 100644 --- a/dwtest/dwloc_test.go +++ b/dwtest/dwloc_test.go
@@ -113,7 +113,7 @@ if err := cmd.Run(); err != nil { t.Fatalf("running 'harness -m %s -f %s': %v", exePath, fcn, err) } - return strings.TrimSpace(string(b.Bytes())) + return strings.TrimSpace(b.String()) } // gobuild is a helper to build a Go program from source code, @@ -151,6 +151,7 @@ "context" "strings" "fmt" + "net/http" ) var G int @@ -207,11 +208,109 @@ return sb.String() } +//go:noinline +func Issue65405(a int, b string) (int, error) { + http.Handle("/", http.StripPrefix("/static/", http.FileServer(http.Dir("./output")))) + return a + len(b), nil +} + +//go:noinline +func RegisterLivenessNamedRetParam() (result int) { + // This function demonstrates the register reuse issue. + // The return value 'result' should only be valid in its register + // after it's actually set, not throughout the entire function. + + // Early in the function, do some work that uses registers + x := 42 + y := 100 + z := x * y // Register allocator may use RAX here for multiplication + + // Do more work that might reuse the return register + for i := 0; i < 10; i++ { + z += i // More register usage + } + + // Only NOW do we actually set the return value + result = z // Return value is only valid in RAX from here + + // A debugger querying 'result' before this point would get + // incorrect values if the location list claims it's in RAX + // for the entire function + return result +} + +//go:noinline +func RegisterLivenessUnnamedRetParam() int { + x := 42 + y := 100 + z := x * y + + for i := 0; i < 10; i++ { + z += i // More register usage + } + + return z +} + +//go:noinline +func multiReturn() (int, int, int, string, float64) { + return 1, 2, 3, "test", 4.5 +} + +//go:noinline +func singleReturn() int { + return 42 +} + +//go:noinline +func multiReturnStmts(i int) int { + if i < 10 { + return 55 + } + i += 100 + i *= i + return i +} + +//go:noinline +func ManyArgsWithNamedReturns(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p int) (sum int, product int) { + // Use all arguments in computations + sum = a + b + c + d + e + f + g + h + product = 1 + + // More operations to ensure all args are used + temp1 := i * j + temp2 := k * l + temp3 := m * n + temp4 := o * p + + sum += temp1 + temp2 + temp3 + temp4 + + // Compute product using all arguments + product = (a + 1) * (b + 1) * (c + 1) * (d + 1) + product += (e + f) * (g + h) * (i + j) * (k + l) + product += (m + n) * (o + p) + + // Final adjustments + sum = sum * 2 + product = product / 2 + + return sum, product +} + func main() { Issue47354("poo") var d DB d.Issue46845(context.Background(), nil, func(error) {}, "foo", nil) Issue72053() + _, _ = Issue65405(42, "test") + a, b, _, _, _ := multiReturn() + f := singleReturn() + _ = a + b + f + _ = RegisterLivenessNamedRetParam() + _ = RegisterLivenessUnnamedRetParam() + _, _ = ManyArgsWithNamedReturns(1, 2, 3, 4, 5, 6, a, b, f, 1, 2, 3, 4, 5, 6, 7) + _ = multiReturnStmts(a+b) } ` @@ -243,8 +342,10 @@ 4: in-param "release" loc="{ [0: S=0 RSI] }" 5: in-param "query" loc="{ [0: S=8 R8] [1: S=8 R9] }" 6: in-param "args" loc="{ [0: S=8 addr=0x1000] [1: S=8 addr=0x1008] [2: S=8 addr=0x1010] }" -7: out-param "res" loc="<not available>" -8: out-param "err" loc="<not available>" +7: out-param "res" at RET[0] loc="addr=f98" +7: out-param "res" at RET[1] loc="addr=f98" +8: out-param "err" at RET[0] loc="addr=fa8" +8: out-param "err" at RET[1] loc="addr=fa8" `, "arm64": ` 1: in-param "db" loc="{ [0: S=0 R0] }" @@ -261,7 +362,7 @@ got := runHarness(t, harnessPath, ppath, "main."+fname) want := strings.TrimSpace(expected[runtime.GOARCH]) if got != want { - t.Errorf("failed Issue47354 arch %s:\ngot: %s\nwant: %s", + t.Errorf("failed Issue46845 arch %s:\ngot: %s\nwant: %s", runtime.GOARCH, got, want) } } @@ -270,7 +371,7 @@ testenv.NeedsGo1Point(t, 25) testenv.NeedsArch(t, "amd64") - want := "1: in-param \"a\" loc=\"{ [0: S=1 RAX] [1: S=7 addr=0x0] [2: S=8 RBX] [3: S=8 RCX] }\"\n2: out-param \"~r0\" loc=\"addr=fa8\"" + want := "1: in-param \"a\" loc=\"{ [0: S=1 RAX] [1: S=7 addr=0x0] [2: S=8 RBX] [3: S=8 RCX] }\"\n2: out-param \"~r0\" at RET[0] loc=\"addr=fa8\"" got := runHarness(t, harnessPath, ppath, "main.Address.String") if got != want { t.Errorf("failed Issue72053 arch %s:\ngot: %q\nwant: %q", @@ -300,6 +401,182 @@ } } +func testIssue65405(t *testing.T, harnessPath string, ppath string) { + // Test that function parameters have location lists + expected := map[string]string{ + "amd64": `1: in-param "a" loc="{ [0: S=0 RAX] }" +2: in-param "b" loc="{ [0: S=8 RBX] [1: S=8 RCX] }" +3: out-param "~r0" at RET[0] loc="{ [0: S=0 RAX] }" +4: out-param "~r1" at RET[0] loc="{ [0: S=8 RBX] [1: S=8 RCX] }"`, + "arm64": `1: in-param "a" loc="{ [0: S=0 R0] }" +2: in-param "b" loc="{ [0: S=8 R1] [1: S=8 R2] }" +3: out-param "~r0" at RET[0] loc="{ [0: S=0 R0] }" +4: out-param "~r1" at RET[0] loc="{ [0: S=8 R1] [1: S=8 R2] }"`, + } + fname := "Issue65405" + got := runHarness(t, harnessPath, ppath, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Errorf("failed Issue65405 arch %s:\ngot: %q\nwant: %q", + runtime.GOARCH, got, want) + } +} + +func testReturnValueRegisters(t *testing.T, harnessPath string, ppath string) { + // Test return value register assignments for multiReturn function + // Verify that return values follow ABI conventions + expected := map[string]string{ + "amd64": `1: out-param "~r0" at RET[0] loc="{ [0: S=0 RAX] }" +2: out-param "~r1" at RET[0] loc="{ [0: S=0 RBX] }" +3: out-param "~r2" at RET[0] loc="{ [0: S=0 RCX] }" +4: out-param "~r3" at RET[0] loc="{ [0: S=8 RDI] [1: S=8 RSI] }" +5: out-param "~r4" at RET[0] loc="{ [0: S=0 X0] }"`, + "arm64": `1: out-param "~r0" at RET[0] loc="{ [0: S=0 R0] }" +2: out-param "~r1" at RET[0] loc="{ [0: S=0 R1] }" +3: out-param "~r2" at RET[0] loc="{ [0: S=0 R2] }" +4: out-param "~r3" at RET[0] loc="{ [0: S=8 R3] [1: S=8 R4] }" +5: out-param "~r4" at RET[0] loc="{ [0: S=0 F0] }"`, + } + fname := "multiReturn" + got := runHarness(t, harnessPath, ppath, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Errorf("failed ReturnValueRegisters arch %s:\ngot: %q\nwant: %q", + runtime.GOARCH, got, want) + } +} + +func testRegisterLivenessNamedRetParam(t *testing.T, harnessPath, nooptHarnessPath, ppath, nooppath string) { + fname := "RegisterLivenessNamedRetParam" + expected := map[string]string{ + "amd64": `1: out-param "result" at RET[0] loc="{ [0: S=0 RAX] }"`, + "arm64": ``, + } + + // Test only optimized builds (non-optimized results are passed on the stack) + testCases := []struct { + name string + harness string + binary string + }{ + {"optimized", harnessPath, ppath}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := runHarness(t, tc.harness, tc.binary, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Fatalf("return parameter 'result' has incorrect location in %s build:\ngot:\n%s\nwant:\n%s", tc.name, got, want) + } + }) + } +} + +func testLivenessForUnnamedRetParams(t *testing.T, harnessPath, nooptHarnessPath, ppath, nooppath string) { + // This test verifies that return parameters have precise location lists. + // Return parameters should only be valid in their ABI register after assignment, + // not throughout the entire function. The location list should start from the + // assignment point, not from function entry. + + fname := "RegisterLivenessUnnamedRetParam" + expected := map[string]string{ + "amd64": `1: out-param "~r0" at RET[0] loc="{ [0: S=0 RAX] }"`, + "arm64": ``, + } + + // Test only optimized builds (non-optimized results are passed on the stack) + testCases := []struct { + name string + harness string + binary string + }{ + {"optimized", harnessPath, ppath}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := runHarness(t, tc.harness, tc.binary, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Fatalf("return parameter 'result' has incorrect location in %s build:\ngot:\n%s\nwant:\n%s", tc.name, got, want) + } + }) + } +} + +func testManyArgsWithNamedReturns(t *testing.T, harnessPath, nooptHarnessPath, ppath, nooppath string) { + fname := "ManyArgsWithNamedReturns" + expected := map[string]string{ + "amd64": `1: in-param "a" loc="{ [0: S=0 RAX] }" +2: in-param "b" loc="{ [0: S=0 RBX] }" +3: in-param "c" loc="{ [0: S=0 RCX] }" +4: in-param "d" loc="{ [0: S=0 RDI] }" +5: in-param "e" loc="{ [0: S=0 RSI] }" +6: in-param "f" loc="{ [0: S=0 R8] }" +7: in-param "g" loc="{ [0: S=0 R9] }" +8: in-param "h" loc="{ [0: S=0 R10] }" +9: in-param "i" loc="{ [0: S=0 R11] }" +10: in-param "j" loc="addr=1000" +11: in-param "k" loc="addr=1008" +12: in-param "l" loc="addr=1010" +13: in-param "m" loc="addr=1018" +14: in-param "n" loc="addr=1020" +15: in-param "o" loc="addr=1028" +16: in-param "p" loc="addr=1030" +17: out-param "sum" at RET[0] loc="{ [0: S=0 RAX] }" +18: out-param "product" at RET[0] loc="{ [0: S=0 RBX] }"`, + "arm64": ``, + } + + testCases := []struct { + name string + harness string + binary string + }{ + {"optimized", harnessPath, ppath}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := runHarness(t, tc.harness, tc.binary, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Fatalf("return parameter 'result' has incorrect location in %s build:\ngot:\n%s\nwant:\n%s", tc.name, got, want) + } + }) + } +} + +func testMultipleReturnStmts(t *testing.T, harnessPath, nooptHarnessPath, ppath, nooppath string) { + fname := "multiReturnStmts" + expected := map[string]string{ + "amd64": `1: in-param "i" loc="{ [0: S=0 RAX] }" +2: out-param "~r0" at RET[0] loc="{ [0: S=0 RAX] }" +2: out-param "~r0" at RET[1] loc="{ [0: S=0 RAX] }"`, + "arm64": ``, + } + + // Test only optimized builds (non-optimized results are passed on the stack) + testCases := []struct { + name string + harness string + binary string + }{ + {"optimized", harnessPath, ppath}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := runHarness(t, tc.harness, tc.binary, "main."+fname) + want := expected[runtime.GOARCH] + if got != want { + t.Fatalf("return parameter 'result' has incorrect location in %s build:\ngot:\n%s\nwant:\n%s", tc.name, got, want) + } + }) + } +} + func TestDwarfVariableLocations(t *testing.T) { testenv.NeedsGo1Point(t, 18) testenv.MustHaveGoBuild(t) @@ -354,4 +631,28 @@ t.Parallel() testRuntimeThrow(t, harnessPath, nooptHarnessPath, ppath) }) + t.Run("Issue65405", func(t *testing.T) { + t.Parallel() + testIssue65405(t, harnessPath, ppath) + }) + t.Run("ReturnValueRegisters", func(t *testing.T) { + t.Parallel() + testReturnValueRegisters(t, harnessPath, ppath) + }) + t.Run("RegisterLivenessNamedRetParam", func(t *testing.T) { + t.Parallel() + testRegisterLivenessNamedRetParam(t, harnessPath, nooptHarnessPath, ppath, nooppath) + }) + t.Run("RegisterLivenessUnnamedRetParam", func(t *testing.T) { + t.Parallel() + testLivenessForUnnamedRetParams(t, harnessPath, nooptHarnessPath, ppath, nooppath) + }) + t.Run("ManyArgsWithNamedReturns", func(t *testing.T) { + t.Parallel() + testManyArgsWithNamedReturns(t, harnessPath, nooptHarnessPath, ppath, nooppath) + }) + t.Run("multiReturnStmts", func(t *testing.T) { + t.Parallel() + testMultipleReturnStmts(t, harnessPath, nooptHarnessPath, ppath, nooppath) + }) }
diff --git a/dwtest/testdata/dwdumploc.go b/dwtest/testdata/dwdumploc.go index 287a25d..c49bde6 100644 --- a/dwtest/testdata/dwdumploc.go +++ b/dwtest/testdata/dwdumploc.go
@@ -40,7 +40,7 @@ "strings" "github.com/go-delve/delve/pkg/dwarf/op" - "github.com/go-delve/delve/pkg/proc" + "github.com/go-delve/delve/service/debugger" ) var verbflag = flag.Int("v", 0, "Verbose trace output level") @@ -404,16 +404,44 @@ return r, nil } -// processParams initializes a 'proc.BinaryInfo' object for the binary -// in question and then walks the formal parameters of the selected +// processParams launches the executable via debugger.Launch and gets the +// BinaryInfo object from it. It then walks the formal parameters of the selected // function, invoking the Location method on each param to read its // location expression. Results are dumped to stdout, and an error is // returned if something goes wrong. func processParams(executable string, fi *finfo) error { const _cfa = 0x1000 - bi := proc.NewBinaryInfo(runtime.GOOS, runtime.GOARCH) - if err := bi.LoadBinaryInfo(executable, 0, []string{}); err != nil { - return err + + config := &debugger.Config{ + Backend: "native", + WorkingDir: ".", + CheckGoVersion: false, + } + dbg, err := debugger.New(config, []string{executable}) + if err != nil { + return fmt.Errorf("failed to create debugger: %v", err) + } + + // Ensure we detach and kill the process when done + defer dbg.Detach(true) + + // Get the BinaryInfo from the launched process + tgt, unlock := dbg.LockTargetGroup() + bi := tgt.Selected.BinInfo() + unlock() + + // Disassemble the function to find RET instructions for return parameter verification + instructions, err := dbg.Disassemble(0, fi.dwLoPC, fi.dwHiPC) + if err != nil { + return fmt.Errorf("failed to disassemble function: %v", err) + } + + // Find all RET instructions + var retAddrs []uint64 + for _, instr := range instructions { + if instr.IsRet() { + retAddrs = append(retAddrs, instr.Loc.PC) + } } // Walk subprogram DIE's children. @@ -433,21 +461,41 @@ if e.Tag == dwarf.TagFormalParameter { isrvar = e.Val(dwarf.AttrVarParam).(bool) } - addr, pieces, _, err := bi.Location(e, dwarf.AttrLocation, fi.dwLoPC, op.DwarfRegisters{CFA: _cfa, FrameBase: _cfa}, nil) - pdump, err := pstring(addr, pieces, err) - if err != nil { - if fmt.Sprintf("%s", err) == "empty OP stack" { - pdump = "<not available>" - } else { - return fmt.Errorf("bad return from bi.Location at pc 0x%x: %q\n", fi.dwLoPC, err) - } - } - wh := "in" - if isrvar { - wh = "out" - } - fmt.Printf("%d: %s-param %q loc=%q\n", idx, wh, name, pdump) + // For unnamed return parameters, verify location at all RET instructions. Unnamed anonymous return parameters should + // only be considered live around MakeResult OPs. + if isrvar { + if len(retAddrs) == 0 { + return fmt.Errorf("return parameter %q found but no RET instructions in function", name) + } + + // Check location at each RET instruction + for retIdx, retPC := range retAddrs { + addr, pieces, _, err := bi.Location(e, dwarf.AttrLocation, retPC, op.DwarfRegisters{CFA: _cfa, FrameBase: _cfa}, nil) + pdump, err := pstring(addr, pieces, err) + if err != nil { + if fmt.Sprintf("%s", err) == "empty OP stack" { + pdump = "<not available>" + } else { + return fmt.Errorf("bad return from bi.Location at pc 0x%x: %q\n", retPC, err) + } + } + fmt.Printf("%d: out-param %q at RET[%d] loc=%q\n", idx, name, retIdx, pdump) + } + } else { + // For input parameters or named return values, use entry PC + pc := fi.dwLoPC + addr, pieces, _, err := bi.Location(e, dwarf.AttrLocation, pc, op.DwarfRegisters{CFA: _cfa, FrameBase: _cfa}, nil) + pdump, err := pstring(addr, pieces, err) + if err != nil { + if fmt.Sprintf("%s", err) == "empty OP stack" { + pdump = "<not available>" + } else { + return fmt.Errorf("bad return from bi.Location at pc 0x%x: %q\n", fi.dwLoPC, err) + } + } + fmt.Printf("%d: in-param %q loc=%q\n", idx, name, pdump) + } } return nil
diff --git a/dwtest/testdata/go.mod.txt b/dwtest/testdata/go.mod.txt index f93dbe8..cbd1214 100644 --- a/dwtest/testdata/go.mod.txt +++ b/dwtest/testdata/go.mod.txt
@@ -1,13 +1,16 @@ module dwdumploc -go 1.21 +go 1.23.1 -require github.com/go-delve/delve v1.24.1-0.20250303164244-e6e7aeb66705 +require github.com/go-delve/delve v1.25.2 require ( github.com/cilium/ebpf v0.11.0 // indirect github.com/hashicorp/golang-lru v1.0.2 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect golang.org/x/arch v0.11.0 // indirect golang.org/x/exp v0.0.0-20230224173230-c95f2b4c22f2 // indirect golang.org/x/sys v0.26.0 // indirect + golang.org/x/telemetry v0.0.0-20241106142447-58a1122356f5 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect )
diff --git a/dwtest/testdata/go.sum.txt b/dwtest/testdata/go.sum.txt index 5692479..84b37d6 100644 --- a/dwtest/testdata/go.sum.txt +++ b/dwtest/testdata/go.sum.txt
@@ -1,9 +1,11 @@ github.com/cilium/ebpf v0.11.0 h1:V8gS/bTCCjX9uUnkUFUpPsksM8n1lXBAvHcpiFk1X2Y= github.com/cilium/ebpf v0.11.0/go.mod h1:WE7CZAnqOL2RouJ4f1uyNhqr2P4CCvXFIqdRDUgWsVs= +github.com/creack/pty v1.1.20 h1:VIPb/a2s17qNeQgDnkfZC35RScx+blkKF8GV68n80J4= +github.com/creack/pty v1.1.20/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA= github.com/frankban/quicktest v1.14.5/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= -github.com/go-delve/delve v1.24.1-0.20250303164244-e6e7aeb66705 h1:VWoXnrDBGu171DJk2Xh+0KXz73IlwtKS4fhCunevDW0= -github.com/go-delve/delve v1.24.1-0.20250303164244-e6e7aeb66705/go.mod h1:kJk12wo6PqzWknTP6M+Pg3/CrNhFMZvNq1iHESKkhv8= +github.com/go-delve/delve v1.25.2 h1:EI6EIWGKUEC7OVE5nfG2eQSv5xEgCRxO1+REB7FKCtE= +github.com/go-delve/delve v1.25.2/go.mod h1:sBjdpmDVpQd8nIMFldtqJZkk0RpGXrf8AAp5HeRi0CM= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c= @@ -20,7 +22,12 @@ golang.org/x/arch v0.11.0/go.mod h1:FEVrYAQjsQXMVJ1nsMoVVXPZg6p2JE2mx8psSWTDQys= golang.org/x/exp v0.0.0-20230224173230-c95f2b4c22f2 h1:Jvc7gsqn21cJHCmAWx0LiimpP18LZmUxkT5Mp7EZ1mI= golang.org/x/exp v0.0.0-20230224173230-c95f2b4c22f2/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/telemetry v0.0.0-20241106142447-58a1122356f5 h1:TCDqnvbBsFapViksHcHySl/sW4+rTGNIAoJJesHRuMM= +golang.org/x/telemetry v0.0.0-20241106142447-58a1122356f5/go.mod h1:8nZWdGp9pq73ZI//QJyckMQab3yq7hoWi7SI0UIusVI= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=