From 6a56d0eedc182fd867c9265ca16b2d9d75c80e8c Mon Sep 17 00:00:00 2001
From: Andrei Matei <andreimatei1@gmail.com>
Date: Wed, 31 May 2023 12:58:47 -0400
Subject: [PATCH] dwarf: ignore DeclLine for function args (#3400)

---
 _fixtures/decllinetest.go     |  5 ++++-
 pkg/dwarf/reader/variables.go | 20 ++++++++++++--------
 pkg/proc/proc_test.go         |  9 ++++++++-
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/_fixtures/decllinetest.go b/_fixtures/decllinetest.go
index 60fc110f..9074d7e3 100644
--- a/_fixtures/decllinetest.go
+++ b/_fixtures/decllinetest.go
@@ -11,6 +11,9 @@ func main() {
 	f1(a, b)
 }
 
-func f1(a, b int) {
+func f1(
+	a int,
+	b int,
+) {
 	fmt.Printf("%d %d\n", a, b)
 }
diff --git a/pkg/dwarf/reader/variables.go b/pkg/dwarf/reader/variables.go
index d39f9d68..b47b7011 100644
--- a/pkg/dwarf/reader/variables.go
+++ b/pkg/dwarf/reader/variables.go
@@ -54,14 +54,18 @@ func variablesInternal(v []Variable, root *godwarf.Tree, depth int, pc uint64, l
 		}
 		return v
 	default:
-		o := 0
-		if root.Tag != dwarf.TagFormalParameter && (flags&VariablesTrustDeclLine != 0) {
-			// visibility for variables starts the line after declaration line,
-			// except for formal parameters, which are visible on the same line they
-			// are defined.
-			o = 1
-		}
-		if declLine, ok := root.Val(dwarf.AttrDeclLine).(int64); (flags&VariablesNoDeclLineCheck != 0) || !ok || line >= int(declLine)+o {
+		// Variables are considered to be visible starting on the line after the
+		// line they are declared on. Function arguments are an exception - the line
+		// they are declared on does not matter; they are considered to be
+		// visible throughout the function.
+		declLine, varHasDeclLine := root.Val(dwarf.AttrDeclLine).(int64)
+		checkDeclLine :=
+			root.Tag != dwarf.TagFormalParameter && // var is not a function argument
+				varHasDeclLine && // we know the DeclLine
+				(flags&VariablesNoDeclLineCheck == 0) // we were not explicitly instructed to ignore DeclLine
+
+		varVisible := !checkDeclLine || (line >= int(declLine)+1) // +1 because visibility starts on the line after DeclLine
+		if varVisible {
 			return append(v, Variable{root, depth})
 		}
 		return v
diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go
index 4a145591..874434c8 100644
--- a/pkg/proc/proc_test.go
+++ b/pkg/proc/proc_test.go
@@ -3612,7 +3612,11 @@ func TestDeclLine(t *testing.T) {
 		setFileBreakpoint(p, t, fixture.Source, 9)
 		setFileBreakpoint(p, t, fixture.Source, 10)
 		setFileBreakpoint(p, t, fixture.Source, 11)
-		setFileBreakpoint(p, t, fixture.Source, 14)
+		b := setFunctionBreakpoint(p, t, "main.f1")
+		if b.Line != 14 {
+			// Line 14 is hard-coded below.
+			t.Fatalf("expected \"main.f1\" breakpoint to be set on line 14, but got line: %d", b.Line)
+		}
 
 		assertNoError(grp.Continue(), t, "Continue 1")
 		if goversion.VersionAfterOrEqual(runtime.Version(), 1, 15) {
@@ -3635,6 +3639,9 @@ func TestDeclLine(t *testing.T) {
 		testDeclLineCount(t, p, 11, []string{"a", "b"})
 
 		assertNoError(grp.Continue(), t, "Continue 5")
+		// On line 14, we expect the function's arguments to be available, even
+		// though their DW_AT_decl_line declares higher line numbers. The decl_line
+		// is supposed to be ignored for the visibility of arguments.
 		testDeclLineCount(t, p, 14, []string{"a", "b"})
 	})
 }