proc: add workaround for debug_frame bug on macOS (#2374)

This adds a workaround for the bug described at:

https://github.com/golang/go/issues/25841

Because dsymutil running on PIE does not adjust the address of
debug_frame entries (but adjusts debug_info entries) we try to do the
adjustment ourselves.

Updates #2346

Documentation/backend_test_health.md

@@ -1,23 +1,23 @@
 Tests skipped by each supported backend:
 
-* 386 skipped = 2.7% (4/148)
+* 386 skipped = 2.7% (4/149)
 	* 1 broken
 	* 3 broken - cgo stacktraces
-* arm64 skipped = 2.7% (4/148)
+* arm64 skipped = 2.7% (4/149)
 	* 2 broken
 	* 1 broken - cgo stacktraces
 	* 1 broken - global variable symbolication
-* darwin/arm64 skipped = 0.68% (1/148)
+* darwin/arm64 skipped = 0.67% (1/149)
 	* 1 broken - cgo stacktraces
-* darwin/lldb skipped = 0.68% (1/148)
+* darwin/lldb skipped = 0.67% (1/149)
 	* 1 upstream issue
-* freebsd skipped = 8.1% (12/148)
+* freebsd skipped = 8.1% (12/149)
 	* 11 broken
 	* 1 not implemented
-* linux/386/pie skipped = 0.68% (1/148)
+* linux/386/pie skipped = 0.67% (1/149)
 	* 1 broken
-* pie skipped = 0.68% (1/148)
+* pie skipped = 0.67% (1/149)
 	* 1 upstream issue - https://github.com/golang/go/issues/29322
-* windows skipped = 1.4% (2/148)
+* windows skipped = 1.3% (2/149)
 	* 1 broken
 	* 1 upstream issue

pkg/dwarf/frame/entries.go

@@ -49,6 +49,11 @@ func (fde *FrameDescriptionEntry) End() uint64 {
 	return fde.begin + fde.size
 }
 
+// Translate moves the beginning of fde forward by delta.
+func (fde *FrameDescriptionEntry) Translate(delta uint64) {
+	fde.begin += delta
+}
+
 // EstablishFrame set up frame for the given PC.
 func (fde *FrameDescriptionEntry) EstablishFrame(pc uint64) *FrameContext {
 	return executeDwarfProgramUntilPC(fde, pc)

pkg/dwarf/frame/parser.go

@@ -84,7 +84,7 @@ func parselength(ctx *parseContext) parsefunc {
 	ctx.length -= 4 // take off the length of the CIE id / CIE pointer.
 
 	if ctx.cieEntry(cieid) {
-		ctx.common = &CommonInformationEntry{Length: ctx.length, staticBase: ctx.staticBase}
+		ctx.common = &CommonInformationEntry{Length: ctx.length, staticBase: ctx.staticBase, CIE_id: cieid}
 		ctx.ciemap[start] = ctx.common
 		return parseCIE
 	}

pkg/proc/bininfo.go

@@ -704,6 +704,7 @@ func (bi *BinaryInfo) AddImage(path string, addr uint64) error {
 	if err != nil {
 		bi.Images[len(bi.Images)-1].loadErr = err
 	}
+	bi.macOSDebugFrameBugWorkaround()
 	return err
 }
 
@@ -1495,6 +1496,84 @@ func (bi *BinaryInfo) parseDebugFrameMacho(image *Image, exe *macho.File, debugI
 	bi.parseDebugFrameGeneral(image, debugFrameBytes, "__debug_frame", debugFrameErr, ehFrameBytes, ehFrameAddr, "__eh_frame", frame.DwarfEndian(debugInfoBytes))
 }
 
+// macOSDebugFrameBugWorkaround applies a workaround for:
+//  https://github.com/golang/go/issues/25841
+// It finds the Go function with the lowest entry point and the first
+// debug_frame FDE, calculates the difference between the start of the
+// function and the start of the FDE and sums it to all debug_frame FDEs.
+// A number of additional checks are performed to make sure we don't ruin
+// executables unaffected by this bug.
+func (bi *BinaryInfo) macOSDebugFrameBugWorkaround() {
+	//TODO: log extensively because of bugs in the field
+	if bi.GOOS != "darwin" || bi.Arch.Name != "arm64" {
+		return
+	}
+	if len(bi.Images) > 1 {
+		// Only do this for the first executable, but it might work for plugins as
+		// well if we had a way to distinguish where entries in bi.frameEntries
+		// come from
+		return
+	}
+	exe, ok := bi.Images[0].closer.(*macho.File)
+	if !ok {
+		return
+	}
+	if exe.Flags&macho.FlagPIE == 0 {
+		bi.logger.Infof("debug_frame workaround not needed: not a PIE (%#x)", exe.Flags)
+		return
+	}
+
+	// Find first Go function (first = lowest entry point)
+	var fn *Function
+	for i := range bi.Functions {
+		if bi.Functions[i].cu.isgo && bi.Functions[i].Entry > 0 {
+			fn = &bi.Functions[i]
+			break
+		}
+	}
+	if fn == nil {
+		bi.logger.Warn("debug_frame workaround not applied: could not find a Go function")
+		return
+	}
+
+	if fde, _ := bi.frameEntries.FDEForPC(fn.Entry); fde != nil {
+		// Function is covered, no need to apply workaround
+		bi.logger.Warnf("debug_frame workaround not applied: function %s (at %#x) covered by %#x-%#x", fn.Name, fn.Entry, fde.Begin(), fde.End())
+		return
+	}
+
+	// Find lowest FDE in debug_frame
+	var fde *frame.FrameDescriptionEntry
+	for i := range bi.frameEntries {
+		if bi.frameEntries[i].CIE.CIE_id == ^uint32(0) {
+			fde = bi.frameEntries[i]
+			break
+		}
+	}
+
+	if fde == nil {
+		bi.logger.Warnf("debug_frame workaround not applied because there are no debug_frame entries (%d)", len(bi.frameEntries))
+		return
+	}
+
+	fnsize := fn.End - fn.Entry
+
+	if fde.End()-fde.Begin() != fnsize || fde.Begin() > fn.Entry {
+		bi.logger.Warnf("debug_frame workaround not applied: function %s (at %#x-%#x) has a different size than the first FDE (%#x-%#x) (or the FDE starts after the function)", fn.Name, fn.Entry, fn.End, fde.Begin(), fde.End())
+		return
+	}
+
+	delta := fn.Entry - fde.Begin()
+
+	bi.logger.Infof("applying debug_frame workaround +%#x: function %s (at %#x-%#x) and FDE %#x-%#x", delta, fn.Name, fn.Entry, fn.End, fde.Begin(), fde.End())
+
+	for i := range bi.frameEntries {
+		if bi.frameEntries[i].CIE.CIE_id == ^uint32(0) {
+			bi.frameEntries[i].Translate(delta)
+		}
+	}
+}
+
 // Do not call this function directly it isn't able to deal correctly with package paths
 func (bi *BinaryInfo) findType(name string) (godwarf.Type, error) {
 	ref, found := bi.types[name]

pkg/proc/proc_test.go

@@ -5243,3 +5243,21 @@ func TestCompositeMemoryWrite(t *testing.T) {
 		}
 	})
 }
+
+func TestVariablesWithExternalLinking(t *testing.T) {
+	// Tests that macOSDebugFrameBugWorkaround works.
+	// See:
+	//  https://github.com/golang/go/issues/25841
+	//  https://github.com/go-delve/delve/issues/2346
+	withTestProcessArgs("testvariables2", t, ".", []string{}, protest.BuildModeExternalLinker, func(p *proc.Target, fixture protest.Fixture) {
+		assertNoError(p.Continue(), t, "Continue()")
+		str1Var := evalVariable(p, t, "str1")
+		if str1Var.Unreadable != nil {
+			t.Fatalf("variable str1 is unreadable: %v", str1Var.Unreadable)
+		}
+		t.Logf("%#v", str1Var)
+		if constant.StringVal(str1Var.Value) != "01234567890" {
+			t.Fatalf("wrong value for str1: %v", str1Var.Value)
+		}
+	})
+}

pkg/proc/target.go

@@ -322,7 +322,9 @@ func setAsyncPreemptOff(p *Target, v int64) {
 	p.asyncPreemptOff, _ = constant.Int64Val(asyncpreemptoffv.Value)
 
 	err = scope.setValue(asyncpreemptoffv, newConstant(constant.MakeInt64(v), scope.Mem), "")
-	logger.Warnf("could not set asyncpreemptoff %v", err)
+	if err != nil {
+		logger.Warnf("could not set asyncpreemptoff %v", err)
+	}
 }
 
 // createUnrecoveredPanicBreakpoint creates the unrecoverable-panic breakpoint.