From: thepudds Date: Sat, 12 Jul 2025 21:01:44 +0000 (-0400) Subject: cmd/compile/internal/escape: improve DWARF .debug_line numbering for literal rewritin... X-Git-Tag: go1.25rc3~5^2~13 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=66536242fc;p=gostls13.git cmd/compile/internal/escape: improve DWARF .debug_line numbering for literal rewriting optimizations 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 Reviewed-by: David Chase Reviewed-by: Alessandro Arzilli Reviewed-by: Keith Randall --- diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index ae6941a097..6b34830b3d 100644 --- 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 857cce785f..5ca844403e 100644 --- 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 index 0000000000..40bb7b6069 --- /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 index 0000000000..fa89063299 --- /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 index 0000000000..92cacaf0d7 --- /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