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

[Michael Snyder <msnyder at vmware dot com>]

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.



> ---
>  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 "