Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: drow@false.org, gdb-patches@sourceware.org
Subject: [rfa] gdbserver multiple register set issue: PBUFSIZ
Date: Thu, 08 May 2008 19:52:00 -0000	[thread overview]
Message-ID: <200805081828.m48ISqlb004187@d12av02.megacenter.de.ibm.com> (raw)

Hello,

here is another problem with supporting multiple register sets: PBUFSIZ.
The value of PBUFSIZ is currently defined depending on registers_length ().
However, this value is communicated only once to GDB when it queries
qSupported.  If after this initial query a different register set is
selected (e.g. in --multi mode), it may happen that GDB and gdbserver
disagree on the maximum allowed packet length, which can lead to failing
communication.

I guess one way to fix this would be to have GDB issue another qSupported
query after it re-attaches.  On the other hand, I'm not sure why this
parameter is even limited by the register set size: when transferring
memory or files (or other data like XML feature descriptions), allowing
larger packet sizes would be helpful even on platforms where the register
set is small.

As GDB limits the packet size to 16 KB anyway, I'd propose to simply
always use 16 KB as PBUFSIZ as well.  The patch below implements this
(together with a sanity check that register packets still fit).

Tested on powerpc-linux and powerpc64-linux in local gdbserver mode.

What do you think?   OK for mainline?

Bye,
Ulrich


ChangeLog:

	* regcache.c (registers_length): Remove.
	(set_register_cache): Verify that PBUFSIZ is large enough to hold a
	full register packet.
	* regcache.h (registers_length): Remove prototype.
	* server.h (PBUFSIZ): Define to 16384.

diff -urNp gdb-orig/gdb/gdbserver/regcache.c gdb-head/gdb/gdbserver/regcache.c
--- gdb-orig/gdb/gdbserver/regcache.c	2008-05-08 19:23:10.401848573 +0200
+++ gdb-head/gdb/gdbserver/regcache.c	2008-05-08 19:00:50.343952878 +0200
@@ -86,12 +86,6 @@ regcache_invalidate ()
   for_each_inferior (&all_threads, regcache_invalidate_one);
 }
 
-int
-registers_length (void)
-{
-  return 2 * register_bytes;
-}
-
 void *
 new_register_cache (void)
 {
@@ -147,6 +141,10 @@ set_register_cache (struct reg *regs, in
 
   register_bytes = offset / 8;
 
+  /* Make sure PBUFSIZ is large enough to hold a full register packet.  */
+  if (2 * register_bytes + 32 > PBUFSIZ)
+    fatal ("Register packet size exceeds PBUFSIZ.");
+
   /* Re-allocate all pre-existing register caches.  */
   for_each_inferior (&all_threads, realloc_register_cache);
 }
diff -urNp gdb-orig/gdb/gdbserver/regcache.h gdb-head/gdb/gdbserver/regcache.h
--- gdb-orig/gdb/gdbserver/regcache.h	2008-05-08 19:23:10.405847998 +0200
+++ gdb-head/gdb/gdbserver/regcache.h	2008-05-08 19:00:06.529604976 +0200
@@ -43,10 +43,6 @@ void registers_to_string (char *buf);
 
 void registers_from_string (char *buf);
 
-/* Return the size in bytes of a string-encoded register packet.  */
-
-int registers_length (void);
-
 /* Return a pointer to the description of register ``n''.  */
 
 struct reg *find_register_by_number (int n);
diff -urNp gdb-orig/gdb/gdbserver/server.h gdb-head/gdb/gdbserver/server.h
--- gdb-orig/gdb/gdbserver/server.h	2008-05-08 19:23:10.410847280 +0200
+++ gdb-head/gdb/gdbserver/server.h	2008-05-08 18:57:08.091278849 +0200
@@ -225,11 +225,10 @@ void warning (const char *string,...) AT
    is chosen to fill up a packet (the headers account for the 32).  */
 #define MAXBUFBYTES(N) (((N)-32)/2)
 
-/* Buffer sizes for transferring memory, registers, etc.  Round up PBUFSIZ to
-   hold all the registers, at least.  */
-#define	PBUFSIZ ((registers_length () + 32 > 2000) \
-		 ? (registers_length () + 32) \
-		 : 2000)
+/* Buffer sizes for transferring memory, registers, etc.   Set to a constant
+   value to accomodate multiple register formats.  This value must be at least
+   as large as the largest register set supported by gdbserver.  */
+#define PBUFSIZ 16384
 
 /* Version information, from version.c.  */
 extern const char version[];
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


             reply	other threads:[~2008-05-08 18:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-08 19:52 Ulrich Weigand [this message]
2008-05-08 20:33 ` Daniel Jacobowitz
2008-05-09  1:23   ` Ulrich Weigand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200805081828.m48ISqlb004187@d12av02.megacenter.de.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox