On 2017-07-26 09:42 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
>
>> /* Convenience wrapper. Returns true if LWP is presently collecting a
>> fast tracepoint. */
>>
>
> The comments should be updated too.
Yeah of course.
>> -static int
>> +static fast_tpoint_collect_result
>> linux_fast_tracepoint_collecting (struct lwp_info *lwp,
>> struct fast_tpoint_collect_status *status)
>> {
>
>
>> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
>> index 6328da0..18ca6a0 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
>>
>> @@ -358,7 +359,7 @@ struct lwp_info
>> 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;
>> + fast_tpoint_collect_result collecting_fast_tracepoint;
>
> The comments should be updated too.
Indeed.
>>
>> /* 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
>
> Otherwise, the patch is good to me.
Thanks, here is what I pushed:
From 229d26fc9ebca61b8d899cf8fe4342a6cc9795ff Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
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) <collecting_fast_tracepoint>: 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(-)
@@ -1,3 +1,21 @@
+2017-07-26 Simon Marchi <simon.marchi@ericsson.com>
+
+ * 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) <collecting_fast_tracepoint>: 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 <yao.qi@linaro.org>
* linux-x86-low.c (x86_linux_read_description): Re-indent the code.
@@ -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
@@ -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
@@ -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 */
}
}
@@ -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);