Re: Bug in i386_process_record?

[Hui Zhu <teawater at gmail dot com>]

On Tue, Aug 18, 2009 at 17:21, Hui Zhu <teawater@gmail.com> wrote:
>
> On Tue, Aug 18, 2009 at 12:15, Michael Snyder<msnyder@vmware.com> wrote:
> > Hui Zhu wrote:
> >>
> >> 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.
> >
> > 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 ?<teawater@gmail.com>
>
> ? ? ? ?* 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  <teawater@gmail.com>

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