[PATCH 3/3] Fix "Remote 'g' packet reply is too long" problems with multiple inferiors

[Pedro Alves <palves at redhat dot com>]

When debugging two inferiors (or more) against gdbserver, and the
inferiors have different architectures, such as e.g., on x86_64
GNU/Linux and one inferior is 64-bit while the other is 32-bit, then
GDB can get confused with the different architectures in a couple
spots.

In both cases I ran into, GDB incorrectly ended up using the
architecture of whatever happens to be the selected inferior instead
of the architecture of some other given inferior:

#1 - When parsing the expedited registers in stop replies.

#2 - In the default implementation of the target_thread_architecture
     target method.

These resulted in instances of the infamous "Remote 'g' packet reply
is too long" error.  For example, with the test added in this commit,
we get:

~~~
  Continuing.
  Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): ad064000000000000[snip]
  (gdb) FAIL: gdb.multi/multi-arch.exp: inf1 event with inf2 selected: continue to hello_loop

  c
  Continuing.
  Truncated register 50 in remote 'g' packet
  (gdb) PASS: gdb.multi/multi-arch.exp: inf2 event with inf1 selected: c
~~~

This commit fixes that.

gdb/ChangeLog:
2017-10-02  Pedro Alves  <palves@redhat.com>

	* remote.c (get_remote_arch_state): New 'gdbarch' parameter.  Use
	it instead of target_gdbarch.
	(get_remote_state, get_remote_packet_size): Adjust
	get_remote_arch_state calls, passing down target_gdbarch
	explicitly.
	(packet_reg_from_regnum, packet_reg_from_pnum): New parameter
	'gdbarch' and use it instead of target_gdbarch.
	(get_memory_packet_size): Adjust get_remote_arch_state calls,
	passing down target_gdbarch explicitly.
	(remote_parse_stop_reply): Use the stopped thread's architecture,
	not the current inferior's.
	(process_g_packet, remote_fetch_registers)
	(remote_prepare_to_store, store_registers_using_G)
	(remote_store_registers): Adjust get_remote_arch_state calls,
	using the regcache's architecture.
	(remote_get_trace_status): Adjust get_remote_arch_state calls,
	passing down target_gdbarch explicitly.
	* target.c (default_thread_architecture): Use the sepecified
	inferior's architecture, instead of the current inferior's
	architecture (via target_gdbarch).

gdb/testsuite/ChangeLog:
2017-10-02  Pedro Alves  <palves@redhat.com>

	* gdb.multi/hangout.c: Include <unistd.h>.
	(hangout_loop): New function.
	(main): Call alarm.  Call hangout_loop in a loop.
	* gdb.multi/hello.c: Include <unistd.h>.
	(hello_loop): New function.
	(main): Call alarm.  Call hangout_loop in a loop.
	* gdb.multi/multi-arch.exp: Test running to a breakpoint one
	inferior with the other selected.
---
 gdb/remote.c                           | 92 ++++++++++++++++++++++++----------
 gdb/target.c                           |  4 +-
 gdb/testsuite/gdb.multi/hangout.c      | 14 ++++++
 gdb/testsuite/gdb.multi/hello.c        | 15 +++++-
 gdb/testsuite/gdb.multi/multi-arch.exp | 24 +++++++++
 5 files changed, 121 insertions(+), 28 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 54d810f..b6a81a2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -654,11 +654,11 @@ remote_get_noisy_reply ()
 static struct gdbarch_data *remote_gdbarch_data_handle;
 
 static struct remote_arch_state *
-get_remote_arch_state (void)
+get_remote_arch_state (struct gdbarch *gdbarch)
 {
-  gdb_assert (target_gdbarch () != NULL);
+  gdb_assert (gdbarch != NULL);
   return ((struct remote_arch_state *)
-	  gdbarch_data (target_gdbarch (), remote_gdbarch_data_handle));
+	  gdbarch_data (gdbarch, remote_gdbarch_data_handle));
 }
 
 /* Fetch the global remote target state.  */
@@ -671,7 +671,7 @@ get_remote_state (void)
      function which calls getpkt also needs to be mindful of changes
      to rs->buf, but this call limits the number of places which run
      into trouble.  */
-  get_remote_arch_state ();
+  get_remote_arch_state (target_gdbarch ());
 
   return get_remote_state_raw ();
 }
@@ -878,7 +878,7 @@ static long
 get_remote_packet_size (void)
 {
   struct remote_state *rs = get_remote_state ();
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ());
 
   if (rs->explicit_packet_size)
     return rs->explicit_packet_size;
@@ -887,9 +887,10 @@ get_remote_packet_size (void)
 }
 
 static struct packet_reg *
-packet_reg_from_regnum (struct remote_arch_state *rsa, long regnum)
+packet_reg_from_regnum (struct gdbarch *gdbarch, struct remote_arch_state *rsa,
+			long regnum)
 {
-  if (regnum < 0 && regnum >= gdbarch_num_regs (target_gdbarch ()))
+  if (regnum < 0 && regnum >= gdbarch_num_regs (gdbarch))
     return NULL;
   else
     {
@@ -901,11 +902,12 @@ packet_reg_from_regnum (struct remote_arch_state *rsa, long regnum)
 }
 
 static struct packet_reg *
-packet_reg_from_pnum (struct remote_arch_state *rsa, LONGEST pnum)
+packet_reg_from_pnum (struct gdbarch *gdbarch, struct remote_arch_state *rsa,
+		      LONGEST pnum)
 {
   int i;
 
-  for (i = 0; i < gdbarch_num_regs (target_gdbarch ()); i++)
+  for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
     {
       struct packet_reg *r = &rsa->regs[i];
 
@@ -1049,7 +1051,7 @@ static long
 get_memory_packet_size (struct memory_packet_config *config)
 {
   struct remote_state *rs = get_remote_state ();
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ());
 
   long what_they_get;
   if (config->fixed_p)
@@ -6835,7 +6837,8 @@ strprefix (const char *p, const char *pend, const char *prefix)
 static void
 remote_parse_stop_reply (char *buf, struct stop_reply *event)
 {
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  remote_arch_state *rsa = NULL;
+  struct gdbarch *reply_arch = NULL;
   ULONGEST addr;
   const char *p;
   int skipregs = 0;
@@ -7015,9 +7018,43 @@ Packet: '%s'\n"),
 		 reason.  */
 	      if (p_temp == p1)
 		{
-		  struct packet_reg *reg = packet_reg_from_pnum (rsa, pnum);
+		  /* If we haven't parsed the event's thread yet, find
+		     it now, in order to find the architecture of the
+		     reported expedited registers.  */
+		  if (event->ptid == null_ptid)
+		    {
+		      const char *thr = strstr (p1 + 1, ";thread:");
+		      if (thr != NULL)
+			event->ptid = read_ptid (thr + strlen (";thread:"),
+						 NULL);
+		      else
+			event->ptid = magic_null_ptid;
+		    }
+
+		  if (rsa == NULL)
+		    {
+		      inferior *inf = (event->ptid == null_ptid
+				       ? NULL
+				       : find_inferior_ptid (event->ptid));
+		      /* If this is the first time we learn anything
+			 about this process, skip the registers
+			 included in this packet, since we don't yet
+			 know which architecture to use to parse
+			 them.  */
+		      if (inf == NULL)
+			{
+			  p = strchrnul (p1 + 1, ';');
+			  p++;
+			  continue;
+			}
+
+		      reply_arch = inf->gdbarch;
+		      rsa = get_remote_arch_state (reply_arch);
+		    }
+
+		  struct packet_reg *reg = packet_reg_from_pnum (reply_arch,
+								 rsa, pnum);
 		  cached_reg_t cached_reg;
-		  struct gdbarch *gdbarch = target_gdbarch ();
 
 		  if (reg == NULL)
 		    error (_("Remote sent bad register number %s: %s\n\
@@ -7026,13 +7063,13 @@ Packet: '%s'\n"),
 
 		  cached_reg.num = reg->regnum;
 		  cached_reg.data = (gdb_byte *)
-		    xmalloc (register_size (gdbarch, reg->regnum));
+		    xmalloc (register_size (reply_arch, reg->regnum));
 
 		  p = p1 + 1;
 		  fieldsize = hex2bin (p, cached_reg.data,
-				       register_size (gdbarch, reg->regnum));
+				       register_size (reply_arch, reg->regnum));
 		  p += 2 * fieldsize;
-		  if (fieldsize < register_size (gdbarch, reg->regnum))
+		  if (fieldsize < register_size (reply_arch, reg->regnum))
 		    warning (_("Remote reply is too short: %s"), buf);
 
 		  VEC_safe_push (cached_reg_t, event->regcache, &cached_reg);
@@ -7605,7 +7642,7 @@ process_g_packet (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   struct remote_state *rs = get_remote_state ();
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct remote_arch_state *rsa = get_remote_arch_state (gdbarch);
   int i, buf_len;
   char *p;
   char *regs;
@@ -7739,7 +7776,8 @@ static void
 remote_fetch_registers (struct target_ops *ops,
 			struct regcache *regcache, int regnum)
 {
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  remote_arch_state *rsa = get_remote_arch_state (gdbarch);
   int i;
 
   set_remote_traceframe ();
@@ -7747,7 +7785,7 @@ remote_fetch_registers (struct target_ops *ops,
 
   if (regnum >= 0)
     {
-      struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum);
+      struct packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum);
 
       gdb_assert (reg != NULL);
 
@@ -7773,7 +7811,7 @@ remote_fetch_registers (struct target_ops *ops,
 
   fetch_registers_using_g (regcache);
 
-  for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++)
+  for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
     if (!rsa->regs[i].in_g_packet)
       if (!fetch_register_using_p (regcache, &rsa->regs[i]))
 	{
@@ -7789,7 +7827,7 @@ remote_fetch_registers (struct target_ops *ops,
 static void
 remote_prepare_to_store (struct target_ops *self, struct regcache *regcache)
 {
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct remote_arch_state *rsa = get_remote_arch_state (regcache->arch ());
   int i;
 
   /* Make sure the entire registers array is valid.  */
@@ -7855,7 +7893,7 @@ static void
 store_registers_using_G (const struct regcache *regcache)
 {
   struct remote_state *rs = get_remote_state ();
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  remote_arch_state *rsa = get_remote_arch_state (regcache->arch ());
   gdb_byte *regs;
   char *p;
 
@@ -7894,7 +7932,8 @@ static void
 remote_store_registers (struct target_ops *ops,
 			struct regcache *regcache, int regnum)
 {
-  struct remote_arch_state *rsa = get_remote_arch_state ();
+  struct gdbarch *gdbarch = regcache->arch ();
+  remote_arch_state *rsa = get_remote_arch_state (gdbarch);
   int i;
 
   set_remote_traceframe ();
@@ -7902,7 +7941,7 @@ remote_store_registers (struct target_ops *ops,
 
   if (regnum >= 0)
     {
-      struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum);
+      struct packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum);
 
       gdb_assert (reg != NULL);
 
@@ -7926,7 +7965,7 @@ remote_store_registers (struct target_ops *ops,
 
   store_registers_using_G (regcache);
 
-  for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++)
+  for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
     if (!rsa->regs[i].in_g_packet)
       if (!store_register_using_P (regcache, &rsa->regs[i]))
 	/* See above for why we do not issue an error here.  */
@@ -12653,7 +12692,8 @@ remote_get_trace_status (struct target_ops *self, struct trace_status *ts)
   if (packet_support (PACKET_qTStatus) == PACKET_DISABLE)
     return -1;
 
-  trace_regblock_size = get_remote_arch_state ()->sizeof_g_packet;
+  trace_regblock_size
+    = get_remote_arch_state (target_gdbarch ())->sizeof_g_packet;
 
   putpkt ("qTStatus");
 
diff --git a/gdb/target.c b/gdb/target.c
index 733c086..cb66158 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3131,7 +3131,9 @@ default_watchpoint_addr_within_range (struct target_ops *target,
 struct gdbarch *
 default_thread_architecture (struct target_ops *ops, ptid_t ptid)
 {
-  return target_gdbarch ();
+  inferior *inf = find_inferior_ptid (ptid);
+  gdb_assert (inf != NULL);
+  return inf->gdbarch;
 }
 
 static int
diff --git a/gdb/testsuite/gdb.multi/hangout.c b/gdb/testsuite/gdb.multi/hangout.c
index 44dfba6..7266a18 100644
--- a/gdb/testsuite/gdb.multi/hangout.c
+++ b/gdb/testsuite/gdb.multi/hangout.c
@@ -16,14 +16,28 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <stdio.h>
+#include <unistd.h>
+
+static void
+hangout_loop (void)
+{
+}
 
 int
 main(int argc, char *argv[])
 {
   int i;
 
+  alarm (30);
+
   for (i = 0; i < argc; ++i)
     {
       printf("Arg %d is %s\n", i, argv[i]);
     }
+
+  while (1)
+    {
+      hangout_loop ();
+      usleep (20);
+    }
 }
diff --git a/gdb/testsuite/gdb.multi/hello.c b/gdb/testsuite/gdb.multi/hello.c
index 2c5bee9..7cc5f3c 100644
--- a/gdb/testsuite/gdb.multi/hello.c
+++ b/gdb/testsuite/gdb.multi/hello.c
@@ -16,6 +16,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <stdlib.h>
+#include <unistd.h>
 
 short hglob = 1;
 
@@ -37,15 +38,27 @@ hello(int x)
   return x + 45;
 }
 
+static void
+hello_loop (void)
+{
+}
+
 int
 main()
 {
   int tmpx;
 
+  alarm (30);
+
   bar();
   tmpx = hello(glob);
   commonfun();
   glob = tmpx;
   commonfun();
-}
 
+  while (1)
+    {
+      hello_loop ();
+      usleep (20);
+    }
+}
diff --git a/gdb/testsuite/gdb.multi/multi-arch.exp b/gdb/testsuite/gdb.multi/multi-arch.exp
index d73b4f7..733afa0 100644
--- a/gdb/testsuite/gdb.multi/multi-arch.exp
+++ b/gdb/testsuite/gdb.multi/multi-arch.exp
@@ -96,3 +96,27 @@ if ![runto_main] then {
 
 gdb_test "info inferiors" \
     "Executable.*${exec1}.*${exec2}.*"
+
+# Now select inferior 2, and trigger an event in inferior 1.  This
+# tries to check that GDB doesn't incorrectly uses the architecture of
+# inferior 2 when parsing the expedited registers in a stop reply for
+# inferior 1 (when remote debugging).
+
+gdb_test_no_output "set schedule-multiple on"
+
+with_test_prefix "inf1 event with inf2 selected" {
+    gdb_test "inferior 2" "Switching to inferior 2.*thread 2\.1.*main.*${srcfile2}.*"
+    gdb_test "b hello_loop" "Breakpoint .* at .*${srcfile1}.*"
+    gdb_test "c" " hello_loop.*" "continue to hello_loop"
+}
+
+delete_breakpoints
+
+# Same, but the other way around: select inferior 1 and trigger an
+# event in inferior 2.
+
+with_test_prefix "inf2 event with inf1 selected" {
+    gdb_test "inferior 1" "Switching to inferior 1.*thread 1\.1.*hello_loop.*${srcfile1}.*"
+    gdb_test "b hangout_loop" "Breakpoint .* at .*${srcfile2}.*"
+    gdb_test "c" " hangout_loop.*" "continue to hangout_loop"
+}
-- 
2.5.5