cmd/compile/internal/escape: improve DWARF .debug_line numbering for literal rewritin...

The literal rewriting optimizations to reduce user allocations
that were implemented in CL 649079 and related CLs like CL 684116
could produce .debug_line tables such that the line numbers
sometimes jumped back to a variable's declaration.

This CL adjusts the positions of the IR nodes to avoid this.

For the first test added here in i74576a.go:

  11  func main() {
  12       a := 1
  13       runtime.Breakpoint()
  14       sink = a
  15  }

Without this fix, the test reports debug lines of 12, 13, 13, 12, 14.
Note it goes backwards from 13 to a second 12.

With this fix, the test reports debug lines of 12, 13, 13, 14
without going backwards.

The test added in i74576b.go creates a slice via make with a
non-constant argument, which similarly shows debug lines going backwards
before this fix but not after. To address the slice make case, we
create a new BasicLit node to then set its position.

There were some related allocation optimizations for struct literals
such as CL 649555 during the Go 1.25 dev cycle, but at least in some
basic test cases, those optimizations did not seem to produce
debug line issues. The struct literal interface conversion test
added in i74576c.go has the same behavior before and after this change.

Finally, running 'go test ./...' in the delve repo root (4a2a6e1aeb)
seems to have many failures with go1.25rc2, but seems to pass with
this CL.

Fixes #74576
Updates #71359

Change-Id: I31faf5fe4bb352fdcd06bdc8606dbdbc4bbd65f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/687815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>

diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go
index ae6941a097477c33ec2a3bddbe990251613a83e7..6b34830b3dd5e6c8fa78327af4ec2a3bec083d98 100644 (file)
--- a/src/cmd/compile/internal/escape/escape.go
+++ b/src/cmd/compile/internal/escape/escape.go
@@ -539,9 +539,8 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
                return
        }
 
-       assignTemp := func(n ir.Node, init *ir.Nodes) {
+       assignTemp := func(pos src.XPos, n ir.Node, init *ir.Nodes) {
                // Preserve any side effects of n by assigning it to an otherwise unused temp.
-               pos := n.Pos()
                tmp := typecheck.TempAt(pos, fn, n.Type())
                init.Append(typecheck.Stmt(ir.NewDecl(pos, ir.ODCL, tmp)))
                init.Append(typecheck.Stmt(ir.NewAssignStmt(pos, tmp, n)))
@@ -575,8 +574,8 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
                                                return
                                        }
                                        // Preserve any side effects of the original expression, then replace it.
-                                       assignTemp(*r, n.PtrInit())
-                                       *r = lit
+                                       assignTemp(n.Pos(), *r, n.PtrInit())
+                                       *r = ir.NewBasicLit(n.Pos(), (*r).Type(), lit.Val())
                                }
                        }
                }
@@ -601,9 +600,9 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
                                        base.WarnfAt(n.Pos(), "rewriting OCONVIFACE value from %v (%v) to %v (%v)", conv.X, conv.X.Type(), v, v.Type())
                                }
                                // Preserve any side effects of the original expression, then replace it.
-                               assignTemp(conv.X, conv.PtrInit())
+                               assignTemp(conv.Pos(), conv.X, conv.PtrInit())
                                v := v.(*ir.BasicLit)
-                               conv.X = ir.NewBasicLit(conv.X.Pos(), conv.X.Type(), v.Val())
+                               conv.X = ir.NewBasicLit(conv.Pos(), conv.X.Type(), v.Val())
                                typecheck.Expr(conv)
                        }
                }

diff --git a/src/cmd/compile/internal/ssa/debug_lines_test.go b/src/cmd/compile/internal/ssa/debug_lines_test.go
index 857cce785f496d60a26453ab9b2658a65318d502..5ca844403e54fe5e7c2c4d3424a86a28538f4eca 100644 (file)
--- a/src/cmd/compile/internal/ssa/debug_lines_test.go
+++ b/src/cmd/compile/internal/ssa/debug_lines_test.go
@@ -115,6 +115,24 @@ func TestDebugLines_53456(t *testing.T) {
        testDebugLinesDefault(t, "-N -l", "b53456.go", "(*T).Inc", []int{15, 16, 17, 18}, true)
 }
 
+func TestDebugLines_74576(t *testing.T) {
+       tests := []struct {
+               file      string
+               wantStmts []int
+       }{
+               {"i74576a.go", []int{12, 13, 13, 14}},
+               {"i74576b.go", []int{12, 13, 13, 14}},
+               {"i74576c.go", []int{12, 13, 13, 14}},
+       }
+       t.Parallel()
+       for _, test := range tests {
+               t.Run(test.file, func(t *testing.T) {
+                       t.Parallel()
+                       testDebugLines(t, "-N -l", test.file, "main", test.wantStmts, false)
+               })
+       }
+}
+
 func compileAndDump(t *testing.T, file, function, moreGCFlags string) []byte {
        testenv.MustHaveGoBuild(t)
 
@@ -223,6 +241,9 @@ func testInlineStack(t *testing.T, file, function string, wantStacks [][]int) {
 // then verifies that the statement-marked lines in that file are the same as those in wantStmts
 // These files must all be short because this is super-fragile.
 // "go build" is run in a temporary directory that is normally deleted, unless -test.v
+//
+// TODO: the tests calling this are somewhat expensive; perhaps more tests can be marked t.Parallel,
+// or perhaps the mechanism here can be made more efficient.
 func testDebugLines(t *testing.T, gcflags, file, function string, wantStmts []int, ignoreRepeats bool) {
        dumpBytes := compileAndDump(t, file, function, gcflags)
        dump := bufio.NewScanner(bytes.NewReader(dumpBytes))

diff --git a/src/cmd/compile/internal/ssa/testdata/i74576a.go b/src/cmd/compile/internal/ssa/testdata/i74576a.go
new file mode 100644 (file)
index 0000000..40bb7b6
--- /dev/null
+++ b/src/cmd/compile/internal/ssa/testdata/i74576a.go
@@ -0,0 +1,17 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "runtime"
+)
+
+func main() {
+       a := 1
+       runtime.Breakpoint()
+       sink = a
+}
+
+var sink any

diff --git a/src/cmd/compile/internal/ssa/testdata/i74576b.go b/src/cmd/compile/internal/ssa/testdata/i74576b.go
new file mode 100644 (file)
index 0000000..fa89063
--- /dev/null
+++ b/src/cmd/compile/internal/ssa/testdata/i74576b.go
@@ -0,0 +1,15 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "runtime"
+)
+
+func main() {
+       a := 1
+       runtime.Breakpoint()
+       _ = make([]int, a)
+}

diff --git a/src/cmd/compile/internal/ssa/testdata/i74576c.go b/src/cmd/compile/internal/ssa/testdata/i74576c.go
new file mode 100644 (file)
index 0000000..92cacaf
--- /dev/null
+++ b/src/cmd/compile/internal/ssa/testdata/i74576c.go
@@ -0,0 +1,19 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "runtime"
+)
+
+func main() {
+       s := S{1, 1}
+       runtime.Breakpoint()
+       sink = s
+}
+
+type S struct{ a, b uint64 }
+
+var sink any