From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 871 invoked by alias); 8 May 2008 18:29:18 -0000 Received: (qmail 863 invoked by uid 22791); 8 May 2008 18:29:17 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate2.de.ibm.com (HELO mtagate2.de.ibm.com) (195.212.29.151) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 08 May 2008 18:28:56 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate2.de.ibm.com (8.13.8/8.13.8) with ESMTP id m48ISq9s167228 for ; Thu, 8 May 2008 18:28:52 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m48ISq8l3047464 for ; Thu, 8 May 2008 20:28:52 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m48ISqNB004192 for ; Thu, 8 May 2008 20:28:52 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id m48ISqlb004187; Thu, 8 May 2008 20:28:52 +0200 Message-Id: <200805081828.m48ISqlb004187@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 8 May 2008 20:28:52 +0200 Subject: [rfa] gdbserver multiple register set issue: PBUFSIZ To: drow@false.org, gdb-patches@sourceware.org Date: Thu, 08 May 2008 19:52:00 -0000 From: "Ulrich Weigand" X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-05/txt/msg00285.txt.bz2 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