Skip to content

Commit

Permalink
proc: add workaround for debug_frame bug on macOS
Browse files Browse the repository at this point in the history
This adds a workaround for the bug described at:

golang/go#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 go-delve#2346
  • Loading branch information
aarzilli committed Mar 9, 2021
1 parent b40774c commit 6cc9215
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 10 deletions.
16 changes: 8 additions & 8 deletions Documentation/backend_test_health.md
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions pkg/dwarf/frame/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/dwarf/frame/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
79 changes: 79 additions & 0 deletions pkg/proc/bininfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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]
Expand Down
18 changes: 18 additions & 0 deletions pkg/proc/proc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
4 changes: 3 additions & 1 deletion pkg/proc/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 6cc9215

Please sign in to comment.