Initialise target descrption after skipping extra traps for --wrapper
authorYao Qi <yao.qi@linaro.org>
Fri, 24 Jul 2015 13:40:34 +0000 (14:40 +0100)
committerYao Qi <yao.qi@linaro.org>
Fri, 24 Jul 2015 13:40:34 +0000 (14:40 +0100)
Nowadays, when --wrapper is used, GDBserver skips extra traps/stops
in the wrapper program, and stops at the first instruction of the
program to be debugged.  However, GDBserver created target description
in the first stop of inferior, and the executable of the inferior
is the wrapper program rather than the program to be debugged.  In
this way, the target description can be wrong if the architectures
of wrapper program and program to be debugged are different.  This
is shown by some fails in gdb.server/wrapper.exp on buildbot.

We are testing i686-linux GDB (Fedora-i686) on an x86_64-linux box
(fedora-x86-64-4) in buildbot, such configuration causes fails in
gdb.server/wrapper.exp like this:

spawn /home/gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuite/../../gdb/gdbserver/gdbserver --once --wrapper env TEST=1 -- :2346 /home/gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuite/outputs/gdb.server/wrapper/wrapper
Process /home/gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuite/outputs/gdb.server/wrapper/wrapper created; pid = 8795
Can't debug 64-bit process with 32-bit GDBserver
Exiting
target remote localhost:2346
localhost:2346: Connection timed out.
(gdb) FAIL: gdb.server/wrapper.exp: setting breakpoint at marker

See https://sourceware.org/ml/gdb-testers/2015-q3/msg01541.html

In this case, program to be debugged ("wrapper") is 32-bit but wrapper
program ("/usr/bin/env") is 64-bit, so GDBserver gets the 64-bit
target description instead of 32-bit.

The root cause of this problem is that GDBserver creates target
description too early, and the rationale of fix could be creating
target description once the GDBserver skips extra traps and inferior
stops at the first instruction of the program we want to debug.  IOW,
when GDBserver skips extra traps, the inferior's tdesc is NULL, and
mywait and its callees shouldn't use inferior's tdesc, so in this
patch, we skip code that requires register access, see changes in
linux_resume_one_lwp_throw and need_step_over_p.

In linux_low_filter_event, if target description isn't initialised and
GDBserver attached the process, we create target description immediately,
because GDBserver don't have to skip extra traps for attach, IOW, it
makes no sense to use --attach and --wrapper together.  Otherwise, the
process is launched by GDBserver, we keep the status pending, and return.

After GDBserver skipped extra traps in start_inferior, we call a
target_ops hook arch_setup to initialise target description there.

gdb/gdbserver:

2015-07-24  Yao Qi  <yao.qi@linaro.org>

* linux-low.c (linux_arch_setup): New function.
(linux_low_filter_event): If proc->tdesc is NULL and
proc->attached is true, call the_low_target.arch_setup.
Otherwise, keep status pending, and return.
(linux_resume_one_lwp_throw): Don't call get_pc if
thread->while_stepping isn't NULL.  Don't call
get_thread_regcache if proc->tdesc is NULL.
(need_step_over_p): Return 0 if proc->tdesc is NULL.
(linux_target_ops): Install arch_setup.
* server.c (start_inferior): Call the_target->arch_setup.
* target.h (struct target_ops) <arch_setup>: New field.
(target_arch_setup): New marco.
* lynx-low.c (lynx_target_ops): Update.
* nto-low.c (nto_target_ops): Update.
* spu-low.c (spu_target_ops): Update.
* win32-low.c (win32_target_ops): Update.

gdb/gdbserver/ChangeLog
gdb/gdbserver/linux-low.c
gdb/gdbserver/lynx-low.c
gdb/gdbserver/nto-low.c
gdb/gdbserver/server.c
gdb/gdbserver/spu-low.c
gdb/gdbserver/target.h
gdb/gdbserver/win32-low.c

index d215d7a39ee284612dc2c28e85e1f7a01e695843..cff2f366b2a16a67199fc6e3cb3101577a45a2e0 100644 (file)
@@ -1,3 +1,22 @@
+2015-07-24  Yao Qi  <yao.qi@linaro.org>
+
+       * linux-low.c (linux_arch_setup): New function.
+       (linux_low_filter_event): If proc->tdesc is NULL and
+       proc->attached is true, call the_low_target.arch_setup.
+       Otherwise, keep status pending, and return.
+       (linux_resume_one_lwp_throw): Don't call get_pc if
+       thread->while_stepping isn't NULL.  Don't call
+       get_thread_regcache if proc->tdesc is NULL.
+       (need_step_over_p): Return 0 if proc->tdesc is NULL.
+       (linux_target_ops): Install arch_setup.
+       * server.c (start_inferior): Call the_target->arch_setup.
+       * target.h (struct target_ops) <arch_setup>: New field.
+       (target_arch_setup): New marco.
+       * lynx-low.c (lynx_target_ops): Update.
+       * nto-low.c (nto_target_ops): Update.
+       * spu-low.c (spu_target_ops): Update.
+       * win32-low.c (win32_target_ops): Update.
+
 2015-07-24  Yao Qi  <yao.qi@linaro.org>
 
        * linux-low.c (linux_add_process): Don't set
index fa9dc29ded406754565201bb26797c43b79dc1b8..ac1ad6f2147eb971336c714ceeb519a1b1ffe4ec 100644 (file)
@@ -822,6 +822,14 @@ linux_create_inferior (char *program, char **allargs)
   return pid;
 }
 
+/* Implement the arch_setup target_ops method.  */
+
+static void
+linux_arch_setup (void)
+{
+  the_low_target.arch_setup ();
+}
+
 /* Attach to an inferior process.  Returns 0 on success, ERRNO on
    error.  */
 
@@ -2105,23 +2113,35 @@ linux_low_filter_event (int lwpid, int wstat)
     {
       struct process_info *proc;
 
-      /* Architecture-specific setup after inferior is running.  This
-        needs to happen after we have attached to the inferior and it
-        is stopped for the first time, but before we access any
-        inferior registers.  */
+      /* Architecture-specific setup after inferior is running.  */
       proc = find_process_pid (pid_of (thread));
-      if (proc->priv->new_inferior)
+      if (proc->tdesc == NULL)
        {
-         struct thread_info *saved_thread;
+         if (proc->attached)
+           {
+             struct thread_info *saved_thread;
 
-         saved_thread = current_thread;
-         current_thread = thread;
+             /* This needs to happen after we have attached to the
+                inferior and it is stopped for the first time, but
+                before we access any inferior registers.  */
+             saved_thread = current_thread;
+             current_thread = thread;
 
-         the_low_target.arch_setup ();
+             the_low_target.arch_setup ();
 
-         current_thread = saved_thread;
+             current_thread = saved_thread;
 
-         proc->priv->new_inferior = 0;
+             proc->priv->new_inferior = 0;
+           }
+         else
+           {
+             /* The process is started, but GDBserver will do
+                architecture-specific setup after the program stops at
+                the first instruction.  */
+             child->status_pending_p = 1;
+             child->status_pending = wstat;
+             return child;
+           }
        }
     }
 
@@ -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);
 
@@ -4014,6 +4042,12 @@ need_step_over_p (struct inferior_list_entry *entry, void *dummy)
   struct lwp_info *lwp = get_thread_lwp (thread);
   struct thread_info *saved_thread;
   CORE_ADDR pc;
+  struct process_info *proc = get_thread_process (thread);
+
+  /* GDBserver is skipping the extra traps from the wrapper program,
+     don't have to do step over.  */
+  if (proc->tdesc == NULL)
+    return 0;
 
   /* LWPs which will not be resumed are not interesting, because we
      might not wait for them next time through linux_wait.  */
@@ -6635,6 +6669,7 @@ current_lwp_ptid (void)
 
 static struct target_ops linux_target_ops = {
   linux_create_inferior,
+  linux_arch_setup,
   linux_attach,
   linux_kill,
   linux_detach,
index ee7b28a3a0b6188abbbcad87f48cf9b7795e397a..5cf03be3d26877ab0f2a64b1420352afe5d74674 100644 (file)
@@ -722,6 +722,7 @@ lynx_request_interrupt (void)
 
 static struct target_ops lynx_target_ops = {
   lynx_create_inferior,
+  NULL,  /* arch_setup */
   lynx_attach,
   lynx_kill,
   lynx_detach,
index 92767362043f5a8dfaa68074fdaa9f38842a8664..19f492ff7701e92bf0991611a7f99ff37413b3a1 100644 (file)
@@ -925,6 +925,7 @@ nto_supports_non_stop (void)
 
 static struct target_ops nto_target_ops = {
   nto_create_inferior,
+  NULL,  /* arch_setup */
   nto_attach,
   nto_kill,
   nto_detach,
index 36e8987d8312baa48cc1dd1166c972ed960d380a..29187704c1972e021b1fceb77d01ee5b659c2ae1 100644 (file)
@@ -272,6 +272,7 @@ start_inferior (char **argv)
            }
          while (last_status.value.sig != GDB_SIGNAL_TRAP);
        }
+      target_arch_setup ();
       return signal_pid;
     }
 
@@ -279,6 +280,8 @@ start_inferior (char **argv)
      (assuming success).  */
   last_ptid = mywait (pid_to_ptid (signal_pid), &last_status, 0, 0);
 
+  target_arch_setup ();
+
   if (last_status.kind != TARGET_WAITKIND_EXITED
       && last_status.kind != TARGET_WAITKIND_SIGNALLED)
     {
index a56a8893c4a3179b22a73025121e4f17653e451f..cbee960d8a0495b0296f3aacf2531e22d885775d 100644 (file)
@@ -638,6 +638,7 @@ spu_request_interrupt (void)
 
 static struct target_ops spu_target_ops = {
   spu_create_inferior,
+  NULL,  /* arch_setup */
   spu_attach,
   spu_kill,
   spu_detach,
index 9a4086708cf3b15c1931c21d816403d38d4dabf0..fefd8d180ecadc448244c042e93955423846ebe6 100644 (file)
@@ -74,6 +74,9 @@ struct target_ops
 
   int (*create_inferior) (char *program, char **args);
 
+  /* Architecture-specific setup.  */
+  void (*arch_setup) (void);
+
   /* Attach to a running process.
 
      PID is the process ID to attach to, specified by the user
@@ -445,6 +448,13 @@ void set_target_ops (struct target_ops *);
 #define create_inferior(program, args) \
   (*the_target->create_inferior) (program, args)
 
+#define target_arch_setup()                     \
+  do                                            \
+    {                                           \
+      if (the_target->arch_setup != NULL)       \
+       (*the_target->arch_setup) ();            \
+    } while (0)
+
 #define myattach(pid) \
   (*the_target->attach) (pid)
 
index 64caf2483934352affe0ffd32d5669b45a9dda24..7ccb3dde182b0f439a330bcc599f8829c09d3609 100644 (file)
@@ -1785,6 +1785,7 @@ win32_get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 
 static struct target_ops win32_target_ops = {
   win32_create_inferior,
+  NULL,  /* arch_setup */
   win32_attach,
   win32_kill,
   win32_detach,