From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26877 invoked by alias); 17 Mar 2008 19:07:18 -0000 Received: (qmail 26774 invoked by uid 22791); 17 Mar 2008 19:07:17 -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; Mon, 17 Mar 2008 19:06:40 +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 m2HJ6bD8142780 for ; Mon, 17 Mar 2008 19:06:37 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 m2HJ6bQX1941758 for ; Mon, 17 Mar 2008 20:06:37 +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 m2HJ6Ywf030694 for ; Mon, 17 Mar 2008 20:06:36 +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 m2HJ6YCB030691; Mon, 17 Mar 2008 20:06:34 +0100 Message-Id: <200803171906.m2HJ6YCB030691@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Mon, 17 Mar 2008 20:06:34 +0100 Subject: Re: [RFC] Add support for PPC Altivec registers in gcore To: cseo@linux.vnet.ibm.com (Carlos Eduardo Seo) Date: Mon, 17 Mar 2008 19:07:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org (GDB Patches Mailing List) In-Reply-To: <47D543F9.20201@linux.vnet.ibm.com> from "Carlos Eduardo Seo" at Mar 10, 2008 11:21:45 AM 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/msg00242.txt.bz2 Carlos Eduardo Seo wrote: > +# Supported register notes in core files '.' at the end. > +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!) > @@ -2653,35 +2657,31 @@ linux_nat_do_thread_registers (bfd *obfd > lwp, > stop_signal, &gregs); > > - if (core_regset_p > - && (regset = gdbarch_regset_from_core_section (gdbarch, ".reg2", > - sizeof (fpregs))) != NULL > - && regset->collect_regset != NULL) > - regset->collect_regset (regset, regcache, -1, > - &fpregs, sizeof (fpregs)); > - else > - fill_fpregset (regcache, &fpregs, -1); > - > - note_data = (char *) elfcore_write_prfpreg (obfd, > - note_data, > - note_size, > - &fpregs, sizeof (fpregs)); Ah. I think I asked you to do this, but I overlooked something here: there are quite a number of Linux targets that either do not define gdbarch_regset_from_core_section or do not implement a collect_regset function. We don't want to break those, so we'll probably have to keep the fall-back to fill_fpregset for now. > -#ifdef FILL_FPXREGSET > - if (core_regset_p > - && (regset = gdbarch_regset_from_core_section (gdbarch, ".reg-xfp", > - sizeof (fpxregs))) != NULL > - && regset->collect_regset != NULL) > - regset->collect_regset (regset, regcache, -1, > - &fpxregs, sizeof (fpxregs)); > - else > - fill_fpxregset (regcache, &fpxregs, -1); > - > - note_data = (char *) elfcore_write_prxfpreg (obfd, > - note_data, > - note_size, > - &fpxregs, sizeof (fpxregs)); > -#endif But *this* definitely can go away, as the i386 target does use gdbarch_regset_from_core_section with collect_regset. > + /* The loop below uses the new struct core_regset_section, which stores > + the supported section names and sizes for the core file. Note that > + note PRSTATUS needs to be treated specially. But the other notes are > + structurally the same, so they can benefit from the new struct. */ > + > + if (core_regset_p && sect_list != NULL) sect_list cannot really be NULL as it has a default. > + while ((++sect_list)->sect_name != NULL) This implicitly assumes that ".reg" must be the very first element. I'd rather see an explicit check of the type /* ".reg" was already handled above. */ if (strcmp (sect_list->sect_name, ".reg") == 0) continue; (And similarly for ".reg2" if we keep the special case above.) > + if ((regset = gdbarch_regset_from_core_section (gdbarch, > + sect_list->sect_name, > + (*sect_list).size)) I'd rather use sect_list->size (here and elsewhere). > +#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" These need Makefile.in dependency list changes. > +#include > +#include "gregset.h" > #include "version.h" > +#include "regset.h" Likewise. > Index: src/gdb/arch-utils.h > +#include "regset.h" > struct gdbarch; > struct frame_info; > struct minimal_symbol; > struct type; > struct gdbarch_info; > > +extern struct core_regset_section default_regset_sections[]; I'd prefer *not* to include a new header file here; a forward declaration of the struct should be enough: struct core_regset_section; Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com