Re: Bug in i386_process_record?

Michael Snyder
2009-08-10T21:59:15Z

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.

Hui Zhu wrote:
> 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);