Re: [PATCH 7/8] Initialise target descrption after skipping extra traps for --wrapper

[Yao Qi <qiyaoltc@gmail.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);