From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14402 invoked by alias); 27 Mar 2008 19:54:45 -0000 Received: (qmail 14394 invoked by uid 22791); 27 Mar 2008 19:54:45 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate8.de.ibm.com (HELO mtagate8.de.ibm.com) (195.212.29.157) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 27 Mar 2008 19:54:19 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.13.8/8.13.8) with ESMTP id m2RJsGZl070562 for ; Thu, 27 Mar 2008 19:54:16 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 m2RJsGrr3932336 for ; Thu, 27 Mar 2008 20:54:16 +0100 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 m2RJsG85029453 for ; Thu, 27 Mar 2008 20:54:16 +0100 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 m2RJsGg4029450; Thu, 27 Mar 2008 20:54:16 +0100 Message-Id: <200803271954.m2RJsGg4029450@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 27 Mar 2008 20:54:16 +0100 Subject: Re: [RFC] Add support for PPC Altivec registers in gcore To: cseo@linux.vnet.ibm.com (Carlos Eduardo Seo) Date: Thu, 27 Mar 2008 19:54:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org (GDB Patches Mailing List) In-Reply-To: <47EAFD9C.9090708@linux.vnet.ibm.com> from "Carlos Eduardo Seo" at Mar 26, 2008 10:51:24 PM 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-03/txt/msg00454.txt.bz2 Carlos Eduardo Seo wrote: > >> +v:struct core_regset_section *:core_regset_sections:const char *name, int len::::default_regset_sections::(char *) '0' > > > > Not '0' -- just 0. (That *is* a difference!) > > > > Yes, I know. But gcc 4.x will complain about reading through a null > pointer if I use 0. As Andreas pointed out, (char *) '0' is definitely broken -- this will interpret the numerical value of '0' (i.e. 48 in ASCII) as pointer value and try to dereference it, which will certainly crash. I had suggested to use a null pointer as this will be caught at run-time and printed as "(null)". However, re-thinking it, this is not actually guaranteed by the standard -- which is probably why you're seeing a warning ... Unfortunately, I've noticed another, more serious problem: > Index: src/gdb/arch-utils.c > =================================================================== > --- src.orig/gdb/arch-utils.c > +++ src/gdb/arch-utils.c > @@ -31,11 +31,19 @@ > #include "gdbcore.h" > #include "osabi.h" > #include "target-descriptions.h" > - > +#include > +#include "gregset.h" > #include "version.h" > +#include "regset.h" > > #include "floatformat.h" > > +struct core_regset_section default_regset_sections[] = > +{ > + { ".reg", sizeof (gdb_gregset_t) }, > + { ".reg2", sizeof (gdb_fpregset_t) }, > + { NULL, 0 } > +}; This isn't really feasible: you cannot simply include the platform-specific into a generic file -- it may not even be available on some platforms; and on other platforms it may not provide the required gregset_t type. I'm sorry for all the back-and-forth on this, but it seems we do have to implement a Linux-specific fallback to that; I don't see how we can do a generic default. To do so, you'll need to let the gdbarch variable default to NULL, and in linux_nat_do_thread_registers perform the fpregset fallback if that is the case. ppc and i386 would then be the only architectures that install the new gdbarch variable; over time, the remaining Linux platforms should follow suite and we can remove the fallbacks. > Index: src/gdb/ppc-linux-tdep.c > =================================================================== > --- src.orig/gdb/ppc-linux-tdep.c > +++ src/gdb/ppc-linux-tdep.c > @@ -37,6 +37,16 @@ > #include "trad-frame.h" > #include "frame-unwind.h" > #include "tramp-frame.h" > +#include You cannot include a platform-specific header into a -tdep file either, this file is also used for cross-debugging from arbitrary hosts. > +#include "gregset.h" > + > +static struct core_regset_section ppc_regset_sections[] = > +{ > + { ".reg", sizeof (gdb_gregset_t) }, > + { ".reg2", sizeof (gdb_fpregset_t) }, However, you can simply use all numerical values here -- the sizes of the register sections in PowerPC core files are constants. > + { ".reg-ppc-vmx", 544 }, > + { NULL, 0 } > +}; The same applies to the i386 changes: > +++ src/gdb/i386-linux-tdep.c > @@ -28,12 +28,24 @@ > #include "reggroups.h" > #include "dwarf2-frame.h" > #include "gdb_string.h" > - > +#include > +#include "gregset.h" > #include "i386-tdep.h" > #include "i386-linux-tdep.h" > #include "glibc-tdep.h" > #include "solib-svr4.h" > #include "symtab.h" > +#include "regset.h" > + > +static struct core_regset_section i386_regset_sections[] = > +{ > + { ".reg", sizeof (gdb_gregset_t) }, > + { ".reg2", sizeof (gdb_fpregset_t) }, > +#ifdef FILL_FPXREGSET > + { ".reg-xfp", sizeof (gdb_fpxregset_t) }, > +#endif > + { NULL, 0 } > +}; One more issue in linux-nat.c: > + if (core_regset_p) > + while ((++sect_list)->sect_name != NULL) The ++sect_list still skips the first entry unconditionally. You should increment sect_list at the *end* of the loop. Once again, sorry for leading you down a wrong track here. Thanks for your continued work to fix this problem ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com