diff --git a/Documentation/backend_test_health.md b/Documentation/backend_test_health.md index 279ac095..c79bd2a8 100644 --- a/Documentation/backend_test_health.md +++ b/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 diff --git a/pkg/dwarf/frame/entries.go b/pkg/dwarf/frame/entries.go index cdfd204b..4c6f94d0 100644 --- a/pkg/dwarf/frame/entries.go +++ b/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) diff --git a/pkg/dwarf/frame/parser.go b/pkg/dwarf/frame/parser.go index ba5ddd59..bcec7e56 100644 --- a/pkg/dwarf/frame/parser.go +++ b/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 } diff --git a/pkg/proc/bininfo.go b/pkg/proc/bininfo.go index 22e5237f..8bda0702 100644 --- a/pkg/proc/bininfo.go +++ b/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] diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 02fada29..c25f7799 100644 --- a/pkg/proc/proc_test.go +++ b/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) + } + }) +} diff --git a/pkg/proc/target.go b/pkg/proc/target.go index 9f5235bd..96727a8a 100644 --- a/pkg/proc/target.go +++ b/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.