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

Michael Snyder
2009-07-13T00:38:56Z

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?