proc: move resume notify and manual stop handling to Target (#2921)

Moves handling of ResumeNotify and manualStopRequested to Target instead of the backends

Updates #2551
This commit is contained in:
Alessandro Arzilli 2022-03-21 20:42:37 +01:00 committed by GitHub
parent a19931c9d3
commit e1e4b09a5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 102 additions and 126 deletions

@ -415,7 +415,7 @@ func (p *process) ClearInternalBreakpoints() error {
// ContinueOnce will always return an error because you
// cannot control execution of a core file.
func (p *process) ContinueOnce() (proc.Thread, proc.StopReason, error) {
func (p *process) ContinueOnce(cctx *proc.ContinueOnceContext) (proc.Thread, proc.StopReason, error) {
return nil, proc.StopUnknown, ErrContinueCore
}
@ -427,7 +427,7 @@ func (p *process) StepInstruction() error {
// RequestManualStop will return nil and have no effect
// as you cannot control execution of a core file.
func (p *process) RequestManualStop() error {
func (p *process) RequestManualStop(cctx *proc.ContinueOnceContext) error {
return nil
}

@ -152,8 +152,6 @@ type gdbProcess struct {
almostExited bool // true if 'rr' has sent its synthetic SIGKILL
ctrlC bool // ctrl-c was sent to stop inferior
manualStopRequested bool
breakpoints proc.BreakpointMap
gcmdok bool // true if the stub supports g and (maybe) G commands
@ -762,12 +760,6 @@ func (p *gdbProcess) Valid() (bool, error) {
return true, nil
}
// ResumeNotify specifies a channel that will be closed the next time
// ContinueOnce finishes resuming the target.
func (p *gdbProcess) ResumeNotify(ch chan<- struct{}) {
p.conn.resumeChan = ch
}
// FindThread returns the thread with the given ID.
func (p *gdbProcess) FindThread(threadID int) (proc.Thread, bool) {
thread, ok := p.threads[threadID]
@ -809,7 +801,7 @@ const (
// ContinueOnce will continue execution of the process until
// a breakpoint is hit or signal is received.
func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
func (p *gdbProcess) ContinueOnce(cctx *proc.ContinueOnceContext) (proc.Thread, proc.StopReason, error) {
if p.exited {
return nil, proc.StopExited, proc.ErrProcessExited{Pid: p.conn.pid}
}
@ -835,7 +827,7 @@ func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
th.clearBreakpointState()
}
p.setCtrlC(false)
p.setCtrlC(cctx, false)
// resume all threads
var threadID string
@ -845,7 +837,7 @@ func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
continueLoop:
for {
tu.Reset()
sp, err := p.conn.resume(p.threads, &tu)
sp, err := p.conn.resume(cctx, p.threads, &tu)
threadID = sp.threadID
if err != nil {
if _, exited := err.(proc.ErrProcessExited); exited {
@ -868,7 +860,7 @@ continueLoop:
}
var shouldStop, shouldExitErr bool
trapthread, atstart, shouldStop, shouldExitErr = p.handleThreadSignals(trapthread)
trapthread, atstart, shouldStop, shouldExitErr = p.handleThreadSignals(cctx, trapthread)
if shouldExitErr {
p.almostExited = true
return nil, proc.StopExited, proc.ErrProcessExited{Pid: p.conn.pid}
@ -922,7 +914,7 @@ func (p *gdbProcess) findThreadByStrID(threadID string) *gdbThread {
// and returns true if we should stop execution in response to one of the
// signals and return control to the user.
// Adjusts trapthread to a thread that we actually want to stop at.
func (p *gdbProcess) handleThreadSignals(trapthread *gdbThread) (trapthreadOut *gdbThread, atstart, shouldStop, shouldExitErr bool) {
func (p *gdbProcess) handleThreadSignals(cctx *proc.ContinueOnceContext, trapthread *gdbThread) (trapthreadOut *gdbThread, atstart, shouldStop, shouldExitErr bool) {
var trapthreadCandidate *gdbThread
for _, th := range p.threads {
@ -935,7 +927,7 @@ func (p *gdbProcess) handleThreadSignals(trapthread *gdbThread) (trapthreadOut *
// the ctrlC flag to know that we are the originators.
switch th.sig {
case interruptSignal: // interrupt
if p.getCtrlC() {
if p.getCtrlC(cctx) {
isStopSignal = true
}
case breakpointSignal: // breakpoint
@ -994,7 +986,7 @@ func (p *gdbProcess) handleThreadSignals(trapthread *gdbThread) (trapthreadOut *
trapthread = trapthreadCandidate
}
if p.getCtrlC() || p.getManualStopRequested() {
if p.getCtrlC(cctx) || cctx.GetManualStopRequested() {
// If we request an interrupt and a target thread simultaneously receives
// an unrelated singal debugserver will discard our interrupt request and
// report the signal but we should stop anyway.
@ -1006,44 +998,23 @@ func (p *gdbProcess) handleThreadSignals(trapthread *gdbThread) (trapthreadOut *
// RequestManualStop will attempt to stop the process
// without a breakpoint or signal having been received.
func (p *gdbProcess) RequestManualStop() error {
p.conn.manualStopMutex.Lock()
p.manualStopRequested = true
func (p *gdbProcess) RequestManualStop(cctx *proc.ContinueOnceContext) error {
if !p.conn.running {
p.conn.manualStopMutex.Unlock()
return nil
}
p.ctrlC = true
p.conn.manualStopMutex.Unlock()
return p.conn.sendCtrlC()
}
// CheckAndClearManualStopRequest will check for a manual
// stop and then clear that state.
func (p *gdbProcess) CheckAndClearManualStopRequest() bool {
p.conn.manualStopMutex.Lock()
msr := p.manualStopRequested
p.manualStopRequested = false
p.conn.manualStopMutex.Unlock()
return msr
}
func (p *gdbProcess) getManualStopRequested() bool {
p.conn.manualStopMutex.Lock()
msr := p.manualStopRequested
p.conn.manualStopMutex.Unlock()
return msr
}
func (p *gdbProcess) setCtrlC(v bool) {
p.conn.manualStopMutex.Lock()
func (p *gdbProcess) setCtrlC(cctx *proc.ContinueOnceContext, v bool) {
cctx.StopMu.Lock()
p.ctrlC = v
p.conn.manualStopMutex.Unlock()
cctx.StopMu.Unlock()
}
func (p *gdbProcess) getCtrlC() bool {
p.conn.manualStopMutex.Lock()
defer p.conn.manualStopMutex.Unlock()
func (p *gdbProcess) getCtrlC(cctx *proc.ContinueOnceContext) bool {
cctx.StopMu.Lock()
defer cctx.StopMu.Unlock()
return p.ctrlC
}
@ -1077,7 +1048,7 @@ func (p *gdbProcess) Detach(kill bool) error {
}
// Restart will restart the process from the given position.
func (p *gdbProcess) Restart(pos string) (proc.Thread, error) {
func (p *gdbProcess) Restart(cctx *proc.ContinueOnceContext, pos string) (proc.Thread, error) {
if p.tracedir == "" {
return nil, proc.ErrNotRecorded
}
@ -1089,7 +1060,7 @@ func (p *gdbProcess) Restart(pos string) (proc.Thread, error) {
th.clearBreakpointState()
}
p.setCtrlC(false)
p.ctrlC = false
err := p.conn.restart(pos)
if err != nil {
@ -1098,7 +1069,7 @@ func (p *gdbProcess) Restart(pos string) (proc.Thread, error) {
// for some reason we have to send a vCont;c after a vRun to make rr behave
// properly, because that's what gdb does.
_, err = p.conn.resume(nil, nil)
_, err = p.conn.resume(cctx, nil, nil)
if err != nil {
return nil, err
}

@ -13,7 +13,6 @@ import (
"os"
"strconv"
"strings"
"sync"
"time"
"github.com/go-delve/delve/pkg/logflags"
@ -28,9 +27,7 @@ type gdbConn struct {
inbuf []byte
outbuf bytes.Buffer
manualStopMutex sync.Mutex
running bool
resumeChan chan<- struct{}
running bool
direction proc.Direction // direction of execution
@ -567,7 +564,7 @@ func (conn *gdbConn) writeRegister(threadID string, regnum int, data []byte) err
// resume each thread. If a thread has sig == 0 the 'c' action will be used,
// otherwise the 'C' action will be used and the value of sig will be passed
// to it.
func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (stopPacket, error) {
func (conn *gdbConn) resume(cctx *proc.ContinueOnceContext, threads map[int]*gdbThread, tu *threadUpdater) (stopPacket, error) {
if conn.direction == proc.Forward {
conn.outbuf.Reset()
fmt.Fprintf(&conn.outbuf, "$vCont")
@ -584,21 +581,21 @@ func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (stop
conn.outbuf.Reset()
fmt.Fprint(&conn.outbuf, "$bc")
}
conn.manualStopMutex.Lock()
cctx.StopMu.Lock()
if err := conn.send(conn.outbuf.Bytes()); err != nil {
conn.manualStopMutex.Unlock()
cctx.StopMu.Unlock()
return stopPacket{}, err
}
conn.running = true
conn.manualStopMutex.Unlock()
cctx.StopMu.Unlock()
defer func() {
conn.manualStopMutex.Lock()
cctx.StopMu.Lock()
conn.running = false
conn.manualStopMutex.Unlock()
cctx.StopMu.Unlock()
}()
if conn.resumeChan != nil {
close(conn.resumeChan)
conn.resumeChan = nil
if cctx.ResumeChan != nil {
close(cctx.ResumeChan)
cctx.ResumeChan = nil
}
return conn.waitForvContStop("resume", "-1", tu)
}

@ -1,6 +1,8 @@
package proc
import (
"sync"
"github.com/go-delve/delve/pkg/elfwriter"
"github.com/go-delve/delve/pkg/proc/internal/ebpf"
)
@ -13,18 +15,9 @@ import (
// There is one exception to this rule: it is safe to call RequestManualStop
// concurrently with ContinueOnce.
type Process interface {
// ResumeNotify specifies a channel that will be closed the next time
// ContinueOnce finishes resuming the target.
ResumeNotify(chan<- struct{})
BinInfo() *BinaryInfo
EntryPoint() (uint64, error)
// RequestManualStop attempts to stop all the process' threads.
RequestManualStop() error
// CheckAndClearManualStopRequest returns true the first time it's called
// after a call to RequestManualStop.
CheckAndClearManualStopRequest() bool
FindThread(threadID int) (Thread, bool)
ThreadList() []Thread
@ -45,7 +38,10 @@ type ProcessInternal interface {
// ErrProcessExited or ErrProcessDetached).
Valid() (bool, error)
Detach(bool) error
ContinueOnce() (trapthread Thread, stopReason StopReason, err error)
ContinueOnce(*ContinueOnceContext) (trapthread Thread, stopReason StopReason, err error)
// RequestManualStop attempts to stop all the process' threads.
RequestManualStop(cctx *ContinueOnceContext) error
WriteBreakpoint(*Breakpoint) error
EraseBreakpoint(*Breakpoint) error
@ -93,7 +89,7 @@ type RecordingManipulationInternal interface {
// If pos starts with 'c' it's a checkpoint ID, otherwise it's an event
// number.
// Returns the new current thread after the restart has completed.
Restart(pos string) (Thread, error)
Restart(cctx *ContinueOnceContext, pos string) (Thread, error)
}
// Direction is the direction of execution for the target process.
@ -112,3 +108,30 @@ type Checkpoint struct {
When string
Where string
}
// ContinueOnceContext is an object passed to ContinueOnce that the backend
// can use to communicate with the target layer.
type ContinueOnceContext struct {
ResumeChan chan<- struct{}
StopMu sync.Mutex
// manualStopRequested is set if all the threads in the process were
// signalled to stop as a result of a Halt API call. Used to disambiguate
// why a thread is found to have stopped.
manualStopRequested bool
}
// CheckAndClearManualStopRequest will check for a manual
// stop and then clear that state.
func (cctx *ContinueOnceContext) CheckAndClearManualStopRequest() bool {
cctx.StopMu.Lock()
defer cctx.StopMu.Unlock()
msr := cctx.manualStopRequested
cctx.manualStopRequested = false
return msr
}
func (cctx *ContinueOnceContext) GetManualStopRequested() bool {
cctx.StopMu.Lock()
defer cctx.StopMu.Unlock()
return cctx.manualStopRequested
}

@ -61,7 +61,7 @@ func (dbp *nativeProcess) trapWait(pid int) (*nativeThread, error) {
panic(ErrNativeBackendDisabled)
}
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativeThread) (*nativeThread, error) {
panic(ErrNativeBackendDisabled)
}

@ -3,7 +3,6 @@ package native
import (
"os"
"runtime"
"sync"
"github.com/go-delve/delve/pkg/proc"
)
@ -27,15 +26,9 @@ type nativeProcess struct {
os *osProcessDetails
firstStart bool
resumeChan chan<- struct{}
ptraceChan chan func()
ptraceDoneChan chan interface{}
childProcess bool // this process was launched, not attached to
stopMu sync.Mutex // protects manualStopRequested
// manualStopRequested is set if all the threads in the process were
// signalled to stop as a result of a Halt API call. Used to disambiguate
// why a thread is found to have stopped.
manualStopRequested bool
childProcess bool // this process was launched, not attached to
// Controlling terminal file descriptor for
// this process.
@ -112,12 +105,6 @@ func (dbp *nativeProcess) Valid() (bool, error) {
return true, nil
}
// ResumeNotify specifies a channel that will be closed the next time
// ContinueOnce finishes resuming the target.
func (dbp *nativeProcess) ResumeNotify(ch chan<- struct{}) {
dbp.resumeChan = ch
}
// ThreadList returns a list of threads in the process.
func (dbp *nativeProcess) ThreadList() []proc.Thread {
r := make([]proc.Thread, 0, len(dbp.threads))
@ -145,28 +132,13 @@ func (dbp *nativeProcess) Breakpoints() *proc.BreakpointMap {
// RequestManualStop sets the `manualStopRequested` flag and
// sends SIGSTOP to all threads.
func (dbp *nativeProcess) RequestManualStop() error {
func (dbp *nativeProcess) RequestManualStop(cctx *proc.ContinueOnceContext) error {
if dbp.exited {
return proc.ErrProcessExited{Pid: dbp.pid}
}
dbp.stopMu.Lock()
defer dbp.stopMu.Unlock()
dbp.manualStopRequested = true
return dbp.requestManualStop()
}
// CheckAndClearManualStopRequest checks if a manual stop has
// been requested, and then clears that state.
func (dbp *nativeProcess) CheckAndClearManualStopRequest() bool {
dbp.stopMu.Lock()
defer dbp.stopMu.Unlock()
msr := dbp.manualStopRequested
dbp.manualStopRequested = false
return msr
}
func (dbp *nativeProcess) WriteBreakpoint(bp *proc.Breakpoint) error {
if bp.WatchType != 0 {
for _, thread := range dbp.threads {
@ -202,7 +174,7 @@ func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error {
// ContinueOnce will continue the target until it stops.
// This could be the result of a breakpoint or signal.
func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
func (dbp *nativeProcess) ContinueOnce(cctx *proc.ContinueOnceContext) (proc.Thread, proc.StopReason, error) {
if dbp.exited {
return nil, proc.StopExited, proc.ErrProcessExited{Pid: dbp.pid}
}
@ -217,16 +189,16 @@ func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
th.CurrentBreakpoint.Clear()
}
if dbp.resumeChan != nil {
close(dbp.resumeChan)
dbp.resumeChan = nil
if cctx.ResumeChan != nil {
close(cctx.ResumeChan)
cctx.ResumeChan = nil
}
trapthread, err := dbp.trapWait(-1)
if err != nil {
return nil, proc.StopUnknown, err
}
trapthread, err = dbp.stop(trapthread)
trapthread, err = dbp.stop(cctx, trapthread)
if err != nil {
return nil, proc.StopUnknown, err
}

@ -121,7 +121,7 @@ func Launch(cmd []string, wd string, flags proc.LaunchFlags, _ []string, _ strin
if err != nil {
return nil, err
}
if _, err := dbp.stop(nil); err != nil {
if _, err := dbp.stop(nil, nil); err != nil {
return nil, err
}
@ -318,9 +318,7 @@ func (dbp *nativeProcess) trapWait(pid int) (*nativeThread, error) {
return nil, proc.ErrProcessExited{Pid: dbp.pid, Status: status.ExitStatus()}
case C.MACH_RCV_INTERRUPTED:
dbp.stopMu.Lock()
halt := dbp.os.halt
dbp.stopMu.Unlock()
if !halt {
// Call trapWait again, it seems
// MACH_RCV_INTERRUPTED is emitted before
@ -349,9 +347,7 @@ func (dbp *nativeProcess) trapWait(pid int) (*nativeThread, error) {
dbp.updateThreadList()
th, ok := dbp.threads[int(port)]
if !ok {
dbp.stopMu.Lock()
halt := dbp.os.halt
dbp.stopMu.Unlock()
if halt {
dbp.os.halt = false
return th, nil
@ -433,7 +429,7 @@ func (dbp *nativeProcess) resume() error {
}
// stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited {
return nil, proc.ErrProcessExited{Pid: dbp.pid}
}

@ -350,7 +350,7 @@ func (dbp *nativeProcess) resume() error {
// Used by ContinueOnce
// stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited {
return nil, proc.ErrProcessExited{Pid: dbp.pid}
}

@ -538,7 +538,7 @@ func (dbp *nativeProcess) resume() error {
}
// stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited {
return nil, proc.ErrProcessExited{Pid: dbp.pid}
}
@ -618,9 +618,7 @@ func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error)
if th.CurrentBreakpoint.Breakpoint == nil && th.os.setbp && (th.Status != nil) && ((*sys.WaitStatus)(th.Status).StopSignal() == sys.SIGTRAP) && dbp.BinInfo().Arch.BreakInstrMovesPC() {
manualStop := false
if th.ThreadID() == trapthread.ThreadID() {
dbp.stopMu.Lock()
manualStop = dbp.manualStopRequested
dbp.stopMu.Unlock()
manualStop = cctx.GetManualStopRequested()
}
if !manualStop && th.os.phantomBreakpointPC == pc {
// Thread received a SIGTRAP but we don't have a breakpoint for it and

@ -425,7 +425,7 @@ func (dbp *nativeProcess) resume() error {
}
// stop stops all running threads threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
func (dbp *nativeProcess) stop(cctx *proc.ContinueOnceContext, trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited {
return nil, proc.ErrProcessExited{Pid: dbp.pid}
}

@ -83,6 +83,8 @@ type Target struct {
// can be given a unique address.
fakeMemoryRegistry []*compositeMemory
fakeMemoryRegistryMap map[string]*compositeMemory
cctx *ContinueOnceContext
}
type KeepSteppingBreakpoints uint8
@ -198,6 +200,7 @@ func NewTarget(p ProcessInternal, pid int, currentThread Thread, cfg NewTargetCo
currentThread: currentThread,
CanDump: cfg.CanDump,
pid: pid,
cctx: &ContinueOnceContext{},
}
if recman, ok := p.(RecordingManipulationInternal); ok {
@ -283,7 +286,7 @@ func (t *Target) ClearCaches() {
// Restarting of a normal process happens at a higher level (debugger.Restart).
func (t *Target) Restart(from string) error {
t.ClearCaches()
currentThread, err := t.recman.Restart(from)
currentThread, err := t.recman.Restart(t.cctx, from)
if err != nil {
return err
}
@ -464,6 +467,20 @@ func (t *Target) GetBufferedTracepoints() []*UProbeTraceResult {
return results
}
// ResumeNotify specifies a channel that will be closed the next time
// Continue finishes resuming the target.
func (t *Target) ResumeNotify(ch chan<- struct{}) {
t.cctx.ResumeChan = ch
}
// RequestManualStop attempts to stop all the process' threads.
func (t *Target) RequestManualStop() error {
t.cctx.StopMu.Lock()
defer t.cctx.StopMu.Unlock()
t.cctx.manualStopRequested = true
return t.proc.RequestManualStop(t.cctx)
}
const (
FakeAddressBase = 0xbeef000000000000
fakeAddressUnresolv = 0xbeed000000000000 // this address never resloves to memory
@ -593,4 +610,6 @@ func (*dummyRecordingManipulation) ClearCheckpoint(int) error { return ErrNotRec
// Restart will always return an error in the native proc backend, only for
// recorded traces.
func (*dummyRecordingManipulation) Restart(string) (Thread, error) { return nil, ErrNotRecorded }
func (*dummyRecordingManipulation) Restart(*ContinueOnceContext, string) (Thread, error) {
return nil, ErrNotRecorded
}

@ -56,11 +56,11 @@ func (dbp *Target) Continue() error {
}
dbp.Breakpoints().WatchOutOfScope = nil
dbp.clearHardcodedBreakpoints()
dbp.CheckAndClearManualStopRequest()
dbp.cctx.CheckAndClearManualStopRequest()
defer func() {
// Make sure we clear internal breakpoints if we simultaneously receive a
// manual stop request and hit a breakpoint.
if dbp.CheckAndClearManualStopRequest() {
if dbp.cctx.CheckAndClearManualStopRequest() {
dbp.StopReason = StopManual
dbp.clearHardcodedBreakpoints()
if dbp.KeepSteppingBreakpoints&HaltKeepsSteppingBreakpoints == 0 {
@ -69,7 +69,7 @@ func (dbp *Target) Continue() error {
}
}()
for {
if dbp.CheckAndClearManualStopRequest() {
if dbp.cctx.CheckAndClearManualStopRequest() {
dbp.StopReason = StopManual
dbp.clearHardcodedBreakpoints()
if dbp.KeepSteppingBreakpoints&HaltKeepsSteppingBreakpoints == 0 {
@ -78,7 +78,7 @@ func (dbp *Target) Continue() error {
return nil
}
dbp.ClearCaches()
trapthread, stopReason, contOnceErr := dbp.proc.ContinueOnce()
trapthread, stopReason, contOnceErr := dbp.proc.ContinueOnce(dbp.cctx)
dbp.StopReason = stopReason
threads := dbp.ThreadList()