pkg/proc: fix for file reference handling with DWARF 5 compilation units (#2327)

Add a helper method for collecting line table file references that
does the correct thing for DWARF 5 vs DWARF 4 (in the latter case you
have an implicit 0 entry which is the comp dir, whereas in the former
case you do not). This is to avoid out-of-bounds errors when examining
the file table section of a DWARF 5 compilation unit's line table.

Included is a new linux/amd-only test that includes a precompiled C
object file with a DWARF-5 section that triggers the bug in question.

Fixes #2319

Documentation/backend_test_health.md

@@ -1,19 +1,19 @@
 Tests skipped by each supported backend:
 
-* 386 skipped = 2.1% (3/145)
+* 386 skipped = 2.1% (3/146)
 	* 1 broken
 	* 2 broken - cgo stacktraces
-* arm64 skipped = 2.1% (3/145)
+* arm64 skipped = 2.1% (3/146)
 	* 2 broken
 	* 1 broken - global variable symbolication
-* darwin/lldb skipped = 0.69% (1/145)
+* darwin/lldb skipped = 0.68% (1/146)
 	* 1 upstream issue
-* freebsd skipped = 7.6% (11/145)
+* freebsd skipped = 7.5% (11/146)
 	* 11 broken
-* linux/386/pie skipped = 0.69% (1/145)
+* linux/386/pie skipped = 0.68% (1/146)
 	* 1 broken
-* pie skipped = 0.69% (1/145)
+* pie skipped = 0.68% (1/146)
 	* 1 upstream issue - https://github.com/golang/go/issues/29322
-* windows skipped = 1.4% (2/145)
+* windows skipped = 1.4% (2/146)
 	* 1 broken
 	* 1 upstream issue

_fixtures/issue2319/README.txt

@@ -0,0 +1,57 @@
+
+Note:
+-----
+
+cfile-linux-amd64.syso was generated from the C source file that appears below.
+
+Build with Clang version 10:
+
+ $ clang-10 -O -gdwarf-5 -c cfile.c -o cfile.syso
+
+The DWARF of interest is for the function "qtop". Triggering the bug
+requires that the source file in question appears as the first item
+in the DWARF line table file section, e.g.
+
+$ llvm-dwarfdump-10 --debug-line cfile.syso
+....
+standard_opcode_lengths[DW_LNS_set_epilogue_begin] = 0
+standard_opcode_lengths[DW_LNS_set_isa] = 1
+include_directories[  0] = "/ssd2/go1/src/tmp/dlvbug"
+file_names[  0]:
+           name: "cfile.c"
+      dir_index: 0
+   md5_checksum: ...
+
+
+// ------------------------begin source code for cfile.c----------------
+
+#include <stdlib.h>
+#include <string.h>
+
+int glob = 99;
+
+inline int qleaf(int lx, int ly, int *lv)
+{
+  lv[lx&3] += 3;
+  return lv[ly&3];
+}
+
+int qmid(int mx, int my, int *lv, int *mv)
+{
+  mv[mx&3] += qleaf(mx, my, lv);
+  return mv[my&3];
+}
+
+int qtop(int mx, int my)
+{
+  int mv[64], lv[66], n = (mx < 64 ? 64 : mx);
+
+  memset(&mv[0], 9, sizeof(mv));
+  memset(&lv[0], 11, sizeof(mv));
+  return qmid(mx, my, lv, mv) + qleaf(mx, my, lv);
+}
+
+void Cfunc(int x) {
+  glob += qtop(x, 43);
+}
+

_fixtures/issue2319/asm-linux-amd64.s

@@ -0,0 +1,5 @@
+
+#include "textflag.h"
+
+TEXT ·cfunc(SB),$0
+        JMP Cfunc(SB)

_fixtures/issue2319/main.go

@@ -0,0 +1,7 @@
+package main
+
+func cfunc()
+
+func main() {
+	cfunc()
+}

pkg/proc/bininfo.go

@@ -1911,17 +1911,12 @@ func (bi *BinaryInfo) loadDebugInfoMapsInlinedCalls(ctxt *loadDebugInfoMapsConte
 				reader.SkipChildren()
 				continue
 			}
-			if cu.lineInfo == nil {
+			callfile, cferr := cu.filePath(int(callfileidx), entry)
-				bi.logger.Warnf("reading debug_info: inlined call on a compilation unit without debug_line section at %#x", entry.Offset)
+			if cferr != nil {
+				bi.logger.Warnf("%v", cferr)
 				reader.SkipChildren()
 				continue
 			}
-			if int(callfileidx-1) >= len(cu.lineInfo.FileNames) {
-				bi.logger.Warnf("reading debug_info: CallFile (%d) of inlined call does not exist in compile unit file table at %#x", callfileidx, entry.Offset)
-				reader.SkipChildren()
-				continue
-			}
-			callfile := cu.lineInfo.FileNames[callfileidx-1].Path
 
 			fn.InlinedCalls = append(fn.InlinedCalls, InlinedCall{
 				cu:     cu,
@@ -2082,3 +2077,25 @@ func (bi *BinaryInfo) ListPackagesBuildInfo(includeFiles bool) []*PackageBuildIn
 	sort.Slice(r, func(i, j int) bool { return r[i].ImportPath < r[j].ImportPath })
 	return r
 }
+
+// cuFilePath takes a compilation unit "cu" and a file index reference
+// "fileidx" and returns the corresponding file name entry from the
+// DWARF line table associated with the unit; "entry" is the offset of
+// the attribute where the file reference originated, for logging
+// purposes. Return value is the file string and an error value; error
+// will be non-nil if the file could not be recovered, perhaps due to
+// malformed DWARF.
+func (cu *compileUnit) filePath(fileidx int, entry *dwarf.Entry) (string, error) {
+	if cu.lineInfo == nil {
+		return "", fmt.Errorf("reading debug_info: file reference within a compilation unit without debug_line section at %#x", entry.Offset)
+	}
+	// File numbering is slightly different before and after DWARF 5;
+	// account for this here. See section 6.2.4 of the DWARF 5 spec.
+	if cu.Version < 5 {
+		fileidx--
+	}
+	if fileidx < 0 || fileidx >= len(cu.lineInfo.FileNames) {
+		return "", fmt.Errorf("reading debug_info: file index (%d) out of range in compile unit file table at %#x", fileidx, entry.Offset)
+	}
+	return cu.lineInfo.FileNames[fileidx].Path, nil
+}

pkg/proc/proc_test.go

@@ -4965,3 +4965,31 @@ func TestStepOutPreservesGoroutine(t *testing.T) {
 		}
 	})
 }
+
+func TestIssue2319(t *testing.T) {
+	// Check to make sure we don't crash on startup when the target is
+	// a binary with a mix of DWARF-5 C++ compilation units and
+	// DWARF-4 Go compilation units.
+
+	// Require CGO, since we need to use the external linker for this test.
+	protest.MustHaveCgo(t)
+
+	// The test fixture uses linux/amd64 assembly and a *.syso file
+	// that is linux/amd64, so skip for other architectures.
+	if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
+		t.Skipf("skipping since not linux/amd64")
+	}
+
+	// Skip unless on 1.14 or later. The test fixture uses a *.syso
+	// file, which in 1.13 is not loaded unless we're in internal
+	// linking mode (we need external linking here).
+	if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) {
+		t.Skip("test contains fixture that is specific to go 1.14+")
+	}
+
+	fixture := protest.BuildFixture("issue2319/", protest.BuildModeExternalLinker)
+
+	// Load up the binary and make sure there are no crashes.
+	bi := proc.NewBinaryInfo("linux", "amd64")
+	assertNoError(bi.LoadBinaryInfo(fixture.Path, 0, nil), t, "LoadBinaryInfo")
+}

pkg/proc/stack.go

@@ -377,7 +377,9 @@ func (it *stackIterator) appendInlineCalls(frames []Stackframe, frame Stackframe
 		if !okname || !okfileidx || !okline {
 			break
 		}
-		if fileidx-1 < 0 || fileidx-1 >= int64(len(frame.Current.Fn.cu.lineInfo.FileNames)) {
+		var e *dwarf.Entry
+		filepath, fileErr := frame.Current.Fn.cu.filePath(int(fileidx), e)
+		if fileErr != nil {
 			break
 		}
 
@@ -400,7 +402,7 @@ func (it *stackIterator) appendInlineCalls(frames []Stackframe, frame Stackframe
 			lastpc:      frame.lastpc,
 		})
 
-		frame.Call.File = frame.Current.Fn.cu.lineInfo.FileNames[fileidx-1].Path
+		frame.Call.File = filepath
 		frame.Call.Line = int(line)
 	}
 

pkg/proc/test/support.go

@@ -64,7 +64,7 @@ func FindFixturesDir() string {
 type BuildFlags uint32
 
 const (
-	// LinkStrip enables '-ldflas="-s"'.
+	// LinkStrip enables '-ldflags="-s"'.
 	LinkStrip BuildFlags = 1 << iota
 	// EnableCGOOptimization will build CGO code with optimizations.
 	EnableCGOOptimization
@@ -76,6 +76,7 @@ const (
 	EnableDWZCompression
 	BuildModePIE
 	BuildModePlugin
+	BuildModeExternalLinker
 	AllNonOptimized
 )
 
@@ -141,6 +142,9 @@ func BuildFixture(name string, flags BuildFlags) Fixture {
 	if flags&BuildModePlugin != 0 {
 		buildFlags = append(buildFlags, "-buildmode=plugin")
 	}
+	if flags&BuildModeExternalLinker != 0 {
+		buildFlags = append(buildFlags, "-ldflags=-linkmode=external")
+	}
 	if ver.IsDevel() || ver.AfterOrEqual(goversion.GoVersion{Major: 1, Minor: 11, Rev: -1}) {
 		if flags&EnableDWZCompression != 0 {
 			buildFlags = append(buildFlags, "-ldflags=-compressdwarf=false")