From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25419 invoked by alias); 23 Aug 2009 03:11:33 -0000 Received: (qmail 25411 invoked by uid 22791); 23 Aug 2009 03:11:32 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_22,J_CHICKENPOX_25 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 23 Aug 2009 03:11:26 +0000 Received: from jupiter.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id 25F6C34015; Sat, 22 Aug 2009 20:11:25 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by jupiter.vmware.com (Postfix) with ESMTP id 18E72DC080; Sat, 22 Aug 2009 20:11:25 -0700 (PDT) Message-ID: <4A90B261.2030602@vmware.com> Date: Sun, 23 Aug 2009 03:15: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: Bug in i386_process_record? References: <4A7BA1DE.6010103@vmware.com> <4A8097B4.2080709@vmware.com> <4A8A2ACD.9000208@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-08/txt/msg00357.txt.bz2 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. 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. It records only 4 bytes each time >>>>> it is called. >>>>> >>>>> But there seems to be still an off-by-one error? With 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 %edx,%ecx >>>> 0xb7edf4eb : rep stos %al,%es:(%edi) >>>> 0xb7edf4ed : mov 0x8(%esp),%eax >>>> 0xb7edf4f1 : pop %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. Please help me review it. >>> This seems much better. Please 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. Gmail handle it very well. :) >> >> Thanks, >> Hui >> >> 2009-08-18 Hui Zhu >> >> * record.c (i386_process_record): Remove some error code. >> > > Oops, the changelog is not right. I make a new one. > > Thanks, > Hui > > 2009-08-21 Hui Zhu > > * i386-tdep.c (i386_process_record): Fix the error of string > ops instructions's handler. > > >> --- >> i386-tdep.c | 61 +++++++++++++++++++++++------------------------------------- >> 1 file changed, 24 insertions(+), 37 deletions(-) >> >> --- a/i386-tdep.c >> +++ b/i386-tdep.c >> @@ -4441,50 +4441,37 @@ reswitch: >> /* insS */ >> case 0x6c: >> case 0x6d: >> - if ((opcode & 1) == 0) >> - ir.ot = OT_BYTE; >> - else >> - ir.ot = ir.dflag + OT_WORD; >> regcache_raw_read_unsigned (ir.regcache, >> - ir.regmap[X86_RECORD_REDI_REGNUM], >> + ir.regmap[X86_RECORD_RECX_REGNUM], >> &tmpulongest); >> - if (!ir.aflag) >> - { >> - tmpulongest &= 0xffff; >> - /* 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 segment " >> - "register.\n"), >> - paddress (gdbarch, ir.addr)); >> - } >> - if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) >> + if (tmpulongest) >> { >> - ULONGEST count, eflags; >> + if ((opcode & 1) == 0) >> + ir.ot = OT_BYTE; >> + else >> + ir.ot = ir.dflag + OT_WORD; >> regcache_raw_read_unsigned (ir.regcache, >> ir.regmap[X86_RECORD_REDI_REGNUM], >> - &count); >> - if (!ir.aflag) >> - count &= 0xffff; >> - regcache_raw_read_unsigned (ir.regcache, >> - ir.regmap[X86_RECORD_EFLAGS_REGNUM], >> - &eflags); >> - if ((eflags >> 10) & 0x1) >> - tmpulongest -= (count - 1) * (1 << ir.ot); >> - if (record_arch_list_add_mem (tmpulongest, count * (1 << ir.ot))) >> - return -1; >> - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM); >> - } >> - else >> - { >> + &tmpulongest); >> + if (ir.aflag) >> + { >> + /* 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 segment " >> + "register.\n"), >> + paddress (gdbarch, ir.addr)); >> + } >> + 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); >> - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); >> + 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); >> + I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); >> + } >> break; >> >> /* cmpsS */ >