[release-branch.go1.15] cmd/{compile,link}: backport fix for issue 39757

During Go 1.15 development, a fix was added to the toolchain for issue
information. The 1.15 line tables were slightly malformed in the way
that they used the DWARF "end sequence" operator, resulting in
incorrect line table info for the final instruction in the final
function of a compilation unit.

This problem was fixed in https://golang.org/cl/235739, which made it
into Go 1.15. It now appears that while the fix works OK for linux, in
certain cases it causes issues with the Darwin linker (the "address
not in any section" ld64 error reported in issue #40974).

During Go 1.16 development, the fix in https://golang.org/cl/235739
was revised so as to fix another related problem (described in issue #39757);
the newer fix does not trigger the problem in the Darwin linker however.

This CL back-ports the changes in https://golang.org/cl/239286 to the
1.15 release branch, so as to fix the Darwin linker error.

Updates #38192.
Updates #39757.
Fixes #40974.

Change-Id: I9350fec4503cd3a76b97aaea0d8aed1511662e29
Reviewed-on: https://go-review.googlesource.com/c/go/+/258422
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Than McIntosh <thanm@google.com>

4 files changed, 157 insertions, 39 deletions

diff --git a/src/cmd/internal/obj/dwarf.go b/src/cmd/internal/obj/dwarf.go
index 4118c6442c..cd1d5b8687 100644
--- a/src/cmd/internal/obj/dwarf.go
+++ b/src/cmd/internal/obj/dwarf.go
@@ -44,14 +44,12 @@ func (ctxt *Link) generateDebugLinesSymbol(s, lines *LSym) {
 		}
 	}
 
-	// Set up the debug_lines state machine.
-	// NB: This state machine is reset to this state when we've finished
-	// generating the line table. See below.
-	// TODO: Once delve can support multiple DW_LNS_end_statements, we don't have
-	// to do this.
+	// Set up the debug_lines state machine to the default values
+	// we expect at the start of a new sequence.
 	stmt := true
 	line := int64(1)
 	pc := s.Func.Text.Pc
+	var lastpc int64 // last PC written to line table, not last PC in func
 	name := ""
 	prologue, wrotePrologue := false, false
 	// Walk the progs, generating the DWARF table.
@@ -86,30 +84,32 @@ func (ctxt *Link) generateDebugLinesSymbol(s, lines *LSym) {
 
 		if line != int64(newLine) || wrote {
 			pcdelta := p.Pc - pc
+			lastpc = p.Pc
 			putpclcdelta(ctxt, dctxt, lines, uint64(pcdelta), int64(newLine)-line)
 			line, pc = int64(newLine), p.Pc
 		}
 	}
 
-	// Because these symbols will be concatenated together by the linker, we need
-	// to reset the state machine that controls the debug symbols. The fields in
-	// the state machine that need to be reset are:
-	//   file = 1
-	//   line = 1
-	//   column = 0
-	//   stmt = set in header, we assume true
-	//   basic_block = false
-	// Careful readers of the DWARF specification will note that we don't reset
-	// the address of the state machine -- but this will happen at the beginning
-	// of the NEXT block of opcodes.
-	dctxt.AddUint8(lines, dwarf.DW_LNS_set_file)
+	// Because these symbols will be concatenated together by the
+	// linker, we need to reset the state machine that controls the
+	// debug symbols. Do this using an end-of-sequence operator.
+	//
+	// Note: at one point in time, Delve did not support multiple end
+	// sequence ops within a compilation unit (bug for this:
+	// https://github.com/go-delve/delve/issues/1694), however the bug
+	// has since been fixed (Oct 2019).
+	//
+	// Issue 38192: the DWARF standard specifies that when you issue
+	// an end-sequence op, the PC value should be one past the last
+	// text address in the translation unit, so apply a delta to the
+	// text address before the end sequence op. If this isn't done,
+	// GDB will assign a line number of zero the last row in the line
+	// table, which we don't want.
+	lastlen := uint64(s.Size - (lastpc - s.Func.Text.Pc))
+	putpclcdelta(ctxt, dctxt, lines, lastlen, 0)
+	dctxt.AddUint8(lines, 0) // start extended opcode
 	dwarf.Uleb128put(dctxt, lines, 1)
-	dctxt.AddUint8(lines, dwarf.DW_LNS_advance_line)
-	dwarf.Sleb128put(dctxt, lines, int64(1-line))
-	if !stmt {
-		dctxt.AddUint8(lines, dwarf.DW_LNS_negate_stmt)
-	}
-	dctxt.AddUint8(lines, dwarf.DW_LNS_copy)
+	dctxt.AddUint8(lines, dwarf.DW_LNE_end_sequence)
 }
 
 func putpclcdelta(linkctxt *Link, dctxt dwCtxt, s *LSym, deltaPC uint64, deltaLC int64) {
diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go
index 8df03d74f1..6d330061ab 100644
--- a/src/cmd/link/internal/ld/dwarf.go
+++ b/src/cmd/link/internal/ld/dwarf.go
@@ -1264,22 +1264,6 @@ func (d *dwctxt2) writelines(unit *sym.CompilationUnit, ls loader.Sym) {
 		}
 	}
 
-	// Issue 38192: the DWARF standard specifies that when you issue
-	// an end-sequence op, the PC value should be one past the last
-	// text address in the translation unit, so apply a delta to the
-	// text address before the end sequence op. If this isn't done,
-	// GDB will assign a line number of zero the last row in the line
-	// table, which we don't want. The 1 + ptrsize amount is somewhat
-	// arbitrary, this is chosen to be consistent with the way LLVM
-	// emits its end sequence ops.
-	lsu.AddUint8(dwarf.DW_LNS_advance_pc)
-	dwarf.Uleb128put(d, lsDwsym, int64(1+d.arch.PtrSize))
-
-	// Emit an end-sequence at the end of the unit.
-	lsu.AddUint8(0) // start extended opcode
-	dwarf.Uleb128put(d, lsDwsym, 1)
-	lsu.AddUint8(dwarf.DW_LNE_end_sequence)
-
 	if d.linkctxt.HeadType == objabi.Haix {
 		saveDwsectCUSize(".debug_line", unit.Lib.Pkg, uint64(lsu.Size()-unitLengthOffset))
 	}
diff --git a/src/cmd/link/internal/ld/dwarf_test.go b/src/cmd/link/internal/ld/dwarf_test.go
index fb9c45b07d..f3dd53792a 100644
--- a/src/cmd/link/internal/ld/dwarf_test.go
+++ b/src/cmd/link/internal/ld/dwarf_test.go
@@ -1479,3 +1479,122 @@ func TestIssue38192(t *testing.T) {
 		t.Logf("row %d: A=%x F=%s L=%d\n", i, r.Address, r.File.Name, r.Line)
 	}
 }
+
+func TestIssue39757(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	if runtime.GOOS == "plan9" {
+		t.Skip("skipping on plan9; no DWARF symbol table in executables")
+	}
+
+	// In this bug the DWARF line table contents for the last couple of
+	// instructions in a function were incorrect (bad file/line). This
+	// test verifies that all of the line table rows for a function
+	// of interest have the same file (no "autogenerated").
+	//
+	// Note: the function in this test was written with an eye towards
+	// ensuring that there are no inlined routines from other packages
+	// (which could introduce other source files into the DWARF); it's
+	// possible that at some point things could evolve in the
+	// compiler/runtime in ways that aren't happening now, so this
+	// might be something to check for if it does start failing.
+
+	tmpdir, err := ioutil.TempDir("", "TestIssue38192")
+	if err != nil {
+		t.Fatalf("could not create directory: %v", err)
+	}
+	defer os.RemoveAll(tmpdir)
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("where am I? %v", err)
+	}
+	pdir := filepath.Join(wd, "testdata", "issue39757")
+	f := gobuildTestdata(t, tmpdir, pdir, DefaultOpt)
+
+	syms, err := f.Symbols()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var addr uint64
+	for _, sym := range syms {
+		if sym.Name == "main.main" {
+			addr = sym.Addr
+			break
+		}
+	}
+	if addr == 0 {
+		t.Fatal("cannot find main.main in symbols")
+	}
+
+	// Open the resulting binary and examine the DWARF it contains.
+	// Look for the function of interest ("main.main")
+	// and verify that all line table entries show the same source
+	// file.
+	dw, err := f.DWARF()
+	if err != nil {
+		t.Fatalf("error parsing DWARF: %v", err)
+	}
+	rdr := dw.Reader()
+	ex := examiner{}
+	if err := ex.populate(rdr); err != nil {
+		t.Fatalf("error reading DWARF: %v", err)
+	}
+
+	// Locate the main.main DIE
+	mains := ex.Named("main.main")
+	if len(mains) == 0 {
+		t.Fatalf("unable to locate DIE for main.main")
+	}
+	if len(mains) != 1 {
+		t.Fatalf("more than one main.main DIE")
+	}
+	maindie := mains[0]
+
+	// Collect the start/end PC for main.main
+	lowpc := maindie.Val(dwarf.AttrLowpc).(uint64)
+	highpc := maindie.Val(dwarf.AttrHighpc).(uint64)
+
+	// Now read the line table for the 'main' compilation unit.
+	mainIdx := ex.idxFromOffset(maindie.Offset)
+	cuentry := ex.Parent(mainIdx)
+	if cuentry == nil {
+		t.Fatalf("main.main DIE appears orphaned")
+	}
+	lnrdr, lerr := dw.LineReader(cuentry)
+	if lerr != nil {
+		t.Fatalf("error creating DWARF line reader: %v", err)
+	}
+	if lnrdr == nil {
+		t.Fatalf("no line table for main.main compilation unit")
+	}
+	rows := []dwarf.LineEntry{}
+	mainrows := 0
+	var lne dwarf.LineEntry
+	for {
+		err := lnrdr.Next(&lne)
+		if err == io.EOF {
+			break
+		}
+		rows = append(rows, lne)
+		if err != nil {
+			t.Fatalf("error reading next DWARF line: %v", err)
+		}
+		if lne.Address < lowpc || lne.Address > highpc {
+			continue
+		}
+		if !strings.HasSuffix(lne.File.Name, "issue39757main.go") {
+			t.Errorf("found row with file=%s (not issue39757main.go)", lne.File.Name)
+		}
+		mainrows++
+	}
+	f.Close()
+
+	// Make sure we saw a few rows.
+	if mainrows < 3 {
+		t.Errorf("not enough line table rows for main.main (got %d, wanted > 3", mainrows)
+		for i, r := range rows {
+			t.Logf("row %d: A=%x F=%s L=%d\n", i, r.Address, r.File.Name, r.Line)
+		}
+	}
+}
diff --git a/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go b/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go
new file mode 100644
index 0000000000..76e0ea1b08
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go
@@ -0,0 +1,15 @@
+// Copyright 2020 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
+
+var G int
+
+func main() {
+	if G != 101 {
+		println("not 101")
+	} else {
+		println("well now that's interesting")
+	}
+}