Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Hui Zhu <teawater@gmail.com>
To: Michael Snyder <msnyder@vmware.com>
Cc: gdb-patches ml <gdb-patches@sourceware.org>
Subject: Re: Bug in i386_process_record?
Date: Tue, 11 Aug 2009 18:31:00 -0000	[thread overview]
Message-ID: <daef60380908102319k56787d57w49ea1cf4ed338584@mail.gmail.com> (raw)
In-Reply-To: <4A8097B4.2080709@vmware.com>

[-- Attachment #1: Type: text/plain, Size: 4521 bytes --]

On Tue, Aug 11, 2009 at 05:57, Michael Snyder <msnyder@vmware.com> 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 <memset+55>:	rep stos %eax,%es:(%edi)
0xb7edf4e9 <memset+57>:	mov    %edx,%ecx
0xb7edf4eb <memset+59>:	rep stos %al,%es:(%edi)
0xb7edf4ed <memset+61>:	mov    0x8(%esp),%eax
0xb7edf4f1 <memset+65>:	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.

Thanks,
Hui

2009-08-11  Hui Zhu  <teawater@gmail.com>

	* record.c (i386_process_record): Remove some error code.

---
 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 */

[-- Attachment #2: prec-fix-x86-strinsn.txt --]
[-- Type: text/plain, Size: 3380 bytes --]

---
 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 */

  parent reply	other threads:[~2009-08-11  6:20 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <4A7BA1DE.6010103@vmware.com>
2009-08-10  9:33 ` Hui Zhu
2009-08-10 22:12   ` Michael Snyder
2009-08-11  6:20     ` Hui Zhu
2009-08-11 18:31     ` Hui Zhu [this message]
2009-08-16 16:12       ` Hui Zhu
2009-08-18  5:35       ` Michael Snyder
2009-08-18 11:52         ` Hui Zhu
2009-08-21  3:23           ` Hui Zhu
2009-08-23  3:15             ` Michael Snyder
2009-08-23  3:33               ` Hui Zhu
2009-08-23  4:13                 ` Michael Snyder
2009-08-23  9:04                   ` Hui Zhu
2009-08-23 17:37                     ` Hui Zhu
2009-08-23 18:23                     ` Michael Snyder
2009-08-23 18:32                     ` Eli Zaretskii
2009-08-23 23:53                       ` Hui Zhu
2009-08-23 23:56                         ` Daniel Jacobowitz
2009-08-24  0:01                           ` Hui Zhu
2009-08-24  7:46                           ` Eli Zaretskii
2009-08-24  3:15                         ` Hui Zhu
2009-08-24 19:20                           ` Eli Zaretskii
2009-08-25  5:04                             ` Hui Zhu
2009-08-25 18:45                               ` Eli Zaretskii
2009-08-26  3:19                                 ` Hui Zhu
2009-08-26  3:27                                   ` Eli Zaretskii
2009-08-26  7:20                                     ` Hui Zhu
2009-08-26 17:37                                       ` Eli Zaretskii
2009-08-27  0:05                                   ` Michael Snyder
2009-08-27  0:32                                     ` Michael Snyder
2009-08-27  1:50                                       ` Hui Zhu
2009-08-27 15:35                                         ` Hui Zhu
2009-08-28  1:44                                           ` Michael Snyder
2009-08-28  2:14                                             ` Hui Zhu
2009-08-28  6:16                                               ` Michael Snyder
2009-08-28  8:46                                                 ` Hui Zhu
2009-08-30  1:12                                                   ` Michael Snyder
2009-08-27  1:44                                     ` Hui Zhu
2009-08-29  6:51                                     ` Hui Zhu
2009-08-24 20:31                         ` Eli Zaretskii
2009-08-25  6:53                           ` Hui Zhu
2009-08-23 18:24                   ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=daef60380908102319k56787d57w49ea1cf4ed338584@mail.gmail.com \
    --to=teawater@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=msnyder@vmware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox