This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Add support for PPC Altivec registers in gcore
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: cseo at linux dot vnet dot ibm dot com (Carlos Eduardo Seo)
- Cc: gdb-patches at sourceware dot org (GDB Patches Mailing List)
- Date: Mon, 18 Feb 2008 19:42:27 +0100 (CET)
- Subject: Re: [RFC] Add support for PPC Altivec registers in gcore
Carlos Eduardo Seo wrote:
> Ok, here's my first shot at this new approach for writing core files. I
> had to add a new BFD function (binutils patch is attached as well - I'll
> submit this to the binutils mailing list later) and I didn't include the
> fill_register fallbacks in the new implementation.
OK.
> I believe a similar loop could also be used in -tdep.c files to read the
> core files as well, right?
Yes, a similar loop should be used to read core files, but this doesn't
go into -tdep.c files, but rather into corelow.c:get_core_registers.
> Only wrote the code for ppc, but this could be easily expanded to other
> archs, adding modifications similar to those I did in ppc-linux-tdep.c.
>
> I think this first code is very raw, but I believe we can improve it and
> use this approach for all archs in the future.
I'd very much prefer if we could just switch over all archs at the same
time -- we already have enough partial transitions in GDB as is :-/
This should in fact be quite easy to achieve, as the only other arch
that supports any non-standard reg section today is i386 with its
.reg-xfp. We'd only need to implement this, and a default fallback
for all other archs that only supports ".reg" and ".reg2".
> fprintf_unfiltered (file,
> + "gdbarch_dump: core_regset_sections = %s\n",
> + (char *) gdbarch->core_regset_sections);
This doesn't work -- core_regset_sections is not a string.
> +v:struct core_regset_section *:core_regset_sections:const char *name, int len::::::(char *) gdbarch->core_regset_sections
You should either use some routine that formats the (host) pointer
properly (similar to gdb_print_host_address) or else maybe just
disable debug output for this field.
> +static struct core_regset_section ppc_regset_sections[] =
> +{
> + {".reg2", 264},
> + {".reg-ppc-vmx", 544},
> + {NULL, 0}
> +};
Formatting: space after { and before }.
I'm wondering if it wouldn't be better to include ".reg" in the
table, and just treat it separately where necessary.
> + if (core_regset_p && sect_list != NULL)
> + while ((sect_list + i)->sect_name != NULL)
> + {
> + if ((regset = gdbarch_regset_from_core_section (gdbarch,
> + (sect_list + i)->sect_name,
> + (*(sect_list + i)).size))
> + != NULL && regset->collect_regset != NULL)
> + {
> + gdb_regset = xmalloc ((*(sect_list + i)).size);
> + regset->collect_regset (regset, regcache, -1,
> + gdb_regset, (*(sect_list + i)).size);
> + note_data = (char *) elfcore_write_register_note (obfd,
> + note_data,
> + note_size,
> + (sect_list + i)->sect_name,
> + gdb_regset,
> + (*(sect_list + i)).size);
> + xfree (gdb_regset);
> + }
> + i++;
> + }
If you'd just use sect_list++, all those *(sect_list + i)
could go away.
> + /* For architectures that does not have the struct core_regset_section implemented,
> + we use the old method. When all the architectures have the new support, the code
> + below should be deprecated. */
As mentioned above, please just convert everything in one go.
> 2008-02-03 Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
>
> * elf.c (elfcore_write_register_note): New function.
> * elf-bfd.h (elfcore_write_register_note): New prototype.
These look fine to me, but need to be posted to the binutils list.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com