Re: [RFA/RFC Prec] Add Linux AMD64 process record support second version, (instruction set support) 1/3

[Michael Snyder <msnyder at vmware dot com>]

Hui Zhu wrote:
> Because AMD64 instruction set is the extend of I386 instruction set.
> So I update the function i386_process_record to make it support AMD64
> instruction set.  some of other software do something like it.
> 2009-07-07  Hui Zhu  <teawater@gmail.com>
>
>         Add AMD64 process record instruction set support.
>
>         * i386-tdep.h (gdbarch_tdep): Add record_regmap for registers
>         because the AMD64's registers order in GDB is not same with
>         I386 instructions.
>         Add i386_syscall_record to be the syscall function handle
>         interface.
>         (record_i386_regnum): Number for record_regmap.
>         * i386-tdep.c (OT_QUAD): For 64 bits.
>         (i386_record_s): Add rex_x, rex_b, rip_offset and
>         popl_esp_hack for AMD64 instruction set. And regmap for
>         record_regmap.
>         (i386_record_lea_modrm_addr): Support AMD64 instruction set
>         64 bits lea.
>         (i386_record_lea_modrm): Ditto.
>         (i386_record_push): New function.  Record the execution log
>         of push.
>         (I386_RECORD_ARCH_LIST_ADD_REG): New macro to record the
>         register.
>         (i386_process_record): Support AMD64 instruction set.
>         amd64-tdep.c (amd64_record_regmap): For record_regmap.
>         (amd64_init_abi): Set amd64_record_regmap to record_regmap.

OK, this is kind of preliminary -- mostly concerned with
whitespace / formatting.  I'll try to do more later.


> @@ -2859,7 +2868,7 @@ i386_record_lea_modrm_addr (struct i386_
>           if ((base & 7) == 5)
>             {
>               base = 0xff;
> -             if (target_read_memory (irp->addr, (gdb_byte *) addr, 4))
> +             if (target_read_memory (irp->addr, (gdb_byte *)&tmpi32, 4))

Space after a cast, please.  There are many instances of this,
I won't try to note them all.

> @@ -2884,10 +2896,10 @@ i386_record_lea_modrm_addr (struct i386_
>               return -1;
>             }
>           irp->addr++;
> -         *addr = (int8_t) tmpu8;
> +         *addr = (int8_t)tmpu8;

Space after cast

> @@ -2949,10 +2974,10 @@ i386_record_lea_modrm_addr (struct i386_
>               return -1;
>             }
>           irp->addr++;
> -         *addr = (int8_t) tmpu8;
> +         *addr = (int8_t)tmpu8;
>           break;
>         case 2:
> -         if (target_read_memory (irp->addr, (gdb_byte *) & tmpu16, 2))
> +         if (target_read_memory (irp->addr, (gdb_byte *) & tmpi16, 2))

No space after '&' operator.  In general, we use a space
both before and after a binary operator (such as 'plus'),
but no space after a unary operator such as '&', '*' or '-'.

> +         if (ir.aflag == 2)
> +           {
> +              if (target_read_memory
> +                 (ir.addr, (gdb_byte *)&addr, 8))

Please try to avoid doing this (putting the left-paren of a
function call on the next line).  Occasionally when a line gets
really long I tend to overlook it, but in this case it would
be really easy to divide the line up like this:

               if (target_read_memory (ir.addr,
                                       (gdb_byte *) &addr, 8))

> +          if (ir.mod ==3)

Space after ==

> +          switch (ir.dflag)
> +            {
> +            case 0:
> +              tmpu64 += ((int16_t)tmpulongest >> 4)<< 4;

Space before <<, space after cast.

> +              break;
> +            case 1:
> +              tmpu64 += ((int32_t)tmpulongest >> 5)<< 5;

Ditto

> +              break;
> +            case 2:
> +              tmpu64 += ((int64_t)tmpulongest >> 6)<< 6;

Ditto

> +  /* The map for registers because the AMD64's registers order in GDB is not
> +     same with I386 instructions.  */

"same as".  And could you shorten the first line please?