From 229d26fc9ebca61b8d899cf8fe4342a6cc9795ff Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 26 Jul 2017 10:55:35 +0200 Subject: [PATCH] Add enum for result of fast_tracepoint_collecting I got confused by the result value of fast_tracepoint_collecting, while it sounds like it would return true/false (whether the thread is collecting or not), it actually returns: 0: not collecting 1: in the jump pad, before the relocated instruction 2: in the jump pad, at or after the relocated instruction To avoid confusion, I think it would be nice to make it return an enum. If you can help find a shorter but still relavant name, it would be awesome. Otherwise, we'll go with that, fast_tpoint_collect_result, which is at least consistent with the existing fast_tpoint_collect_status. gdb/gdbserver/ChangeLog: * tracepoint.h (enum class fast_tpoint_collect_result): New enumeration. (fast_tracepoint_collecting): Change return type to fast_tpoint_collect_result. * tracepoint.c (fast_tracepoint_collecting): Likewise. * linux-low.h: Include tracepoint.h. (struct lwp_info) : Change type to fast_tpoint_collect_result. * linux-low.c (handle_tracepoints): Adjust. (linux_fast_tracepoint_collecting): Change return type to fast_tpoint_collect_result. (maybe_move_out_of_jump_pad, linux_wait_for_event_filtered, linux_wait_1, stuck_in_jump_pad_callback, lwp_signal_can_be_delivered, linux_resume_one_lwp_throw, proceed_one_lwp): Adjust to type change. --- gdb/gdbserver/ChangeLog | 18 +++++++++++ gdb/gdbserver/linux-low.c | 65 +++++++++++++++++++++++--------------- gdb/gdbserver/linux-low.h | 12 +++---- gdb/gdbserver/tracepoint.c | 18 +++++------ gdb/gdbserver/tracepoint.h | 22 +++++++++++-- 5 files changed, 91 insertions(+), 44 deletions(-) diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index c505ccfe6ef..fdc07f295df 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,21 @@ +2017-07-26 Simon Marchi + + * tracepoint.h (enum class fast_tpoint_collect_result): New + enumeration. + (fast_tracepoint_collecting): Change return type to + fast_tpoint_collect_result. + * tracepoint.c (fast_tracepoint_collecting): Likewise. + * linux-low.h: Include tracepoint.h. + (struct lwp_info) : Change type to + fast_tpoint_collect_result. + * linux-low.c (handle_tracepoints): Adjust. + (linux_fast_tracepoint_collecting): Change return type to + fast_tpoint_collect_result. + (maybe_move_out_of_jump_pad, linux_wait_for_event_filtered, + linux_wait_1, stuck_in_jump_pad_callback, + lwp_signal_can_be_delivered, linux_resume_one_lwp_throw, + proceed_one_lwp): Adjust to type change. + 2017-07-10 Yao Qi * linux-x86-low.c (x86_linux_read_description): Re-indent the code. diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 3d7cfe31b5a..fd46d85ed5a 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -2082,7 +2082,9 @@ handle_tracepoints (struct lwp_info *lwp) lwp_suspended_decr (lwp); gdb_assert (lwp->suspended == 0); - gdb_assert (!stabilizing_threads || lwp->collecting_fast_tracepoint); + gdb_assert (!stabilizing_threads + || (lwp->collecting_fast_tracepoint + != fast_tpoint_collect_result::not_collecting)); if (tpoint_related_event) { @@ -2094,10 +2096,10 @@ handle_tracepoints (struct lwp_info *lwp) return 0; } -/* Convenience wrapper. Returns true if LWP is presently collecting a - fast tracepoint. */ +/* Convenience wrapper. Returns information about LWP's fast tracepoint + collection status. */ -static int +static fast_tpoint_collect_result linux_fast_tracepoint_collecting (struct lwp_info *lwp, struct fast_tpoint_collect_status *status) { @@ -2105,14 +2107,14 @@ linux_fast_tracepoint_collecting (struct lwp_info *lwp, struct thread_info *thread = get_lwp_thread (lwp); if (the_low_target.get_thread_area == NULL) - return 0; + return fast_tpoint_collect_result::not_collecting; /* Get the thread area address. This is used to recognize which thread is which when tracing with the in-process agent library. We don't read anything from the address, and treat it as opaque; it's the address itself that we assume is unique per-thread. */ if ((*the_low_target.get_thread_area) (lwpid_of (thread), &thread_area) == -1) - return 0; + return fast_tpoint_collect_result::not_collecting; return fast_tracepoint_collecting (thread_area, lwp->stop_pc, status); } @@ -2136,14 +2138,14 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat) && agent_loaded_p ()) { struct fast_tpoint_collect_status status; - int r; if (debug_threads) debug_printf ("Checking whether LWP %ld needs to move out of the " "jump pad.\n", lwpid_of (current_thread)); - r = linux_fast_tracepoint_collecting (lwp, &status); + fast_tpoint_collect_result r + = linux_fast_tracepoint_collecting (lwp, &status); if (wstat == NULL || (WSTOPSIG (*wstat) != SIGILL @@ -2153,9 +2155,10 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat) { lwp->collecting_fast_tracepoint = r; - if (r != 0) + if (r != fast_tpoint_collect_result::not_collecting) { - if (r == 1 && lwp->exit_jump_pad_bkpt == NULL) + if (r == fast_tpoint_collect_result::before_insn + && lwp->exit_jump_pad_bkpt == NULL) { /* Haven't executed the original instruction yet. Set breakpoint there, and wait till it's hit, @@ -2181,9 +2184,10 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat) reporting to GDB. Otherwise, it's an IPA lib bug: just report the signal to GDB, and pray for the best. */ - lwp->collecting_fast_tracepoint = 0; + lwp->collecting_fast_tracepoint + = fast_tpoint_collect_result::not_collecting; - if (r != 0 + if (r != fast_tpoint_collect_result::not_collecting && (status.adjusted_insn_addr <= lwp->stop_pc && lwp->stop_pc < status.adjusted_insn_addr_end)) { @@ -2715,7 +2719,8 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid, if (stopping_threads == NOT_STOPPING_THREADS && requested_child->status_pending_p - && requested_child->collecting_fast_tracepoint) + && (requested_child->collecting_fast_tracepoint + != fast_tpoint_collect_result::not_collecting)) { enqueue_one_deferred_signal (requested_child, &requested_child->status_pending); @@ -3467,20 +3472,22 @@ linux_wait_1 (ptid_t ptid, } } - if (event_child->collecting_fast_tracepoint) + if (event_child->collecting_fast_tracepoint + != fast_tpoint_collect_result::not_collecting) { if (debug_threads) debug_printf ("LWP %ld was trying to move out of the jump pad (%d). " "Check if we're already there.\n", lwpid_of (current_thread), - event_child->collecting_fast_tracepoint); + (int) event_child->collecting_fast_tracepoint); trace_event = 1; event_child->collecting_fast_tracepoint = linux_fast_tracepoint_collecting (event_child, NULL); - if (event_child->collecting_fast_tracepoint != 1) + if (event_child->collecting_fast_tracepoint + != fast_tpoint_collect_result::before_insn) { /* No longer need this breakpoint. */ if (event_child->exit_jump_pad_bkpt != NULL) @@ -3507,7 +3514,8 @@ linux_wait_1 (ptid_t ptid, } } - if (event_child->collecting_fast_tracepoint == 0) + if (event_child->collecting_fast_tracepoint + == fast_tpoint_collect_result::not_collecting) { if (debug_threads) debug_printf ("fast tracepoint finished " @@ -4192,7 +4200,8 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data) && (gdb_breakpoint_here (lwp->stop_pc) || lwp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT || thread->last_resume_kind == resume_step) - && linux_fast_tracepoint_collecting (lwp, NULL)); + && (linux_fast_tracepoint_collecting (lwp, NULL) + != fast_tpoint_collect_result::not_collecting)); } static void @@ -4369,7 +4378,8 @@ single_step (struct lwp_info* lwp) static int lwp_signal_can_be_delivered (struct lwp_info *lwp) { - return !lwp->collecting_fast_tracepoint; + return (lwp->collecting_fast_tracepoint + == fast_tpoint_collect_result::not_collecting); } /* Resume execution of LWP. If STEP is nonzero, single-step it. If @@ -4381,7 +4391,6 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, { struct thread_info *thread = get_lwp_thread (lwp); struct thread_info *saved_thread; - int fast_tp_collecting; int ptrace_request; struct process_info *proc = get_thread_process (thread); @@ -4397,9 +4406,12 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, gdb_assert (lwp->waitstatus.kind == TARGET_WAITKIND_IGNORE); - fast_tp_collecting = lwp->collecting_fast_tracepoint; + fast_tpoint_collect_result fast_tp_collecting + = lwp->collecting_fast_tracepoint; - gdb_assert (!stabilizing_threads || fast_tp_collecting); + gdb_assert (!stabilizing_threads + || (fast_tp_collecting + != fast_tpoint_collect_result::not_collecting)); /* Cancel actions that rely on GDB not changing the PC (e.g., the user used the "jump" command, or "set $pc = foo"). */ @@ -4455,7 +4467,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, if (can_hardware_single_step ()) { - if (fast_tp_collecting == 0) + if (fast_tp_collecting == fast_tpoint_collect_result::not_collecting) { if (step == 0) warning ("BAD - reinserting but not stepping."); @@ -4468,14 +4480,14 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, step = maybe_hw_step (thread); } - if (fast_tp_collecting == 1) + if (fast_tp_collecting == fast_tpoint_collect_result::before_insn) { if (debug_threads) debug_printf ("lwp %ld wants to get out of fast tracepoint jump pad" " (exit-jump-pad-bkpt)\n", lwpid_of (thread)); } - else if (fast_tp_collecting == 2) + else if (fast_tp_collecting == fast_tpoint_collect_result::at_insn) { if (debug_threads) debug_printf ("lwp %ld wants to get out of fast tracepoint jump pad" @@ -5294,7 +5306,8 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except) if (thread->last_resume_kind == resume_stop && lwp->pending_signals_to_report == NULL - && lwp->collecting_fast_tracepoint == 0) + && (lwp->collecting_fast_tracepoint + == fast_tpoint_collect_result::not_collecting)) { /* We haven't reported this LWP as stopped yet (otherwise, the last_status.kind check above would catch it, and we wouldn't diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h index 6328da03ed8..2bf7e7cd782 100644 --- a/gdb/gdbserver/linux-low.h +++ b/gdb/gdbserver/linux-low.h @@ -26,6 +26,7 @@ /* Included for ptrace type definitions. */ #include "nat/linux-ptrace.h" #include "target/waitstatus.h" /* For enum target_stop_reason. */ +#include "tracepoint.h" #define PTRACE_XFER_TYPE long @@ -353,12 +354,11 @@ struct lwp_info and then processed and cleared in linux_resume_one_lwp. */ struct thread_resume *resume; - /* True if it is known that this lwp is presently collecting a fast - tracepoint (it is in the jump pad or in some code that will - return to the jump pad. Normally, we won't care about this, but - we will if a signal arrives to this lwp while it is - collecting. */ - int collecting_fast_tracepoint; + /* Information bout this lwp's fast tracepoint collection status (is it + currently stopped in the jump pad, and if so, before or at/after the + relocated instruction). Normally, we won't care about this, but we will + if a signal arrives to this lwp while it is collecting. */ + fast_tpoint_collect_result collecting_fast_tracepoint; /* If this is non-zero, it points to a chain of signals which need to be reported to GDB. These were deferred because the thread diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index d4fe76a45a8..68ce10f5ef0 100644 --- a/gdb/gdbserver/tracepoint.c +++ b/gdb/gdbserver/tracepoint.c @@ -5581,7 +5581,7 @@ force_unlock_trace_buffer (void) case, if we want to move the thread out of the jump pad, we need to single-step it until this function returns 0. */ -int +fast_tpoint_collect_result fast_tracepoint_collecting (CORE_ADDR thread_area, CORE_ADDR stop_pc, struct fast_tpoint_collect_status *status) @@ -5656,7 +5656,7 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, if (tpoint == NULL) { warning ("in jump pad, but no matching tpoint?"); - return 0; + return fast_tpoint_collect_result::not_collecting; } else { @@ -5684,7 +5684,7 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, if (tpoint == NULL) { warning ("in trampoline, but no matching tpoint?"); - return 0; + return fast_tpoint_collect_result::not_collecting; } else { @@ -5712,14 +5712,14 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, { trace_debug ("fast_tracepoint_collecting:" " failed reading 'collecting' in the inferior"); - return 0; + return fast_tpoint_collect_result::not_collecting; } if (!ipa_collecting) { trace_debug ("fast_tracepoint_collecting: not collecting" " (and nobody is)."); - return 0; + return fast_tpoint_collect_result::not_collecting; } /* Some thread is collecting. Check which. */ @@ -5732,7 +5732,7 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, { trace_debug ("fast_tracepoint_collecting: not collecting " "(another thread is)"); - return 0; + return fast_tpoint_collect_result::not_collecting; } tpoint @@ -5742,7 +5742,7 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, warning ("fast_tracepoint_collecting: collecting, " "but tpoint %s not found?", paddress ((CORE_ADDR) ipa_collecting_obj.tpoint)); - return 0; + return fast_tpoint_collect_result::not_collecting; } /* The thread is within `gdb_collect', skip over the rest of @@ -5769,7 +5769,7 @@ fast_tracepoint_collecting (CORE_ADDR thread_area, fast_tracepoint_collecting, returning continue-until-break at %s", paddress (tpoint->adjusted_insn_addr)); - return 1; /* continue */ + return fast_tpoint_collect_result::before_insn; /* continue */ } else { @@ -5780,7 +5780,7 @@ fast_tracepoint_collecting, returning continue-until-break at %s", paddress (tpoint->adjusted_insn_addr), paddress (tpoint->adjusted_insn_addr_end)); - return 2; /* single-step */ + return fast_tpoint_collect_result::at_insn; /* single-step */ } } diff --git a/gdb/gdbserver/tracepoint.h b/gdb/gdbserver/tracepoint.h index 799a16c778b..31103a32457 100644 --- a/gdb/gdbserver/tracepoint.h +++ b/gdb/gdbserver/tracepoint.h @@ -115,9 +115,25 @@ struct fast_tpoint_collect_status CORE_ADDR adjusted_insn_addr_end; }; -int fast_tracepoint_collecting (CORE_ADDR thread_area, - CORE_ADDR stop_pc, - struct fast_tpoint_collect_status *status); +/* The possible states a thread can be in, related to the collection of fast + tracepoint. */ + +enum class fast_tpoint_collect_result +{ + /* Not collecting a fast tracepoint. */ + not_collecting, + + /* In the jump pad, but before the relocated instruction. */ + before_insn, + + /* In the jump pad, but at (or after) the relocated instruction. */ + at_insn, +}; + +fast_tpoint_collect_result fast_tracepoint_collecting + (CORE_ADDR thread_area, CORE_ADDR stop_pc, + struct fast_tpoint_collect_status *status); + void force_unlock_trace_buffer (void); int handle_tracepoint_bkpts (struct thread_info *tinfo, CORE_ADDR stop_pc); -- 2.30.2