From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21532 invoked by alias); 5 Sep 2009 15:38:15 -0000 Received: (qmail 21518 invoked by uid 22791); 5 Sep 2009 15:38:13 -0000 X-SWARE-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_24,J_CHICKENPOX_25,J_CHICKENPOX_28,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-pz0-f185.google.com (HELO mail-pz0-f185.google.com) (209.85.222.185) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 05 Sep 2009 15:38:06 +0000 Received: by pzk15 with SMTP id 15so1593781pzk.24 for ; Sat, 05 Sep 2009 08:38:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.152.26 with SMTP id z26mr404453wfd.201.1252165085077; Sat, 05 Sep 2009 08:38:05 -0700 (PDT) In-Reply-To: <200909050814.n858EvHR016392@brahms.sibelius.xs4all.nl> References: <4A999BC3.5020606@vmware.com> <4AA1CFD1.4000502@vmware.com> <200909050814.n858EvHR016392@brahms.sibelius.xs4all.nl> From: Hui Zhu Date: Sat, 05 Sep 2009 15:38:00 -0000 Message-ID: Subject: Re: [RFA/prec] Make i386 handle segment register better To: Mark Kettenis Cc: msnyder@vmware.com, gdb-patches@sourceware.org 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: 2009-09/txt/msg00118.txt.bz2 On Sat, Sep 5, 2009 at 16:14, Mark Kettenis wrote: >> 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 wrot= e: >> >> >> And this one is also an if/else. =A0So 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. =A0Keep 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. =A0Please help me review it. >> >> I think I like this version. >> Want to check it in? Thanks for you help, Michael. > > The code is basically ok, but I'd like to ask Hui to avoid using > meaningless variable names like "tmp". Thanks for remind me, Mark. I checked in this patch with change the "tmp" to "orv". Hui > >> > 2009-08-30 =A0Hui Zhu =A0 >> > >> > =A0 =A0 =A0 =A0 * i386-tdep.c (i386_record_s): Add orig_addr. >> > =A0 =A0 =A0 =A0 (i386_record_check_override): New function. >> > =A0 =A0 =A0 =A0 (i386_record_lea_modrm): Call i386_record_check_overri= de. >> > =A0 =A0 =A0 =A0 (i386_process_record): Ditto. >> > >> > --- >> > =A0i386-tdep.c | =A0103 ++++++++++++++++++++++++++++++++++------------= -------------- >> > =A01 file changed, 59 insertions(+), 44 deletions(-) >> > >> > --- a/i386-tdep.c >> > +++ b/i386-tdep.c >> > @@ -2867,6 +2867,7 @@ struct i386_record_s >> > =A0{ >> > =A0 =A0struct gdbarch *gdbarch; >> > =A0 =A0struct regcache *regcache; >> > + =A0CORE_ADDR orig_addr; >> > =A0 =A0CORE_ADDR addr; >> > =A0 =A0int aflag; >> > =A0 =A0int dflag; >> > @@ -3147,6 +3148,26 @@ no_rm: >> > =A0 =A0return 0; >> > =A0} >> > >> > +static int >> > +i386_record_check_override (struct i386_record_s *irp) >> > +{ >> > + =A0if (irp->override >=3D 0 && irp->override !=3D X86_RECORD_DS_REGN= UM) >> > + =A0 =A0{ >> > + =A0 =A0 =A0ULONGEST tmp, ds; >> > + >> > + =A0 =A0 =A0regcache_raw_read_unsigned (irp->regcache, >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0i= rp->regmap[irp->override], >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&= tmp); >> > + =A0 =A0 =A0regcache_raw_read_unsigned (irp->regcache, >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0i= rp->regmap[X86_RECORD_DS_REGNUM], >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&= ds); >> > + =A0 =A0 =A0if (tmp !=3D ds) >> > + =A0 =A0 =A0 =A0return 1; >> > + =A0 =A0} >> > + >> > + =A0return 0; >> > +} >> > + >> > =A0/* Record the value of the memory that willbe changed in current in= struction >> > =A0 =A0 to "record_arch_list". >> > =A0 =A0 Return -1 if something wrong. */ >> > @@ -3157,13 +3178,12 @@ i386_record_lea_modrm (struct i386_recor >> > =A0 =A0struct gdbarch *gdbarch =3D irp->gdbarch; >> > =A0 =A0uint64_t addr; >> > >> > - =A0if (irp->override >=3D 0) >> > + =A0if (i386_record_check_override (irp)) >> > =A0 =A0 =A0{ >> > - =A0 =A0 =A0if (record_debug) >> > - =A0 =A0 =A0 printf_unfiltered (_("Process record ignores the memory = change " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"of instructi= on at address %s because it " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"can't get th= e value of the segment register.\n"), >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (gdbarch= , irp->addr)); >> > + =A0 =A0 =A0warning (_("Process record ignores the memory change " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "of instruction at address %s becaus= e it " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "can't get the value of the segment = register."), >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 paddress (gdbarch, irp->orig_addr)); >> > =A0 =A0 =A0 =A0return 0; >> > =A0 =A0 =A0} >> > >> > @@ -3221,6 +3241,7 @@ i386_process_record (struct gdbarch *gdb >> > =A0 =A0memset (&ir, 0, sizeof (struct i386_record_s)); >> > =A0 =A0ir.regcache =3D regcache; >> > =A0 =A0ir.addr =3D addr; >> > + =A0ir.orig_addr =3D addr; >> > =A0 =A0ir.aflag =3D 1; >> > =A0 =A0ir.dflag =3D 1; >> > =A0 =A0ir.override =3D -1; >> > @@ -4039,14 +4060,13 @@ reswitch: >> > =A0 =A0 =A0 =A0/* mov EAX */ >> > =A0 =A0 =A0case 0xa2: >> > =A0 =A0 =A0case 0xa3: >> > - =A0 =A0 =A0if (ir.override >=3D 0) >> > + =A0 =A0 =A0if (i386_record_check_override (&ir)) >> > =A0 =A0 =A0 =A0 =A0{ >> > - =A0 =A0 =A0 =A0 if (record_debug) >> > - =A0 =A0 =A0 =A0 =A0 printf_unfiltered (_("Process record ignores the= memory change " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"of i= nstruction at address 0x%s because " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"it c= an't get the value of the segment " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"regi= ster.\n"), >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress = (gdbarch, ir.addr)); >> > + =A0 =A0 =A0 =A0 warning (_("Process record ignores the memory change= " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "of instruction at address 0= x%s because " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "it can't get the value of t= he segment " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "register."), >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 paddress (gdbarch, ir.orig_addr)= ); >> > =A0 =A0 =A0 =A0 } >> > =A0 =A0 =A0 =A0else >> > =A0 =A0 =A0 =A0 { >> > @@ -4458,27 +4478,24 @@ reswitch: >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0ir.regmap[X86_RECORD_REDI_REGNUM], >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0&tmpulongest); >> > >> > - =A0 =A0 =A0 =A0 =A0regcache_raw_read_unsigned (ir.regcache, >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0ir.regmap[X86_RECORD_ES_REGNUM], >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0&es); >> > - =A0 =A0 =A0 =A0 =A0regcache_raw_read_unsigned (ir.regcache, >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0ir.regmap[X86_RECORD_DS_REGNUM], >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0&ds); >> > - =A0 =A0 =A0 =A0 =A0if (ir.aflag && (es !=3D ds)) >> > + =A0 =A0 =A0 =A0 =A0ir.override =3D X86_RECORD_ES_REGNUM; >> > + =A0 =A0 =A0 =A0 =A0if (ir.aflag && i386_record_check_override (&ir)) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0{ >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* addr +=3D ((uint32_t) read_register = (I386_ES_REGNUM)) << 4; */ >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0if (record_debug) >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf_unfiltered (_("Process record = ignores the memory " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0"change of instruction at address 0x%s " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0"because it can't get the value of the " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0"ES segment register.\n"), >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = paddress (gdbarch, ir.addr)); >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0warning (_("Process record ignores the me= mory " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "change of instructi= on at address 0x%s " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "because it can't ge= t the value of the " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "ES segment register= ."), >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 paddress (gdbarch, ir.or= ig_addr)); >> > + =A0 =A0 =A0 =A0 =A0 =A0} >> > + =A0 =A0 =A0 =A0 =A0else >> > + =A0 =A0 =A0 =A0 =A0 =A0{ >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0if (record_arch_list_add_mem (tmpulongest= , 1 << ir.ot)) >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -1; >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> > >> > =A0 =A0 =A0 =A0 =A0 =A0if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_R= ECX_REGNUM); >> > - =A0 =A0 =A0 =A0 =A0if (record_arch_list_add_mem (tmpulongest, 1 << i= r.ot)) >> > - =A0 =A0 =A0 =A0 =A0 =A0return -1; >> > =A0 =A0 =A0 =A0 =A0 =A0if (opcode =3D=3D 0xa4 || opcode =3D=3D 0xa5) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_R= ESI_REGNUM); >> > =A0 =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_= REGNUM); >> > @@ -5086,15 +5103,14 @@ reswitch: >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 opcode =3D opcode << 8 | ir.modrm; >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto no_support; >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> > - =A0 =A0 =A0 =A0 =A0 if (ir.override >=3D 0) >> > + =A0 =A0 =A0 =A0 =A0 if (i386_record_check_override (&ir)) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (record_debug) >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf_unfiltered (_("Process record= ignores the memory " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0"change of instruction at " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0"address %s because it can't get " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0"the value of the segment " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0"register.\n"), >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0paddress (gdbarch, ir.addr)); >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warning (_("Process record ignores the m= emory " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "change of instr= uction at " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "address %s beca= use it can't get " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "the value of th= e segment " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "register."), >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 paddress (gdbarch, i= r.orig_addr)); >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> > =A0 =A0 =A0 =A0 =A0 =A0 else >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> > @@ -5138,15 +5154,14 @@ reswitch: >> > =A0 =A0 =A0 =A0 =A0 else >> > =A0 =A0 =A0 =A0 =A0 =A0 { >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* sidt */ >> > - =A0 =A0 =A0 =A0 =A0 =A0 if (ir.override >=3D 0) >> > + =A0 =A0 =A0 =A0 =A0 =A0 if (i386_record_check_override (&ir)) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (record_debug) >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf_unfiltered (_("Process re= cord ignores the memory " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0"change of instruction at " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0"address %s because it can't get " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0"the value of the segment " >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0"register.\n"), >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0paddress (gdbarch, ir.addr)); >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 warning (_("Process record ignores t= he memory " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "change of i= nstruction at " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "address %s = because it can't get " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "the value o= f the segment " >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "register."), >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 paddress (gdbarc= h, ir.orig_addr)); >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 else >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> >> >