[03/16,v2] Refactor ptrace extended event status
Commit Message
This patch implements functions for identifying and extracting extended
ptrace event information from a Linux wait status. These are just
convenience functions intended to hide the ">> 16" used to extract the
event from the wait status word, replacing the hard-coded shift with a more
descriptive function call. This is preparatory work for implementation of
follow-fork and detach-on-fork for extended-remote linux targets.
The functions linux_is_extended_waitstatus and
linux_ptrace_get_extended_event are defined in nat/linux-ptrace.c, and
called in linux-nat.c and gdbserver/linux-low.c.
My initial approach was to implement predicates for every extended event,
e.g. linux_is_traced_clone (status), linux_is_traced_fork (status), but
that didn't fit the current implementation as well, bloated the code a bit,
and didn't add anything to readability, so I went with just extracting the
event bits from the status instead.
Tested on x64 Ubuntu Lucid, native only.
Thanks
--Don
gdb/
2014-08-20 Don Breazeal <donb@codesourcery.com>
* linux-nat.c (linux_handle_extended_wait): Call
linux_ptrace_get_extended_event.
(wait_lwp): Call linux_is_extended_waitstatus.
(linux_nat_filter_event): Call linux_ptrace_get_extended_event
and linux_is_extended_waitstatus.
* nat/linux-ptrace.c (linux_test_for_tracefork): Call
linux_ptrace_get_extended_event.
(linux_ptrace_get_extended_event): New function.
(linux_is_extended_waitstatus): New function.
* nat/linux-ptrace.h: Declare new functions.
gdbserver/
2014-08-20 Don Breazeal <donb@codesourcery.com>
* linux-low.c (handle_extended_wait): Call
linux_ptrace_get_extended_event.
(get_stop_pc, get_detach_signal, linux_low_filter_event): Call
linux_is_extended_waitstatus.
---
gdb/gdbserver/linux-low.c | 8 ++++----
gdb/linux-nat.c | 11 +++++++----
gdb/nat/linux-ptrace.c | 18 +++++++++++++++++-
gdb/nat/linux-ptrace.h | 2 ++
4 files changed, 30 insertions(+), 9 deletions(-)
Comments
Hi Don,
Let's get this one out of the way.
On 08/21/2014 01:29 AM, Don Breazeal wrote:
> This patch implements functions for identifying and extracting extended
> ptrace event information from a Linux wait status. These are just
> convenience functions intended to hide the ">> 16" used to extract the
> event from the wait status word, replacing the hard-coded shift with a more
> descriptive function call. This is preparatory work for implementation of
> follow-fork and detach-on-fork for extended-remote linux targets.
>
> The functions linux_is_extended_waitstatus and
> linux_ptrace_get_extended_event are defined in nat/linux-ptrace.c, and
> called in linux-nat.c and gdbserver/linux-low.c.
>
> My initial approach was to implement predicates for every extended event,
> e.g. linux_is_traced_clone (status), linux_is_traced_fork (status), but
> that didn't fit the current implementation as well, bloated the code a bit,
> and didn't add anything to readability, so I went with just extracting the
> event bits from the status instead.
>
> Tested on x64 Ubuntu Lucid, native only.
This is OK. Please push.
Though I wonder why not push the SIGTRAP check to
linux_is_extended_waitstatus too.
> gdb/
> 2014-08-20 Don Breazeal <donb@codesourcery.com>
>
> * linux-nat.c (linux_handle_extended_wait): Call
> linux_ptrace_get_extended_event.
> (wait_lwp): Call linux_is_extended_waitstatus.
> (linux_nat_filter_event): Call linux_ptrace_get_extended_event
> and linux_is_extended_waitstatus.
> * nat/linux-ptrace.c (linux_test_for_tracefork): Call
> linux_ptrace_get_extended_event.
> (linux_ptrace_get_extended_event): New function.
> (linux_is_extended_waitstatus): New function.
> * nat/linux-ptrace.h: Declare new functions.
Please spell out the new declarations. Like:
* nat/linux-ptrace.h (linux_ptrace_get_extended_event)
(linux_is_extended_waitstatus): New declarations.
>
> gdbserver/
> 2014-08-20 Don Breazeal <donb@codesourcery.com>
> * linux-low.c (handle_extended_wait): Call
Missing empty line above.
> linux_ptrace_get_extended_event.
> (get_stop_pc, get_detach_signal, linux_low_filter_event): Call
> linux_is_extended_waitstatus.
Thanks,
Pedro Alves
On 9/9/2014 4:31 AM, Pedro Alves wrote:
> Hi Don,
>
> Let's get this one out of the way.
>
> On 08/21/2014 01:29 AM, Don Breazeal wrote:
>> This patch implements functions for identifying and extracting extended
>> ptrace event information from a Linux wait status. These are just
>> convenience functions intended to hide the ">> 16" used to extract the
>> event from the wait status word, replacing the hard-coded shift with a more
>> descriptive function call. This is preparatory work for implementation of
>> follow-fork and detach-on-fork for extended-remote linux targets.
>>
>> The functions linux_is_extended_waitstatus and
>> linux_ptrace_get_extended_event are defined in nat/linux-ptrace.c, and
>> called in linux-nat.c and gdbserver/linux-low.c.
>>
>> My initial approach was to implement predicates for every extended event,
>> e.g. linux_is_traced_clone (status), linux_is_traced_fork (status), but
>> that didn't fit the current implementation as well, bloated the code a bit,
>> and didn't add anything to readability, so I went with just extracting the
>> event bits from the status instead.
>>
>> Tested on x64 Ubuntu Lucid, native only.
>
> This is OK. Please push.
Thanks for the review. This is pushed in now, with fixes detailed
below.
>
> Though I wonder why not push the SIGTRAP check to
> linux_is_extended_waitstatus too.
My thinking was to implement something at the same level as WIFSTOPPED
et al.
>
>> gdb/
>> 2014-08-20 Don Breazeal <donb@codesourcery.com>
>>
>> * linux-nat.c (linux_handle_extended_wait): Call
>> linux_ptrace_get_extended_event.
>> (wait_lwp): Call linux_is_extended_waitstatus.
>> (linux_nat_filter_event): Call linux_ptrace_get_extended_event
>> and linux_is_extended_waitstatus.
>> * nat/linux-ptrace.c (linux_test_for_tracefork): Call
>> linux_ptrace_get_extended_event.
>> (linux_ptrace_get_extended_event): New function.
>> (linux_is_extended_waitstatus): New function.
>> * nat/linux-ptrace.h: Declare new functions.
>
> Please spell out the new declarations. Like:
>
> * nat/linux-ptrace.h (linux_ptrace_get_extended_event)
> (linux_is_extended_waitstatus): New declarations.
Fixed.
>
>>
>> gdbserver/
>> 2014-08-20 Don Breazeal <donb@codesourcery.com>
>> * linux-low.c (handle_extended_wait): Call
>
> Missing empty line above.
Fixed.
>
>> linux_ptrace_get_extended_event.
>> (get_stop_pc, get_detach_signal, linux_low_filter_event): Call
>> linux_is_extended_waitstatus.
>
> Thanks,
> Pedro Alves
>
@@ -370,7 +370,7 @@ linux_add_process (int pid, int attached)
static void
handle_extended_wait (struct lwp_info *event_child, int wstat)
{
- int event = wstat >> 16;
+ int event = linux_ptrace_get_extended_event (wstat);
struct thread_info *event_thr = get_lwp_thread (event_child);
struct lwp_info *new_lwp;
@@ -512,7 +512,7 @@ get_stop_pc (struct lwp_info *lwp)
if (WSTOPSIG (lwp->last_status) == SIGTRAP
&& !lwp->stepping
&& !lwp->stopped_by_watchpoint
- && lwp->last_status >> 16 == 0)
+ && !linux_is_extended_waitstatus (lwp->last_status))
stop_pc -= the_low_target.decr_pc_after_break;
if (debug_threads)
@@ -1056,7 +1056,7 @@ get_detach_signal (struct thread_info *thread)
}
/* Extended wait statuses aren't real SIGTRAPs. */
- if (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ if (WSTOPSIG (status) == SIGTRAP && linux_is_extended_waitstatus (status))
{
if (debug_threads)
debug_printf ("GPS: lwp %s had stopped with extended "
@@ -1869,7 +1869,7 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && wstat >> 16 != 0)
+ && linux_is_extended_waitstatus (wstat))
{
handle_extended_wait (child, wstat);
return NULL;
@@ -1800,7 +1800,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
{
int pid = ptid_get_lwp (lp->ptid);
struct target_waitstatus *ourstatus = &lp->waitstatus;
- int event = status >> 16;
+ int event = linux_ptrace_get_extended_event (status);
if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK
|| event == PTRACE_EVENT_CLONE)
@@ -2164,7 +2164,8 @@ wait_lwp (struct lwp_info *lp)
}
/* Handle GNU/Linux's extended waitstatus for trace events. */
- if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
+ && linux_is_extended_waitstatus (status))
{
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
@@ -2723,6 +2724,7 @@ static struct lwp_info *
linux_nat_filter_event (int lwpid, int status, int *new_pending_p)
{
struct lwp_info *lp;
+ int event = linux_ptrace_get_extended_event (status);
*new_pending_p = 0;
@@ -2742,7 +2744,7 @@ linux_nat_filter_event (int lwpid, int status, int *new_pending_p)
thread changes its tid to the tgid. */
if (WIFSTOPPED (status) && lp == NULL
- && (WSTOPSIG (status) == SIGTRAP && status >> 16 == PTRACE_EVENT_EXEC))
+ && (WSTOPSIG (status) == SIGTRAP && event == PTRACE_EVENT_EXEC))
{
/* A multi-thread exec after we had seen the leader exiting. */
if (debug_linux_nat)
@@ -2786,7 +2788,8 @@ linux_nat_filter_event (int lwpid, int status, int *new_pending_p)
}
/* Handle GNU/Linux's extended waitstatus for trace events. */
- if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
+ && linux_is_extended_waitstatus (status))
{
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
@@ -421,7 +421,7 @@ linux_test_for_tracefork (int child_pid)
/* Check if we received a fork event notification. */
if (ret == child_pid && WIFSTOPPED (status)
- && status >> 16 == PTRACE_EVENT_FORK)
+ && linux_ptrace_get_extended_event (status) == PTRACE_EVENT_FORK)
{
/* We did receive a fork event notification. Make sure its PID
is reported. */
@@ -555,3 +555,19 @@ linux_ptrace_set_additional_flags (int flags)
{
additional_flags = flags;
}
+
+/* Extract extended ptrace event from wait status. */
+
+int
+linux_ptrace_get_extended_event (int wstat)
+{
+ return (wstat >> 16);
+}
+
+/* Determine whether wait status denotes an extended event. */
+
+int
+linux_is_extended_waitstatus (int wstat)
+{
+ return (linux_ptrace_get_extended_event (wstat) != 0);
+}
@@ -92,5 +92,7 @@ extern int linux_supports_traceclone (void);
extern int linux_supports_tracevforkdone (void);
extern int linux_supports_tracesysgood (void);
extern void linux_ptrace_set_additional_flags (int);
+extern int linux_ptrace_get_extended_event (int wstat);
+extern int linux_is_extended_waitstatus (int wstat);
#endif /* COMMON_LINUX_PTRACE_H */