From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1589 invoked by alias); 5 Sep 2009 02:42:22 -0000 Received: (qmail 1560 invoked by uid 22791); 5 Sep 2009 02:42:20 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_24,J_CHICKENPOX_25,J_CHICKENPOX_28 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 05 Sep 2009 02:42:15 +0000 Received: from mailhost2.vmware.com (mailhost2.vmware.com [10.16.67.167]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id 5685C42001; Fri, 4 Sep 2009 19:42:11 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by mailhost2.vmware.com (Postfix) with ESMTP id 9D0218E7EC; Fri, 4 Sep 2009 19:42:11 -0700 (PDT) Message-ID: <4AA1CFD1.4000502@vmware.com> Date: Sat, 05 Sep 2009 02:42:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches ml Subject: Re: [RFA/prec] Make i386 handle segment register better References: <4A999BC3.5020606@vmware.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: 2009-09/txt/msg00111.txt.bz2 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? Michael > > 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 > {