Skip to content

Commit

Permalink
proc: read context from sigtrampgo, fixes TestCgoStacktrace2 on 1.21
Browse files Browse the repository at this point in the history
Changes stacktrace code to read the signal context from the arguments
of sigtrampgo.
Also changes the automatic fatalthrow breakpoint for go 1.21.
In combination these two changes fix TestCgoStacktrace2 on Go 1.21 on
various platforms.
  • Loading branch information
aarzilli committed May 30, 2023
1 parent 8c11b49 commit 91fd619
Show file tree
Hide file tree
Showing 18 changed files with 927 additions and 146 deletions.
16 changes: 6 additions & 10 deletions Documentation/backend_test_health.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
Tests skipped by each supported backend:

* 386 skipped = 7
* 1 broken
* 386 skipped = 6
* 3 broken - cgo stacktraces
* 3 not implemented
* arm64 skipped = 2
* 1 broken
* arm64 skipped = 1
* 1 broken - global variable symbolication
* darwin/arm64 skipped = 1
* 1 broken - cgo stacktraces
* darwin/arm64 skipped = 2
* 2 broken - cgo stacktraces
* darwin/lldb skipped = 1
* 1 upstream issue
* freebsd skipped = 4
Expand All @@ -17,11 +15,9 @@ Tests skipped by each supported backend:
* 1 broken
* pie skipped = 2
* 2 upstream issue - https://github.com/golang/go/issues/29322
* windows skipped = 5
* windows skipped = 4
* 1 broken
* 3 see https://github.com/go-delve/delve/issues/2768
* 1 upstream issue
* windows/arm64 skipped = 5
* windows/arm64 skipped = 4
* 3 broken
* 1 broken - cgo stacktraces
* 1 broken - step concurrent
19 changes: 5 additions & 14 deletions pkg/proc/amd64_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,25 +225,16 @@ func amd64SwitchStack(it *stackIterator, _ *op.DwarfRegisters) bool {
it.switchToGoroutineStack()
return true

default:
if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") && it.frame.Current.Fn.Name != "runtime.throw" && it.frame.Current.Fn.Name != "runtime.fatalthrow" {
// The runtime switches to the system stack in multiple places.
// This usually happens through a call to runtime.systemstack but there
// are functions that switch to the system stack manually (for example
// runtime.morestack).
// Since we are only interested in printing the system stack for cgo
// calls we switch directly to the goroutine stack if we detect that the
// function at the top of the stack is a runtime function.
//
// The function "runtime.throw" is deliberately excluded from this
// because it can end up in the stack during a cgo call and switching to
// the goroutine stack will exclude all the C functions from the stack
// trace.
case "runtime.newstack", "runtime.systemstack":
if it.systemstack && it.g != nil {
it.switchToGoroutineStack()
return true
}

return false

default:
return false
}
}

Expand Down
13 changes: 3 additions & 10 deletions pkg/proc/arm64_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/binary"
"fmt"
"runtime"
"strings"

"github.com/go-delve/delve/pkg/dwarf/frame"
"github.com/go-delve/delve/pkg/dwarf/op"
Expand Down Expand Up @@ -269,15 +268,9 @@ func arm64SwitchStack(it *stackIterator, callFrameRegs *op.DwarfRegisters) bool
it.switchToGoroutineStack()
return true
}
default:
if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") && it.frame.Current.Fn.Name != "runtime.throw" && it.frame.Current.Fn.Name != "runtime.fatalthrow" {
// The runtime switches to the system stack in multiple places.
// This usually happens through a call to runtime.systemstack but there
// are functions that switch to the system stack manually (for example
// runtime.morestack).
// Since we are only interested in printing the system stack for cgo
// calls we switch directly to the goroutine stack if we detect that the
// function at the top of the stack is a runtime function.

case "runtime.newstack", "runtime.systemstack":
if it.systemstack && it.g != nil {
it.switchToGoroutineStack()
return true
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/proc/breakpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
nextDeferOk := true
if breaklet.Kind&NextDeferBreakpoint != 0 {
var err error
frames, err := ThreadStacktrace(thread, 2)
frames, err := ThreadStacktrace(tgt, thread, 2)
if err == nil {
nextDeferOk, _ = isPanicCall(frames)
if !nextDeferOk {
Expand All @@ -297,7 +297,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa

case WatchOutOfScopeBreakpoint:
if breaklet.checkPanicCall {
frames, err := ThreadStacktrace(thread, 2)
frames, err := ThreadStacktrace(tgt, thread, 2)
if err == nil {
ipc, _ := isPanicCall(frames)
active = active && ipc
Expand Down
23 changes: 13 additions & 10 deletions pkg/proc/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ func logRegisters(t *testing.T, regs proc.Registers, arch *proc.Arch) {
}

func TestCore(t *testing.T) {
if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
return
if runtime.GOOS != "linux" || runtime.GOARCH == "386" {
t.Skip("unsupported")
}
if runtime.GOOS == "linux" && os.Getenv("CI") == "true" && buildMode == "pie" {
t.Skip("disabled on linux, Github Actions, with PIE buildmode")
Expand All @@ -268,7 +268,7 @@ func TestCore(t *testing.T) {
var panickingStack []proc.Stackframe
for _, g := range gs {
t.Logf("Goroutine %d", g.ID)
stack, err := g.Stacktrace(10, 0)
stack, err := proc.GoroutineStacktrace(p, g, 10, 0)
if err != nil {
t.Errorf("Stacktrace() on goroutine %v = %v", g, err)
}
Expand Down Expand Up @@ -315,8 +315,11 @@ func TestCore(t *testing.T) {
}

func TestCoreFpRegisters(t *testing.T) {
if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
return
if runtime.GOOS != "linux" || runtime.GOARCH == "386" {
t.Skip("unsupported")
}
if runtime.GOARCH != "amd64" {
t.Skip("test requires amd64")
}
// in go1.10 the crash is executed on a different thread and registers are
// no longer available in the core dump.
Expand All @@ -334,7 +337,7 @@ func TestCoreFpRegisters(t *testing.T) {

var regs proc.Registers
for _, thread := range p.ThreadList() {
frames, err := proc.ThreadStacktrace(thread, 10)
frames, err := proc.ThreadStacktrace(p, thread, 10)
if err != nil {
t.Errorf("ThreadStacktrace for %x = %v", thread.ThreadID(), err)
continue
Expand Down Expand Up @@ -402,8 +405,8 @@ func TestCoreFpRegisters(t *testing.T) {
}

func TestCoreWithEmptyString(t *testing.T) {
if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
return
if runtime.GOOS != "linux" || runtime.GOARCH == "386" {
t.Skip("unsupported")
}
if runtime.GOOS == "linux" && os.Getenv("CI") == "true" && buildMode == "pie" {
t.Skip("disabled on linux, Github Actions, with PIE buildmode")
Expand All @@ -417,7 +420,7 @@ func TestCoreWithEmptyString(t *testing.T) {
var mainFrame *proc.Stackframe
mainSearch:
for _, g := range gs {
stack, err := g.Stacktrace(10, 0)
stack, err := proc.GoroutineStacktrace(p, g, 10, 0)
assertNoError(err, t, "Stacktrace()")
for _, frame := range stack {
if frame.Current.Fn != nil && frame.Current.Fn.Name == "main.main" {
Expand Down Expand Up @@ -466,7 +469,7 @@ func TestMinidump(t *testing.T) {
t.Logf("%d goroutines", len(gs))
foundMain, foundTime := false, false
for _, g := range gs {
stack, err := g.Stacktrace(10, 0)
stack, err := proc.GoroutineStacktrace(p, g, 10, 0)
if err != nil {
t.Errorf("Stacktrace() on goroutine %v = %v", g, err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/proc/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ func ConvertEvalScope(dbp *Target, gid int64, frame, deferCall int) (*EvalScope,

var locs []Stackframe
if g != nil {
locs, err = g.Stacktrace(frame+1, opts)
locs, err = GoroutineStacktrace(dbp, g, frame+1, opts)
} else {
locs, err = ThreadStacktrace(ct, frame+1)
locs, err = ThreadStacktrace(dbp, ct, frame+1)
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -145,7 +145,7 @@ func FrameToScope(t *Target, thread MemoryReadWriter, g *G, frames ...Stackframe

// ThreadScope returns an EvalScope for the given thread.
func ThreadScope(t *Target, thread Thread) (*EvalScope, error) {
locations, err := ThreadStacktrace(thread, 1)
locations, err := ThreadStacktrace(t, thread, 1)
if err != nil {
return nil, err
}
Expand All @@ -157,7 +157,7 @@ func ThreadScope(t *Target, thread Thread) (*EvalScope, error) {

// GoroutineScope returns an EvalScope for the goroutine running on the given thread.
func GoroutineScope(t *Target, thread Thread) (*EvalScope, error) {
locations, err := ThreadStacktrace(thread, 1)
locations, err := ThreadStacktrace(t, thread, 1)
if err != nil {
return nil, err
}
Expand Down
19 changes: 5 additions & 14 deletions pkg/proc/i386_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,25 +152,16 @@ func i386SwitchStack(it *stackIterator, _ *op.DwarfRegisters) bool {
it.switchToGoroutineStack()
return true

default:
if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") && it.frame.Current.Fn.Name != "runtime.throw" && it.frame.Current.Fn.Name != "runtime.fatalthrow" {
// The runtime switches to the system stack in multiple places.
// This usually happens through a call to runtime.systemstack but there
// are functions that switch to the system stack manually (for example
// runtime.morestack).
// Since we are only interested in printing the system stack for cgo
// calls we switch directly to the goroutine stack if we detect that the
// function at the top of the stack is a runtime function.
//
// The function "runtime.throw" is deliberately excluded from this
// because it can end up in the stack during a cgo call and switching to
// the goroutine stack will exclude all the C functions from the stack
// trace.
case "runtime.newstack", "runtime.systemstack":
if it.systemstack && it.g != nil {
it.switchToGoroutineStack()
return true
}

return false

default:
return false
}
}

Expand Down
Loading

0 comments on commit 91fd619

Please sign in to comment.