Re: [RFA/prec] Make i386 handle segment register better

[Hui Zhu <teawater@gmail.com>]

On Mon, Sep 7, 2009 at 08:06, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> On Sun, Sep 6, 2009 at 14:52, Hui Zhu<teawater@gmail.com> wrote:
>>>
>>> Hi guys,
>>>
>>> Sorry I didn't do more test for this patch on amd64 before I check it in.
>>>
>>> But this patch really work not very good in amd64.
>>>
> [...]
>>>
>>> I think remove this patch from gdb-cvs-head before 7.0 branch  and
>>> make the segment reg clear is better.
>>>
>>> What  do you think about it?
>>>
>>> Thanks,
>>> Hui
>>>
>>
>> I make a patch for it.  Please help me review it.
>>
>> Thanks,
>> Hui
>>
>> 2009-09-06  Hui Zhu  <teawater@gmail.com>
>>
>>        * i386-tdep.c (i386_record_check_override): Deleted.
>>        (i386_record_lea_modrm): Ditto.
>>        (i386_process_record): Ditto.
>
>        (i386_process_record): Revert the use of deleted
>        function 'i386_record_check_override'.
>        (i386_record_lea_modrm): Ditto.
>
> Looks to me like you've preserved what you could
> of the change, and removed the minimum.  Good.
>
> This looks ok to commit.
>
>

Thanks.  Checked in.

Hui

>
>>
>> ---
>>  i386-tdep.c |   37 +++++++++++--------------------------
>>  1 file changed, 11 insertions(+), 26 deletions(-)
>>
>> --- a/i386-tdep.c
>> +++ b/i386-tdep.c
>> @@ -3148,26 +3148,6 @@ no_rm:
>>   return 0;
>>  }
>>
>> -static int
>> -i386_record_check_override (struct i386_record_s *irp)
>> -{
>> -  if (irp->override >= 0 && irp->override != X86_RECORD_DS_REGNUM)
>> -    {
>> -      ULONGEST orv, ds;
>> -
>> -      regcache_raw_read_unsigned (irp->regcache,
>> -                                  irp->regmap[irp->override],
>> -                                  &orv);
>> -      regcache_raw_read_unsigned (irp->regcache,
>> -                                  irp->regmap[X86_RECORD_DS_REGNUM],
>> -                                  &ds);
>> -      if (orv != ds)
>> -        return 1;
>> -    }
>> -
>> -  return 0;
>> -}
>> -
>>  /* Record the value of the memory that willbe changed in current
>> instruction
>>    to "record_arch_list".
>>    Return -1 if something wrong. */
>> @@ -3178,7 +3158,7 @@ i386_record_lea_modrm (struct i386_recor
>>   struct gdbarch *gdbarch = irp->gdbarch;
>>   uint64_t addr;
>>
>> -  if (i386_record_check_override (irp))
>> +  if (irp->override >= 0)
>>     {
>>       warning (_("Process record ignores the memory change "
>>                  "of instruction at address %s because it "
>> @@ -4060,7 +4040,7 @@ reswitch:
>>       /* mov EAX */
>>     case 0xa2:
>>     case 0xa3:
>> -      if (i386_record_check_override (&ir))
>> +      if (ir.override >= 0)
>>         {
>>          warning (_("Process record ignores the memory change "
>>                      "of instruction at address 0x%s because "
>> @@ -4478,8 +4458,13 @@ reswitch:
>>                                       ir.regmap[X86_RECORD_REDI_REGNUM],
>>                                       &tmpulongest);
>>
>> -          ir.override = X86_RECORD_ES_REGNUM;
>> -          if (ir.aflag && i386_record_check_override (&ir))
>> +          regcache_raw_read_unsigned (ir.regcache,
>> +                                      ir.regmap[X86_RECORD_ES_REGNUM],
>> +                                      &es);
>> +          regcache_raw_read_unsigned (ir.regcache,
>> +                                      ir.regmap[X86_RECORD_DS_REGNUM],
>> +                                      &ds);
>> +          if (ir.aflag && (es != ds))
>>             {
>>               /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4;
>> */
>>               warning (_("Process record ignores the memory "
>> @@ -5103,7 +5088,7 @@ reswitch:
>>                opcode = opcode << 8 | ir.modrm;
>>                goto no_support;
>>              }
>> -           if (i386_record_check_override (&ir))
>> +           if (ir.override >= 0)
>>              {
>>                warning (_("Process record ignores the memory "
>>                            "change of instruction at "
>> @@ -5154,7 +5139,7 @@ reswitch:
>>          else
>>            {
>>              /* sidt */
>> -             if (i386_record_check_override (&ir))
>> +             if (ir.override >= 0)
>>                {
>>                  warning (_("Process record ignores the memory "
>>                              "change of instruction at "
>>
>
>