From a336c92a8bf62083e1b0ecaced4329eb89b7d05c Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Tue, 11 Aug 2015 19:12:37 -0500 Subject: [PATCH] Fix: Improve handling of soft signals on darwin Fixes a bug on OSX where, if the debugged process spawned a child, when that process received a SIGCHLD it would cause Delve to hang. Fixes #197 --- _fixtures/sigchldprog.go | 25 ++++++++++++++++++++++ proc/proc_darwin.c | 46 +++++++++++++++++++++++++--------------- proc/proc_darwin.go | 31 +++++++++++++-------------- proc/proc_darwin.h | 3 +++ proc/proc_test.go | 10 +++++++++ 5 files changed, 82 insertions(+), 33 deletions(-) create mode 100644 _fixtures/sigchldprog.go diff --git a/_fixtures/sigchldprog.go b/_fixtures/sigchldprog.go new file mode 100644 index 00000000..417e1498 --- /dev/null +++ b/_fixtures/sigchldprog.go @@ -0,0 +1,25 @@ +package main + +import ( + "bufio" + "fmt" + "log" + "os/exec" +) + +func main() { + cmd := exec.Command("date") + reader, err := cmd.StdoutPipe() + if err != nil { + log.Fatalln(err) + } + scanner := bufio.NewScanner(reader) + go func() { + for scanner.Scan() { + fmt.Println(scanner.Text()) + } + reader.Close() + }() + cmd.Start() + cmd.Wait() +} diff --git a/proc/proc_darwin.c b/proc/proc_darwin.c index 06ff72f5..238fdd82 100644 --- a/proc/proc_darwin.c +++ b/proc/proc_darwin.c @@ -113,6 +113,8 @@ mach_port_t mach_port_wait(mach_port_t port_set) { kern_return_t kret; thread_act_t thread; + NDR_record_t *ndr; + integer_t *data; union { mach_msg_header_t hdr; @@ -128,28 +130,21 @@ mach_port_wait(mach_port_t port_set) { mach_msg_body_t *bod = (mach_msg_body_t*)(&msg.hdr + 1); mach_msg_port_descriptor_t *desc = (mach_msg_port_descriptor_t *)(bod + 1); thread = desc[0].name; + ndr = (NDR_record_t *)(desc + 2); + data = (integer_t *)(ndr + 1); switch (msg.hdr.msgh_id) { case 2401: // Exception - kret = thread_suspend(thread); - if (kret != KERN_SUCCESS) return 0; - + if (thread_suspend(thread) != KERN_SUCCESS) return 0; // Send our reply back so the kernel knows this exception has been handled. - mig_reply_error_t reply; - mach_msg_header_t *rh = &reply.Head; - rh->msgh_bits = MACH_MSGH_BITS(MACH_MSGH_BITS_REMOTE(msg.hdr.msgh_bits), 0); - rh->msgh_remote_port = msg.hdr.msgh_remote_port; - rh->msgh_size = (mach_msg_size_t) sizeof(mig_reply_error_t); - rh->msgh_local_port = MACH_PORT_NULL; - rh->msgh_id = msg.hdr.msgh_id + 100; - - reply.NDR = NDR_record; - reply.RetCode = KERN_SUCCESS; - - kret = mach_msg(&reply.Head, MACH_SEND_MSG|MACH_SEND_INTERRUPT, rh->msgh_size, 0, - MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); - + kret = mach_send_reply(msg.hdr); if (kret != MACH_MSG_SUCCESS) return 0; + if (data[2] == EXC_SOFT_SIGNAL) { + if (data[3] != SIGTRAP) { + if (thread_resume(thread) != KERN_SUCCESS) return 0; + return mach_port_wait(port_set); + } + } break; case 72: // Death @@ -159,6 +154,23 @@ mach_port_wait(mach_port_t port_set) { return thread; } +kern_return_t +mach_send_reply(mach_msg_header_t hdr) { + mig_reply_error_t reply; + mach_msg_header_t *rh = &reply.Head; + rh->msgh_bits = MACH_MSGH_BITS(MACH_MSGH_BITS_REMOTE(hdr.msgh_bits), 0); + rh->msgh_remote_port = hdr.msgh_remote_port; + rh->msgh_size = (mach_msg_size_t) sizeof(mig_reply_error_t); + rh->msgh_local_port = MACH_PORT_NULL; + rh->msgh_id = hdr.msgh_id + 100; + + reply.NDR = NDR_record; + reply.RetCode = KERN_SUCCESS; + + return mach_msg(&reply.Head, MACH_SEND_MSG|MACH_SEND_INTERRUPT, rh->msgh_size, 0, + MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); +} + kern_return_t raise_exception(mach_port_t task, mach_port_t thread, mach_port_t exception_port, exception_type_t exception) { return exception_raise(exception_port, thread, task, exception, 0, 0); diff --git a/proc/proc_darwin.go b/proc/proc_darwin.go index 74bd610f..5c837cb2 100644 --- a/proc/proc_darwin.go +++ b/proc/proc_darwin.go @@ -301,26 +301,25 @@ func (dbp *Process) trapWait(pid int) (*Thread, error) { dbp.updateThreadList() th, err = dbp.handleBreakpointOnThread(int(port)) if err != nil { - if _, ok := err.(NoBreakpointError); ok { - if dbp.halt { - dbp.halt = false - return dbp.Threads[int(port)], nil - } - th := dbp.Threads[int(port)] - if dbp.firstStart || th.singleStepping { - dbp.firstStart = false - return dbp.Threads[int(port)], nil - } - if err := th.Continue(); err != nil { - return nil, err - } - continue + if _, ok := err.(NoBreakpointError); !ok { + return nil, err } - return nil, err + thread := dbp.Threads[int(port)] + if dbp.halt { + dbp.halt = false + return thread, nil + } + if dbp.firstStart || thread.singleStepping { + dbp.firstStart = false + return thread, nil + } + if err := thread.Continue(); err != nil { + return nil, err + } + continue } return th, nil } - return th, nil } func wait(pid, tgid, options int) (int, *sys.WaitStatus, error) { diff --git a/proc/proc_darwin.h b/proc/proc_darwin.h index a92965a1..296294fe 100644 --- a/proc/proc_darwin.h +++ b/proc/proc_darwin.h @@ -38,6 +38,9 @@ thread_count(task_t task); mach_port_t mach_port_wait(mach_port_t); +kern_return_t +mach_send_reply(mach_msg_header_t); + kern_return_t raise_exception(mach_port_t, mach_port_t, mach_port_t, exception_type_t); diff --git a/proc/proc_test.go b/proc/proc_test.go index d02d4cb0..049d5cce 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -694,3 +694,13 @@ func TestBreakpointOnFunctionEntry(t *testing.T) { } }) } + +func TestProcessReceivesSIGCHLD(t *testing.T) { + withTestProcess("sigchldprog", t, func(p *Process, fixture protest.Fixture) { + err := p.Continue() + _, ok := err.(ProcessExitedError) + if !ok { + t.Fatalf("Continue() returned unexpected error type %s", err) + } + }) +}