From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12096 invoked by alias); 28 Feb 2010 20:30:52 -0000 Received: (qmail 12072 invoked by uid 22791); 28 Feb 2010 20:30:51 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 28 Feb 2010 20:30:47 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id o1SKUgjA009779; Sun, 28 Feb 2010 21:30:42 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id o1SKUe6V032104; Sun, 28 Feb 2010 21:30:40 +0100 (CET) Date: Sun, 28 Feb 2010 20:30:00 -0000 Message-Id: <201002282030.o1SKUe6V032104@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: hjl.tools@gmail.com CC: gdb-patches@sourceware.org In-reply-to: <6dc9ffc81002220617o1d348e68hc918d434118cadcb@mail.gmail.com> (hjl.tools@gmail.com) Subject: Re: PATCH: Enable x86 XML target descriptions References: <20100210200303.GA19632@lucon.org> <20100218054312.GA9022@lucon.org> <20100218153402.GA27929@lucon.org> <20100218230135.GA17916@intel.com> <201002221342.o1MDgSZA029705@glazunov.sibelius.xs4all.nl> <6dc9ffc81002220617o1d348e68hc918d434118cadcb@mail.gmail.com> 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: 2010-02/txt/msg00704.txt.bz2 > Date: Mon, 22 Feb 2010 06:17:29 -0800 > From: "H.J. Lu" > > >> +/* 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 > > > > and > > 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 > > Exactly. So why is it ok for 64bit-sse.xml to have ?