From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30736 invoked by alias); 23 Aug 2009 03:31:25 -0000 Received: (qmail 30726 invoked by uid 22791); 23 Aug 2009 03:31:24 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_22,J_CHICKENPOX_25,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-pz0-f198.google.com (HELO mail-pz0-f198.google.com) (209.85.222.198) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 23 Aug 2009 03:31:17 +0000 Received: by pzk36 with SMTP id 36so467604pzk.12 for ; Sat, 22 Aug 2009 20:31:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.209.8 with SMTP id h8mr153734wfg.90.1250998276064; Sat, 22 Aug 2009 20:31:16 -0700 (PDT) In-Reply-To: <4A90B261.2030602@vmware.com> References: <4A7BA1DE.6010103@vmware.com> <4A8097B4.2080709@vmware.com> <4A8A2ACD.9000208@vmware.com> <4A90B261.2030602@vmware.com> From: Hui Zhu Date: Sun, 23 Aug 2009 03:33:00 -0000 Message-ID: Subject: Re: Bug in i386_process_record? To: Michael Snyder Cc: gdb-patches ml 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-08/txt/msg00359.txt.bz2 On Sun, Aug 23, 2009 at 11:07, Michael Snyder wrote: > Hi, please *don't* check this in -- I found a problem with it. > > Try running it with "set debug record 1" during the recording pass. > > I see a whole lot of these: > > Process record ignores the memory change of instruction at address > 0x0x587be9 because it can't get the value of the segment register. Hi Michael, Most of the string ops instruction will use segment register. But I check the some linux program that have string ops insn. I found that in linux (maybe glibc), the value of the segment register is 0, so it will not affect anything. And in linux user level, looks we don't have any good way to get the value of the segment register. So I think this patch is OK. Thanks, Hui > > Hui Zhu wrote: >> >> On Tue, Aug 18, 2009 at 17:21, Hui Zhu wrote: >>> >>> On Tue, Aug 18, 2009 at 12:15, Michael Snyder wrote: >>>> >>>> Hui Zhu wrote: >>>>> >>>>> On Tue, Aug 11, 2009 at 05:57, Michael Snyder >>>>> wrote: >>>>>> >>>>>> Yes, this seems to be better. =A0It records only 4 bytes each time >>>>>> it is called. >>>>>> >>>>>> But there seems to be still an off-by-one error? =A0With the test >>>>>> program that I provided, we call memset with an argument of >>>>>> 1024, but we actually record 1025 bytes... this code gets hit >>>>>> 257 times, with the last time recording only 1 byte. >>>>>> >>>>>> >>>>> Hi Michael, >>>>> >>>>> This issue is because: >>>>> >>>>> 0xb7edf4e7 : rep stos %eax,%es:(%edi) >>>>> 0xb7edf4e9 : mov =A0 =A0%edx,%ecx >>>>> 0xb7edf4eb : rep stos %al,%es:(%edi) >>>>> 0xb7edf4ed : mov =A0 =A00x8(%esp),%eax >>>>> 0xb7edf4f1 : pop =A0 =A0%edi >>>>> >>>>> If the memcpy size is not align with 4, it will handle by second rep >>>>> stos. >>>>> Then rep stos will not execute if %ecx is 0. >>>>> i386_process_record doesn't check %ecx, so it get this error. >>>>> >>>>> I make a new patch for it. =A0Please help me review it. >>>> >>>> This seems much better. =A0Please give us a change log and post it for >>>> review. >>>> >>>> By the way, I'm sorry, I only just realized that I posted two >>>> completely different bug reports with the exact same subject line. >>>> ;-( >>>> >>> Don't worry about it. =A0Gmail handle it very well. =A0:) >>> >>> Thanks, >>> Hui >>> >>> 2009-08-18 =A0Hui Zhu =A0 >>> >>> =A0 =A0 =A0 * record.c (i386_process_record): Remove some error code. >>> >> >> Oops, the changelog is not right. =A0I make a new one. >> >> Thanks, >> Hui >> >> 2009-08-21 =A0Hui Zhu =A0 >> >> =A0 =A0 =A0 =A0* i386-tdep.c (i386_process_record): Fix the error of str= ing >> =A0 =A0 =A0 =A0ops instructions's handler. >> >> >>> --- >>> =A0i386-tdep.c | =A0 61 >>> +++++++++++++++++++++++------------------------------------- >>> =A01 file changed, 24 insertions(+), 37 deletions(-) >>> >>> --- a/i386-tdep.c >>> +++ b/i386-tdep.c >>> @@ -4441,50 +4441,37 @@ reswitch: >>> =A0 =A0 =A0/* insS */ >>> =A0 =A0case 0x6c: >>> =A0 =A0case 0x6d: >>> - =A0 =A0 =A0if ((opcode & 1) =3D=3D 0) >>> - =A0 =A0 =A0 ir.ot =3D OT_BYTE; >>> - =A0 =A0 =A0else >>> - =A0 =A0 =A0 ir.ot =3D ir.dflag + OT_WORD; >>> =A0 =A0 =A0regcache_raw_read_unsigned (ir.regcache, >>> - =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 =A0ir= .regmap[X86_RECORD_RECX_REGNUM], >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&tmp= ulongest); >>> - =A0 =A0 =A0if (!ir.aflag) >>> - =A0 =A0 =A0 =A0{ >>> - =A0 =A0 =A0 =A0 =A0tmpulongest &=3D 0xffff; >>> - =A0 =A0 =A0 =A0 =A0/* addr +=3D ((uint32_t) read_register (I386_ES_RE= GNUM)) << 4; >>> */ >>> - =A0 =A0 =A0 =A0 =A0if (record_debug) >>> - =A0 =A0 =A0 =A0 =A0 =A0printf_unfiltered (_("Process record ignores t= he 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 =A0 paddress = (gdbarch, ir.addr)); >>> - =A0 =A0 =A0 =A0} >>> - =A0 =A0 =A0if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) >>> + =A0 =A0 =A0if (tmpulongest) >>> =A0 =A0 =A0 =A0{ >>> - =A0 =A0 =A0 =A0 =A0ULONGEST count, eflags; >>> + =A0 =A0 =A0 =A0 =A0if ((opcode & 1) =3D=3D 0) >>> + =A0 =A0 =A0 =A0 =A0 ir.ot =3D OT_BYTE; >>> + =A0 =A0 =A0 =A0 =A0else >>> + =A0 =A0 =A0 =A0 =A0 ir.ot =3D ir.dflag + OT_WORD; >>> =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_REDI_REGNUM], >>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0&count); >>> - =A0 =A0 =A0 =A0 =A0if (!ir.aflag) >>> - =A0 =A0 =A0 =A0 =A0 =A0count &=3D 0xffff; >>> - =A0 =A0 =A0 =A0 =A0regcache_raw_read_unsigned (ir.regcache, >>> - >>> =A0ir.regmap[X86_RECORD_EFLAGS_REGNUM], >>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0&eflags); >>> - =A0 =A0 =A0 =A0 =A0if ((eflags >> 10) & 0x1) >>> - =A0 =A0 =A0 =A0 =A0 =A0tmpulongest -=3D (count - 1) * (1 << ir.ot); >>> - =A0 =A0 =A0 =A0 =A0if (record_arch_list_add_mem (tmpulongest, count *= (1 << >>> ir.ot))) >>> - =A0 =A0 =A0 =A0 =A0 =A0return -1; >>> - =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REG= NUM); >>> - =A0 =A0 =A0 =A0} >>> - =A0 =A0 =A0else >>> - =A0 =A0 =A0 =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 =A0if (ir.aflag) >>> + =A0 =A0 =A0 =A0 =A0 =A0{ >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0/* addr +=3D ((uint32_t) read_register (I3= 86_ES_REGNUM)) << >>> 4; */ >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0if (record_debug) >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf_unfiltered (_("Process record i= gnores the >>> memory change " >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 "of instruction at address 0x%s >>> because " >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 "it can't get the value of the >>> segment " >>> + =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 p= address (gdbarch, ir.addr)); >>> + =A0 =A0 =A0 =A0 =A0 =A0} >>> + =A0 =A0 =A0 =A0 =A0if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) >>> + =A0 =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX= _REGNUM); >>> =A0 =A0 =A0 =A0 =A0if (record_arch_list_add_mem (tmpulongest, 1 << ir.o= t)) >>> =A0 =A0 =A0 =A0 =A0 =A0return -1; >>> - =A0 =A0 =A0 =A0} >>> - =A0 =A0 =A0if (opcode =3D=3D 0xa4 || opcode =3D=3D 0xa5) >>> - =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM); >>> - =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM); >>> - =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); >>> + =A0 =A0 =A0 =A0 =A0if (opcode =3D=3D 0xa4 || opcode =3D=3D 0xa5) >>> + =A0 =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI= _REGNUM); >>> + =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REG= NUM); >>> + =A0 =A0 =A0 =A0 =A0I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_R= EGNUM); >>> + =A0 =A0 =A0 } >>> =A0 =A0 =A0break; >>> >>> =A0 =A0 =A0/* cmpsS */ >> > >