Re: Bug in i386_process_record?

[Hui Zhu <teawater at gmail dot com>]

Ping

On Tue, Aug 11, 2009 at 14:19, Hui Zhu<teawater@gmail.com> 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.
>
> 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 */
>