Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proc: read context from sigtrampgo, fixes TestCgoStacktrace2 on 1.21 #3401

Merged
merged 2 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
1 change: 1 addition & 0 deletions Documentation/usage/dlv_log.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ names selected from this list:
dap Log all DAP messages
fncall Log function call protocol
minidump Log minidump loading
stack Log stacktracer

Additionally --log-dest can be used to specify where the logs should be
written.
Expand Down
1 change: 1 addition & 0 deletions cmd/dlv/cmds/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ names selected from this list:
dap Log all DAP messages
fncall Log function call protocol
minidump Log minidump loading
stack Log stacktracer

Additionally --log-dest can be used to specify where the logs should be
written.
Expand Down
12 changes: 12 additions & 0 deletions pkg/logflags/logflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var rpc = false
var dap = false
var fnCall = false
var minidump = false
var stack = false

var logOut io.WriteCloser

Expand Down Expand Up @@ -131,6 +132,15 @@ func MinidumpLogger() Logger {
return makeLogger(minidump, Fields{"layer": "core", "kind": "minidump"})
}

// Stack returns true if the stacktracer should be logged.
func Stack() bool {
return stack
}

func StackLogger() Logger {
return makeLogger(stack, Fields{"layer": "core", "kind": "stack"})
}

// WriteDAPListeningMessage writes the "DAP server listening" message in dap mode.
func WriteDAPListeningMessage(addr net.Addr) {
writeListeningMessage("DAP", addr)
Expand Down Expand Up @@ -215,6 +225,8 @@ func Setup(logFlag bool, logstr, logDest string) error {
fnCall = true
case "minidump":
minidump = true
case "stack":
stack = true
default:
fmt.Fprintf(os.Stderr, "Warning: unknown log output value %q, run 'dlv help log' for usage.\n", logcmd)
}
Expand Down
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