Re: Bug in i386_process_record?

[Hui Zhu <teawater at gmail dot com>]

On Fri, Aug 7, 2009 at 11:39, Michael Snyder<msnyder@vmware.com> wrote:
> Hi Hui,
>
> While experimenting with your dump/load commands, I think I discovered
> a bug in i386_process_record, in the handling of the "string ops"
> and the "rep" prefix. ?Looks like we are saving the same data over
> and over in the log.
>
> This was made using the attached sample program.
>
> ?(gdb) break main
> ? ?Breakpoint 1 at 0x80483c4: file memrange-reverse.c, line 29.
> ?(gdb) run
> ? ?Starting program:
> ? ?Breakpoint 1, main ()
> ? ?29 ? ? ? ?memset (blob1, 'a', sizeof (blob1));
> ?(gdb) record
> ?(gdb) next
> ? ?30 ? ? ? ?blob1[sizeof (blob1) - 1] = '\0';
> ?(gdb) record dump
> ? ?Saving recording to file 'rec.27255'
> ? ?Writing 4-byte magic cookie RECORD_FILE_MAGIC (0x26070920)
> ?[...]
> ?Writing register 7 val 0x0000000008049684 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049680 (1 plus 8 plus 8 bytes plus 1024 bytes)
> ?Writing register 1 val 0x00000000000000ff (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x0000000008049688 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049684 (1 plus 8 plus 8 bytes plus 1020 bytes)
> ?Writing register 1 val 0x00000000000000fe (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x000000000804968c (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049688 (1 plus 8 plus 8 bytes plus 1016 bytes)
> ?Writing register 1 val 0x00000000000000fd (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x0000000008049690 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x0804968c (1 plus 8 plus 8 bytes plus 1012 bytes)
> ?Writing register 1 val 0x00000000000000fc (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x0000000008049694 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049690 (1 plus 8 plus 8 bytes plus 1008 bytes)
> ?Writing register 1 val 0x00000000000000fb (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x0000000008049698 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049694 (1 plus 8 plus 8 bytes plus 1004 bytes)
> ?Writing register 1 val 0x00000000000000fa (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x000000000804969c (1 plus 8 plus 16 bytes)
> ?Writing memory 0x08049698 (1 plus 8 plus 8 bytes plus 1000 bytes)
> ?Writing register 1 val 0x00000000000000f9 (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?Writing record_end (1 byte)
> ?Writing register 7 val 0x00000000080496a0 (1 plus 8 plus 16 bytes)
> ?Writing memory 0x0804969c (1 plus 8 plus 8 bytes plus 996 bytes)
> ?Writing register 1 val 0x00000000000000f8 (1 plus 8 plus 16 bytes)
> ?Writing register 8 val 0x0000000000587be7 (1 plus 8 plus 16 bytes)
> ?[...]
>
> Altogether there were 256 duplicate entries, each one is
> four bytes shorter than the previous one.
>
>

Hi Michael,

I reproduce about issue.  This is because "i386_process_record" record
rep string insn is not right.
I make a patch for it.

Please help me review it.

Thanks,
Hui

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

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

---
 i386-tdep.c |   27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -4448,9 +4448,8 @@ reswitch:
       regcache_raw_read_unsigned (ir.regcache,
                                   ir.regmap[X86_RECORD_REDI_REGNUM],
                                   &tmpulongest);
-      if (!ir.aflag)
+      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 "
@@ -4460,27 +4459,9 @@ reswitch:
                                paddress (gdbarch, ir.addr));
         }
       if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ))
-        {
-          ULONGEST count, eflags;
-          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
-        {
-          if (record_arch_list_add_mem (tmpulongest, 1 << ir.ot))
-            return -1;
-        }
+        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);

Attachment: prec-fix-x86-strinsn.txt
Description: Text document