This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

Thanks Michael,

I updated all the AMD64 patches. Please help me review it.


2009-07-17 Hui Zhu <>

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
        (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
        (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
        (i386_process_record): Support AMD64 instruction set.
        amd64-tdep.c (amd64_record_regmap): For record_regmap.

Should be a "*" at the beginning of the line above.

@@ -2864,6 +2865,7 @@ enum

 struct i386_record_s
+  struct gdbarch *gdbarch;
   struct regcache *regcache;
   CORE_ADDR addr;
   int aflag;
@@ -2872,6 +2874,11 @@ struct i386_record_s
   uint8_t modrm;
   uint8_t mod, reg, rm;
   int ot;
+  uint8_t rex_x;
+  uint8_t rex_b;
+  int rip_offset;
+  int popl_esp_hack;

popl_esp_hack is used but never initialized. Do you still need it? Is it for some future use?

@@ -4611,14 +4633,13 @@ reswitch:
     case 0x0f9d:
     case 0x0f9e:
     case 0x0f9f:
       ir.ot = OT_BYTE;
       if (i386_record_modrm (&ir))
        return -1;
       if (ir.mod == 3)
-       {
-         if (record_arch_list_add_reg (ir.regcache, ir.rm & 0x3))
-           return -1;
-       }
+        I386_RECORD_ARCH_LIST_ADD_REG (ir.rex_b ? (ir.rm | ir.rex_b):
+                                                  (ir.rm & 0x3));

Space before colon operator (':'), and then the 2nd line isn't indented right. I know it looks prettier the way it is, but it should be over to the left. ;-)

@@ -5115,21 +5225,21 @@ reswitch:

       /* arpl */
     case 0x63:
-      ir.ot = ir.dflag ? OT_LONG : OT_WORD;
       if (i386_record_modrm (&ir))
        return -1;
-      if (ir.mod != 3)
-       {
-         if (i386_record_lea_modrm (&ir))
-           return -1;
-       }
+      if (ir.mod == 3 || ir.regmap[X86_RECORD_R8_REGNUM])
+        {
+          I386_RECORD_ARCH_LIST_ADD_REG (ir.regmap[X86_RECORD_R8_REGNUM]?
+                                           (ir.reg | rex_r) : ir.rm);

Space before question-mark, and then the indentation of the second line.

@@ -5232,9 +5341,8 @@ reswitch:

-/* In the future, Maybe still need to deal with need_dasm */
-  if (record_arch_list_add_reg (ir.regcache, I386_EIP_REGNUM))
-    return -1;
+  /* In the future, Maybe still need to deal with need_dasm */

"maybe" lower case. Period and two spaces at end of sentence.

Everything else in this patch looks great!

Mark Kettenis should approve it, though.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]