From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31671 invoked by alias); 5 Sep 2009 08:15:10 -0000 Received: (qmail 31611 invoked by uid 22791); 5 Sep 2009 08:15:08 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_24,J_CHICKENPOX_25,J_CHICKENPOX_28 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 05 Sep 2009 08:15:02 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id n858EwE5009295; Sat, 5 Sep 2009 10:14:58 +0200 (CEST) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id n858EvHR016392; Sat, 5 Sep 2009 10:14:57 +0200 (CEST) Date: Sat, 05 Sep 2009 08:15:00 -0000 Message-Id: <200909050814.n858EvHR016392@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: msnyder@vmware.com CC: teawater@gmail.com, gdb-patches@sourceware.org In-reply-to: <4AA1CFD1.4000502@vmware.com> (message from Michael Snyder on Fri, 04 Sep 2009 19:41:21 -0700) Subject: Re: [RFA/prec] Make i386 handle segment register better References: <4A999BC3.5020606@vmware.com> <4AA1CFD1.4000502@vmware.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: 2009-09/txt/msg00117.txt.bz2 > Date: Fri, 04 Sep 2009 19:41:21 -0700 > From: Michael Snyder > > Hui Zhu wrote: > > On Sun, Aug 30, 2009 at 05:21, Michael Snyder wrote: > > >> And this one is also an if/else. So I guess my questions are: > >> > >> 1) Should you use an "else" in the "String ops" case? > > > > OK. > > > >> 2) Should we go ahead and record the register changes, > >> even though we can't record the memory change? > > > > I think even if we cannot record the memory change. Keep record the > > change of reg is better. > > > >> 3) Should this be a warning, rather than just a debug message? > >> I think yes, because if this happens, it actually means that the > >> record log will be inaccurate. > >> > > OK. > > > > > > I make a new patch for it. Please help me review it. > > I think I like this version. > Want to check it in? The code is basically ok, but I'd like to ask Hui to avoid using meaningless variable names like "tmp". > > 2009-08-30 Hui Zhu > > > > * i386-tdep.c (i386_record_s): Add orig_addr. > > (i386_record_check_override): New function. > > (i386_record_lea_modrm): Call i386_record_check_override. > > (i386_process_record): Ditto. > > > > --- > > i386-tdep.c | 103 ++++++++++++++++++++++++++++++++++-------------------------- > > 1 file changed, 59 insertions(+), 44 deletions(-) > > > > --- a/i386-tdep.c > > +++ b/i386-tdep.c > > @@ -2867,6 +2867,7 @@ struct i386_record_s > > { > > struct gdbarch *gdbarch; > > struct regcache *regcache; > > + CORE_ADDR orig_addr; > > CORE_ADDR addr; > > int aflag; > > int dflag; > > @@ -3147,6 +3148,26 @@ no_rm: > > return 0; > > } > > > > +static int > > +i386_record_check_override (struct i386_record_s *irp) > > +{ > > + if (irp->override >= 0 && irp->override != X86_RECORD_DS_REGNUM) > > + { > > + ULONGEST tmp, ds; > > + > > + regcache_raw_read_unsigned (irp->regcache, > > + irp->regmap[irp->override], > > + &tmp); > > + regcache_raw_read_unsigned (irp->regcache, > > + irp->regmap[X86_RECORD_DS_REGNUM], > > + &ds); > > + if (tmp != ds) > > + return 1; > > + } > > + > > + return 0; > > +} > > + > > /* Record the value of the memory that willbe changed in current instruction > > to "record_arch_list". > > Return -1 if something wrong. */ > > @@ -3157,13 +3178,12 @@ i386_record_lea_modrm (struct i386_recor > > struct gdbarch *gdbarch = irp->gdbarch; > > uint64_t addr; > > > > - if (irp->override >= 0) > > + if (i386_record_check_override (irp)) > > { > > - if (record_debug) > > - printf_unfiltered (_("Process record ignores the memory change " > > - "of instruction at address %s because it " > > - "can't get the value of the segment register.\n"), > > - paddress (gdbarch, irp->addr)); > > + warning (_("Process record ignores the memory change " > > + "of instruction at address %s because it " > > + "can't get the value of the segment register."), > > + paddress (gdbarch, irp->orig_addr)); > > return 0; > > } > > > > @@ -3221,6 +3241,7 @@ i386_process_record (struct gdbarch *gdb > > memset (&ir, 0, sizeof (struct i386_record_s)); > > ir.regcache = regcache; > > ir.addr = addr; > > + ir.orig_addr = addr; > > ir.aflag = 1; > > ir.dflag = 1; > > ir.override = -1; > > @@ -4039,14 +4060,13 @@ reswitch: > > /* mov EAX */ > > case 0xa2: > > case 0xa3: > > - if (ir.override >= 0) > > + if (i386_record_check_override (&ir)) > > { > > - if (record_debug) > > - printf_unfiltered (_("Process record ignores the memory change " > > - "of instruction at address 0x%s because " > > - "it can't get the value of the segment " > > - "register.\n"), > > - paddress (gdbarch, ir.addr)); > > + warning (_("Process record ignores the memory change " > > + "of instruction at address 0x%s because " > > + "it can't get the value of the segment " > > + "register."), > > + paddress (gdbarch, ir.orig_addr)); > > } > > else > > { > > @@ -4458,27 +4478,24 @@ reswitch: > > ir.regmap[X86_RECORD_REDI_REGNUM], > > &tmpulongest); > > > > - regcache_raw_read_unsigned (ir.regcache, > > - ir.regmap[X86_RECORD_ES_REGNUM], > > - &es); > > - regcache_raw_read_unsigned (ir.regcache, > > - ir.regmap[X86_RECORD_DS_REGNUM], > > - &ds); > > - if (ir.aflag && (es != ds)) > > + ir.override = X86_RECORD_ES_REGNUM; > > + if (ir.aflag && i386_record_check_override (&ir)) > > { > > /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */ > > - if (record_debug) > > - printf_unfiltered (_("Process record ignores the memory " > > - "change of instruction at address 0x%s " > > - "because it can't get the value of the " > > - "ES segment register.\n"), > > - paddress (gdbarch, ir.addr)); > > + warning (_("Process record ignores the memory " > > + "change of instruction at address 0x%s " > > + "because it can't get the value of the " > > + "ES segment register."), > > + paddress (gdbarch, ir.orig_addr)); > > + } > > + else > > + { > > + if (record_arch_list_add_mem (tmpulongest, 1 << ir.ot)) > > + return -1; > > } > > > > if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) > > I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM); > > - if (record_arch_list_add_mem (tmpulongest, 1 << ir.ot)) > > - return -1; > > if (opcode == 0xa4 || opcode == 0xa5) > > I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM); > > I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM); > > @@ -5086,15 +5103,14 @@ reswitch: > > opcode = opcode << 8 | ir.modrm; > > goto no_support; > > } > > - if (ir.override >= 0) > > + if (i386_record_check_override (&ir)) > > { > > - if (record_debug) > > - printf_unfiltered (_("Process record ignores the memory " > > - "change of instruction at " > > - "address %s because it can't get " > > - "the value of the segment " > > - "register.\n"), > > - paddress (gdbarch, ir.addr)); > > + warning (_("Process record ignores the memory " > > + "change of instruction at " > > + "address %s because it can't get " > > + "the value of the segment " > > + "register."), > > + paddress (gdbarch, ir.orig_addr)); > > } > > else > > { > > @@ -5138,15 +5154,14 @@ reswitch: > > else > > { > > /* sidt */ > > - if (ir.override >= 0) > > + if (i386_record_check_override (&ir)) > > { > > - if (record_debug) > > - printf_unfiltered (_("Process record ignores the memory " > > - "change of instruction at " > > - "address %s because it can't get " > > - "the value of the segment " > > - "register.\n"), > > - paddress (gdbarch, ir.addr)); > > + warning (_("Process record ignores the memory " > > + "change of instruction at " > > + "address %s because it can't get " > > + "the value of the segment " > > + "register."), > > + paddress (gdbarch, ir.orig_addr)); > > } > > else > > { > >