This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: PATCH: Enable x86 XML target descriptions
> Date: Mon, 22 Feb 2010 06:17:29 -0800
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> >> +/* Get Linux/x86 target description from core dump. ?*/
> >> +
> >> +static const struct target_desc *
> >> +amd64_linux_core_read_description (struct gdbarch *gdbarch,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct target_ops *target,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bfd *abfd)
> >> +{
> >> + ?asection *section = bfd_get_section_by_name (abfd, ".reg2");
> >> +
> >> + ?if (section == NULL)
> >> + ? ?return NULL;
> >> +
> >> + ?switch (bfd_section_size (abfd, section))
> >> + ? ?{
> >> + ? ?case 0x200:
> >> + ? ? ?/* Linux/x86-64. ?*/
> >> + ? ? ?return tdesc_amd64_linux;
> >> + ? ?default:
> >> + ? ? ?return NULL;
> >> + ? ?}
> >> +}
> >
> > This seems a bit odd to me. ?I'd expect this function to just return
> > tdesc_amd64_linux unconditionally.
>
> The folowup patch for AVX will change it to
>
> xcr0 = i386_linux_core_read_xcr0 (gdbarch, target, abfd);
> switch (bfd_section_size (abfd, section))
> {
> case 0x200:
> /* Linux/x86-64. */
> if ((xcr0 & XSTATE_AVX_MASK) == XSTATE_AVX_MASK)
> return tdesc_amd64_avx_linux;
> else
> return tdesc_amd64_linux;
> default:
> return NULL;
> }
>
> Other OSes will need a similar change to support AVX.
Even with that, checking the size of the .reg2 section makes no sense
to me.
> >> @@ -1282,10 +1292,31 @@ amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >>
> >> ? ?/* Add the %orig_rax register used for syscall restarting. ?*/
> >> ? ?set_gdbarch_write_pc (gdbarch, amd64_linux_write_pc);
> >> +
> >> + ?/* Reserve a number for orig_rax. ?*/
> >> ? ?set_gdbarch_num_regs (gdbarch, AMD64_LINUX_NUM_REGS);
> >> - ?set_gdbarch_register_name (gdbarch, amd64_linux_register_name);
> >> - ?set_gdbarch_register_type (gdbarch, amd64_linux_register_type);
> >> - ?set_gdbarch_register_reggroup_p (gdbarch, amd64_linux_register_reggroup_p);
> >
> > Why do you need to set num_regs here, but not the register_name,
> > register_type and register_reggroup_p members? ?All four are set by
> > tdesc_use_registers().
>
> tdesc_use_registers is called after amd64_linux_init_abi. At this
> point, num_regs is incorrect for Linux. We need to update it for
>
> valid_p = tdesc_numbered_register (feature, tdesc_data,
> AMD64_LINUX_ORIG_RAX_REGNUM,
> "orig_rax");
Sorry, but I don't understand this. How does checking a register in
the target description care about the number of registers set in the
gdbarch we're building?
> >> +
> >> + ?if (! tdesc_has_registers (tdesc))
> >> + ? ?tdesc = tdesc_amd64_linux;
> >> + ?tdep->tdesc = tdesc;
> >> +
> >> + ?feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
> >
> > Shouldn't that be org.gnu.gdb.amd64.linux?
>
> 64bit-core.xml and 64bit-sse.xml have
>
> <feature name="org.gnu.gdb.i386.core">
>
> and
>
> <feature name="org.gnu.gdb.i386.sse">
Which seems wrong to me. Both the core registers and the SSE
registers are different in 64-bit mode. But perhaps Daniel can shed
some light on how these features are supposed to be used?
> so that i386_gdbarch_init can have
>
> /* Get core registers. */
> feature_core = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.core");
>
> /* Get SSE registers. */
> feature_vector = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.sse");
>
> after
>
> /* Hook in ABI-specific overrides, if they have been registered. */
> info.tdep_info = (void *) tdesc_data;
> gdbarch_init_osabi (info, gdbarch);
>
> It will be odd for 64bit-linux.xml to have
>
> <feature name="org.gnu.gdb.i386.linux">
Exactly. So why is it ok for 64bit-sse.xml to have
<fearure name="org.gnu.gdb.i386.sse">
?