From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11826 invoked by alias); 17 Feb 2010 15:42:02 -0000 Received: (qmail 11568 invoked by uid 22791); 17 Feb 2010 15:41:57 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-fx0-f209.google.com (HELO mail-fx0-f209.google.com) (209.85.220.209) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 17 Feb 2010 15:41:52 +0000 Received: by fxm1 with SMTP id 1so7922482fxm.4 for ; Wed, 17 Feb 2010 07:41:50 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.186.138 with SMTP id w10mr2624236wem.206.1266421309779; Wed, 17 Feb 2010 07:41:49 -0800 (PST) In-Reply-To: <201002171522.o1HFMpBN006203@glazunov.sibelius.xs4all.nl> References: <20100210200303.GA19632@lucon.org> <20100217145820.GA20676@lucon.org> <201002171522.o1HFMpBN006203@glazunov.sibelius.xs4all.nl> Date: Wed, 17 Feb 2010 15:42:00 -0000 Message-ID: <6dc9ffc81002170741h3611c369mb3efd2b5bcdb8a34@mail.gmail.com> Subject: Re: PATCH: Enable x86 XML target descriptions From: "H.J. Lu" To: Mark Kettenis Cc: gdb-patches@sourceware.org, Daniel Jacobowitz Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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/msg00424.txt.bz2 On Wed, Feb 17, 2010 at 7:22 AM, Mark Kettenis wr= ote: >> Date: Wed, 17 Feb 2010 06:58:20 -0800 >> From: "H.J. Lu" >> >> On Wed, Feb 10, 2010 at 12:03:03PM -0800, H.J. Lu wrote: >> > Hi, >> > >> > This patch enables x86 XML target descriptions. =A0I used >> > i386_linux_init_orig_eax to support the old gdbserver which doesn't >> > have XML target descriptions. >> > >> > The register description processing is handled in i386_gdbarch_init >> > for 32bit/64bit as well as all ABIs to avoid code duplication and >> > unnecessary complexity. >> > >> > OK to install? >> > >> > BTW, I have a followup patch to support al, ax and eax pseudo register= s. >> > >> > Thanks. >> > >> > >> > H.J. >> > ---- >> > gdb/ >> > >> > 2010-02-10 =A0H.J. Lu =A0 >> > >> > =A0 =A0 * amd64-linux-nat.c (_initialize_amd64_linux_nat): Set >> > =A0 =A0 to_read_description to i386_linux_read_description. >> > =A0 =A0 * i386-linux-nat.c (_initialize_i386_linux_nat): Likewise. >> > >> > =A0 =A0 * amd64-linux-tdep.c (amd64_linux_register_name): Removed. >> > =A0 =A0 (amd64_linux_register_type): Likewise. >> > =A0 =A0 (amd64_linux_init_abi): Don't call set_gdbarch_num_regs, >> > =A0 =A0 set_gdbarch_register_name nor set_gdbarch_register_type. =A0Ca= ll >> > =A0 =A0 i386_linux_init_orig_eax and set_gdbarch_core_read_description. >> > >> > =A0 =A0 * amd64-linux-tdep.h: Include "i386-linux-tdep.h". >> > >> > =A0 =A0 * amd64-tdep.c (amd64_register_names): Make it global. >> > =A0 =A0 (amd64_register_name): Removed. >> > =A0 =A0 (amd64_register_type): Likewise. >> > =A0 =A0 (amd64_init_abi): Don't call set_gdbarch_num_regs, >> > =A0 =A0 set_gdbarch_register_name nor set_gdbarch_register_type. >> > >> > =A0 =A0 * amd64-tdep.h (amd64_register_names): New. >> > >> > =A0 =A0 * i386-linux-tdep.c: Include "amd64-tdep.h" and >> > =A0 =A0 * "amd64-linux-tdep.h" instead "i386-tdep.h" and >> > =A0 =A0 "i386-linux-tdep.h". =A0Include features/i386/i386-linux.c and >> > =A0 =A0 features/i386/amd64-linux.c. >> > =A0 =A0 (i386_linux_register_name): Support BFD64. >> > =A0 =A0 (i386_linux_register_type): New. >> > =A0 =A0 (i386_linux_init_orig_eax): Likewise. >> > =A0 =A0 (i386_linux_core_read_description): Likewise. >> > =A0 =A0 (i386_linux_read_description): Likewise. >> > =A0 =A0 (i386_linux_init_abi): Don't call set_gdbarch_num_regs nor >> > =A0 =A0 set_gdbarch_register_name. =A0Call i386_linux_init_orig_eax and >> > =A0 =A0 set_gdbarch_core_read_description. >> > =A0 =A0 (_initialize_i386_linux_tdep): Call initialize_tdesc_i386_linux >> > =A0 =A0 and initialize_tdesc_x86_64_linux. >> > >> > =A0 =A0 * i386-linux-tdep.h (i386_linux_core_read_description): New. >> > =A0 =A0 (i386_linux_read_description): Likewise. >> > =A0 =A0 (i386_linux_init_orig_eax): Likewise. >> > >> > =A0 =A0 * i386-tdep.c: Include "amd64-tdep.h" instead of "i386-tdep.h". >> > =A0 =A0 Include features/i386/i386.c and features/i386/amd64.c. >> > =A0 =A0 (i386_register_names): Make it const. >> > =A0 =A0 (i386_mmx_names): Likewise. >> > =A0 =A0 (i386_num_register_names): Removed. >> > =A0 =A0 (i386_register_name): Likewise. >> > =A0 =A0 (i386_eflags_type): Likewise. >> > =A0 =A0 (i386_mxcsr_type): Likewise. >> > =A0 =A0 (i386_sse_type): Likewise. >> > =A0 =A0 (i386_register_type): Likewise. >> > =A0 =A0 (i387_ext_type): Call tdesc_find_type instead of arch_float_ty= pe. >> > =A0 =A0 (i386_pseudo_register_name): New. >> > =A0 =A0 (i386_pseudo_register_type): Likewise. >> > =A0 =A0 (i386_mmx_type): Make it static. >> > =A0 =A0 (i386_gdbarch_init): Support both 32bit and 64bit x86 target >> > =A0 =A0 descriptions. =A0Don't call set_gdbarch_register_name nor >> > =A0 =A0 set_gdbarch_register_type. =A0Call set_tdesc_pseudo_register_t= ype, >> > =A0 =A0 set_tdesc_pseudo_register_name and tdesc_use_registers. >> > =A0 =A0 (_initialize_i386_tdep): Call initialize_tdesc_i386 and >> > =A0 =A0 initialize_tdesc_x86_64. >> > >> > =A0 =A0 * i386-tdep.h (gdbarch_tdep): Remove i386_eflags_type, >> > =A0 =A0 i386_mxcsr_type and i386_sse_type. >> > =A0 =A0 (i386_eflags_type): Removed. >> > =A0 =A0 (i386_mxcsr_type): Likewise. >> > =A0 =A0 (i386_mmx_type): Likewise. >> > =A0 =A0 (i386_sse_type): Likewise. >> > =A0 =A0 (i386_register_name): Likewise. >> > >> > =A0 =A0 * regformats/i386/i386-linux.dat: Generated. >> > =A0 =A0 * regformats/i386/i386.dat: Likewise. >> > =A0 =A0 * regformats/i386/amd64-linux.dat: Likewise. >> > =A0 =A0 * regformats/i386/amd64.dat: Likewise. >> > >> > =A0 =A0 * regformats/reg-i386-linux.dat: Removed. >> > =A0 =A0 * regformats/reg-i386.dat: Likewise. >> > =A0 =A0 * regformats/reg-x86-64-linux.dat: Likewise. >> > =A0 =A0 * regformats/reg-x86-64.dat: Likewise. >> > >> > gdb/gdbserver/ >> > >> > 2010-02-10 =A0H.J. Lu =A0 >> > >> > =A0 =A0 * Makefile.in (clean): Replace reg-i386.c, reg-x86-64.c, >> > =A0 =A0 reg-i386-linux.c and reg-x86-64-linux.c with i386.c, amd64.c, >> > =A0 =A0 i386-linux.c and amd64-linux.c. >> > =A0 =A0 (reg-i386.o): Removed. >> > =A0 =A0 (reg-i386.c): Likewise. >> > =A0 =A0 (reg-i386-linux.o): Likewise. >> > =A0 =A0 (reg-i386-linux.c): Likewise. >> > =A0 =A0 (reg-x86-64.o): Likewise. >> > =A0 =A0 (reg-x86-64.c): Likewise. >> > =A0 =A0 (reg-x86-64-linux.o): Likewise. >> > =A0 =A0 (reg-x86-64-linux.c): Likewise. >> > =A0 =A0 (i386.o): New. >> > =A0 =A0 (i386.c): Likewise. >> > =A0 =A0 (i386-linux.o): Likewise. >> > =A0 =A0 (i386-linux.c): Likewise. >> > =A0 =A0 (amd64.o): Likewise. >> > =A0 =A0 (amd64.c): Likewise. >> > =A0 =A0 (amd64-linux.o): Likewise. >> > =A0 =A0 (amd64-linux.c): Likewise. >> > >> > =A0 =A0 * configure.srv (srv_i386_regobj): New. >> > =A0 =A0 (srv_i386_linux_regobj): Likewise. >> > =A0 =A0 (srv_amd64_regobj): Likewise. >> > =A0 =A0 (srv_amd64_linux_regobj): Likewise. >> > =A0 =A0 (srv_i386_32bit_xmlfiles): Likewise. >> > =A0 =A0 (srv_i386_64bit_xmlfiles): Likewise. >> > =A0 =A0 (srv_i386_xmlfiles): Likewise. >> > =A0 =A0 (srv_amd64_xmlfiles): Likewise. >> > =A0 =A0 (srv_i386_linux_xmlfiles): Likewise. >> > =A0 =A0 (srv_amd64_linux_xmlfiles): Likewise. >> > =A0 =A0 (i[34567]86-*-cygwin*): Set srv_regobj to $srv_i386_regobj. = =A0Set >> > =A0 =A0 srv_xmlfiles to $srv_i386_xmlfiles. >> > =A0 =A0 (i[34567]86-*-mingw32ce*): Likewise. >> > =A0 =A0 (i[34567]86-*-mingw*): Likewise. >> > =A0 =A0 (i[34567]86-*-nto*): Likewise. >> > =A0 =A0 (i[34567]86-*-linux*): Set srv_regobj to $srv_i386_linux_regobj >> > =A0 =A0 and $srv_amd64_linux_regobj. =A0Set srv_xmlfiles to >> > =A0 =A0 $srv_i386_linux_xmlfiles and $srv_amd64_linux_xmlfiles. >> > =A0 =A0 (x86_64-*-linux*): Likewise. >> > >> > =A0 =A0 * linux-x86-low.c (init_registers_x86_64_linux): Removed. >> > =A0 =A0 (init_registers_amd64_linux): New. >> > =A0 =A0 (x86_arch_setup): Replace init_registers_x86_64_linux with >> > =A0 =A0 init_registers_amd64_linux. >> > >> >> Any objections to enable XML target descriptions for x86? Any comments? > > Thanks for the reminder. > > I really dislike the fact that perfectly well understandable C code > gets replaced by a complicated XML-based mechanism that I don't > completely understand. > > So it is perfectly possible that there is some magic that I'm missing > here, but I get the feeling that your diff breaks all native amd64 and > i386 targets except for Linux. =A0I'm afraid that isn't acceptable. Can you try my patch on non-Linux x86 native targets? > Also, you should split i386_linux_register_name(), > i386_linix_register_type() i386_linux_init_orig_eax() into seperate > i386 and amd64 versions, and get rid of all the #ifdef BFD64's in the > *-tdep.c files. =A0There has been a clear split between 32-bit and > 64-bit versions of the *-tdep.c code and this diff blurrs that. I am not sure it is easy to do, especially for register description. I have an impression that tdesc_use_registers should only be called once per arch. i386 and amd64 belong to the same bfd_arch_i386. That is how it is used in other places: arm-tdep.c: tdesc_use_registers (gdbarch, info.target_desc, tdesc_data= ); m68k-tdep.c: tdesc_use_registers (gdbarch, info.target_desc, tdesc_data); mips-tdep.c: tdesc_use_registers (gdbarch, info.target_desc, tdesc_dat= a); rs6000-tdep.c: tdesc_use_registers (gdbarch, tdesc, tdesc_data); s390-tdep.c: tdesc_use_registers (gdbarch, tdesc, tdesc_data); There is only single call to tdesc_use_registers for 32bit/64bit mips/ppc. All other XML described tdep files support both 32bit/64bit: [hjl@gnu-6 gdb]$ ls mips*-tdep.c rs6000*-tdep.c ppc*-tdep.c mips64obsd-tdep.c mipsnbsd-tdep.c ppcnbsd-tdep.c rs6000-aix-tdep.c mips-irix-tdep.c mips-tdep.c ppcobsd-tdep.c rs6000-tdep.c mips-linux-tdep.c ppc-linux-tdep.c ppc-sysv-tdep.c I can try to minimize BFD64. But I am afraid that I can't totally avoid it. Is that acceptable? > I can't comment on the gdbserver bits; that is Daniel's territory. > Daniel, can you comment on gdbserver changes? Thanks. --=20 H.J.