This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 7/8] Initialise target descrption after skipping extra traps for --wrapper
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Fri, 24 Jul 2015 12:11:56 +0100
- Subject: Re: [PATCH 7/8] Initialise target descrption after skipping extra traps for --wrapper
- Authentication-results: sourceware.org; auth=none
- References: <1437392126-29503-1-git-send-email-yao dot qi at linaro dot org> <1437392126-29503-8-git-send-email-yao dot qi at linaro dot org> <55B17806 dot 2060000 at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
> Instead of this whole code block, I think we should be able to skip
> the bits in the function that require accessing registers. E.g.,
> this:
>
> /* Cancel actions that rely on GDB not changing the PC (e.g., the
> user used the "jump" command, or "set $pc = foo"). */
> if (lwp->stop_pc != get_pc (lwp))
> {
> /* Collecting 'while-stepping' actions doesn't make sense
> anymore. */
> release_while_stepping_state_list (thread);
> }
>
> Could be guarded by:
>
> if (thread->while_stepping != NULL)
>
> And this:
>
> if (the_low_target.get_pc != NULL)
> {
> struct regcache *regcache = get_thread_regcache (current_thread, 1);
>
> lwp->stop_pc = (*the_low_target.get_pc) (regcache);
>
> if (debug_threads)
> {
> debug_printf (" %s from pc 0x%lx\n", step ? "step" : "continue",
> (long) lwp->stop_pc);
> }
> }
>
> could be guarded by:
>
> if (proc->tdesc == NULL)
>
> Did you try that?
To make sure I understand you correctly, is the change below what you suggested?
If yes, I thought about this approach before, but I didn't try that
because I was worried that we should check every piece of code in
linux_resume_one_lwp_throw and its callees that don't access registers
when target description isn't initialised yet. Especially for
the_low_target.prepare_to_resume, the implementation of this hook should
be aware of that target description may not be available. Nowadays,
prepare_to_resume is only used to update HW debug registers, and
should be OK because GDBserver shouldn't update HW debug registers
before the inferior stops at the first instruction of the program.
However, in nat/x86-linux.c:lwp_debug_registers_changed, I saw such
comments
struct arch_lwp_info *info = lwp_arch_private_info (lwp);
/* NULL means either that this is the main thread still going
through the shell, or that no watchpoint has been set yet.
The debug registers are unchanged in either case. */
I was wondering all the implementations of prepare_to_resume of
different targets should be aware that "thread still going through the
shell"? I'll test this patch on targets other than x86 (such as arm and
aarch64) and see if it causes fails.
--
Yao (éå)
@@ -3651,6 +3671,14 @@ 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;
+ struct process_info *proc = get_thread_process (thread);
+
+ /* Note that target description may not be initialised
+ (proc->tdesc == NULL) at this point because the program hasn't
+ stopped at the first instruction yet. It means GDBserver skips
+ the extra traps from the wrapper program (see option --wrapper).
+ Code in this function that requires register access should be
+ guarded by proc->tdesc == NULL or something else. */
if (lwp->stopped == 0)
return;
@@ -3661,7 +3689,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
/* Cancel actions that rely on GDB not changing the PC (e.g., the
user used the "jump" command, or "set $pc = foo"). */
- if (lwp->stop_pc != get_pc (lwp))
+ if (thread->while_stepping != NULL && lwp->stop_pc != get_pc (lwp))
{
/* Collecting 'while-stepping' actions doesn't make sense
anymore. */
@@ -3787,7 +3815,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
step = 1;
}
- if (the_low_target.get_pc != NULL)
+ if (proc->tdesc != NULL && the_low_target.get_pc != NULL)
{
struct regcache *regcache = get_thread_regcache (current_thread, 1);