public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
@ 2013-10-24  0:09 Omair Javaid
  2013-10-24  2:25 ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2013-10-24  0:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Patch Tracking

This patch adds support for recording thumb32 and syscall instruction
for arm targets.
This patch also fixes bugs in arm process record instruction decoding.

gdb:

2013-10-24  Omair Javaid  <omair.javaid@linaro.org>

* arm-tdep.c (struct arm_mem_r): Update.
(arm_record_coproc_data_proc): Update.
(thumb_record_ldm_stm_swi): Update.
(thumb2_record_ld_st_multiple): New function.
(thumb2_record_ld_st_dual_ex_tbb): New function.
(thumb2_record_data_proc_sreg_mimm): New function.
(thumb2_record_unsupported_insn): New function.
(thumb2_record_ps_dest_generic): New function.
(thumb2_record_branch_misc_cntrl): New function.
(thumb2_record_str_single_data): New function.
(thumb2_record_ld_mem_hints): New function.
(thumb2_record_ld_word): New function.
(thumb2_record_lmul_lmla_div): New function.
(thumb2_record_decode_inst_id): New function.
(decode_insn): Update.

Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.381
diff -u -p -r1.381 arm-tdep.c
--- gdb/arm-tdep.c 24 Jun 2013 22:18:31 -0000 1.381
+++ gdb/arm-tdep.c 23 Oct 2013 20:00:45 -0000
@@ -10618,7 +10618,7 @@ vfp - VFP co-processor."),
 struct arm_mem_r
 {
   uint32_t len;    /* Record length.  */
-  CORE_ADDR addr;  /* Memory address.  */
+  uint32_t addr;   /* Memory address.  */
 };

 /* ARM instruction record contains opcode of current insn
@@ -11936,27 +11936,43 @@ arm_record_coproc_data_proc (insn_decode
   struct gdbarch_tdep *tdep = gdbarch_tdep (arm_insn_r->gdbarch);
   struct regcache *reg_cache = arm_insn_r->regcache;
   uint32_t ret = 0; /* function return value: -1:record failure ;
0:success  */
-
-  /* Handle SWI insn; system call would be handled over here.  */
+  ULONGEST u_regval = 0;

   arm_insn_r->opcode = bits (arm_insn_r->arm_insn, 24, 27);
+
+  /* Handle arm SWI/SVC system call instructions.  */
   if (15 == arm_insn_r->opcode)
-  {
-    /* Handle arm syscall insn.  */
-    if (tdep->arm_swi_record != NULL)
-      {
-        ret = tdep->arm_swi_record(reg_cache);
-      }
-    else
-      {
-        printf_unfiltered (_("no syscall record support\n"));
-        ret = -1;
-      }
-  }
+    {
+      if (tdep->arm_syscall_record != NULL)
+        {
+          ULONGEST svc_operand, svc_number;

-  printf_unfiltered (_("Process record does not support instruction "
+          svc_operand = (0x00ffffff & arm_insn_r->arm_insn);
+
+          if (svc_operand)  /* OABI.  */
+            {
+              svc_number = svc_operand - 0x900000;
+            }
+          else /* EABI.  */
+            {
+              regcache_raw_read_unsigned (reg_cache, 7, &svc_number);
+            }
+
+          ret = tdep->arm_syscall_record(reg_cache, svc_number);
+        }
+      else
+        {
+          printf_unfiltered (_("no syscall record support\n"));
+          ret = -1;
+        }
+    }
+  else
+    {
+      printf_unfiltered (_("Process record does not support instruction "
                         "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
                         paddress (arm_insn_r->gdbarch, arm_insn_r->this_addr));
+      ret = -1;
+    }
   return ret;
 }

@@ -12361,9 +12377,10 @@ thumb_record_ldm_stm_swi (insn_decode_re
   else if (0x1F == opcode1)
     {
         /* Handle arm syscall insn.  */
-        if (tdep->arm_swi_record != NULL)
+        if (tdep->arm_syscall_record != NULL)
           {
-            ret = tdep->arm_swi_record(reg_cache);
+            regcache_raw_read_unsigned (reg_cache, 7, &u_regval);
+            ret = tdep->arm_syscall_record(reg_cache, u_regval);
           }
         else
           {
@@ -12414,6 +12431,630 @@ thumb_record_branch (insn_decode_record
   return 0;
 }

+/* Handler for thumb2 load/store multiple instructions.  */
+
+static int
+thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, op;
+  uint32_t register_bits = 0, register_count = 0;
+  uint32_t index = 0, start_address = 0;
+  uint32_t record_buf[24], record_buf_mem[48];
+
+  ULONGEST u_regval = 0;
+
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  op = bits (thumb2_insn_r->arm_insn, 23, 24);
+
+  if (0 == op || 3 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle RFE instruction.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          /* Handle SRS instruction after reading banked SP.  */
+          printf_unfiltered (_("Process record does not support instruction "
+                            "0x%0x at address %s.\n"),
+                            thumb2_insn_r->arm_insn,
+                            paddress (thumb2_insn_r->gdbarch,
+                            thumb2_insn_r->this_addr));
+          return -1;
+        }
+    }
+  else if(1 == op || 2 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                {
+                  record_buf[index++] = register_count;
+                }
+              register_count++;
+              register_bits = register_bits >> 1;
+            }
+          record_buf[index++] = reg_rn;
+          record_buf[index++] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = index;
+        }
+      else
+        {
+          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                {
+                  register_count++;
+                }
+              register_bits = register_bits >> 1;
+            }
+
+          if (1 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = u_regval;
+            }
+          else if (2 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = (u_regval) - (register_count * 4);
+            }
+
+          thumb2_insn_r->mem_rec_count = register_count;
+          while (register_count)
+            {
+              record_buf_mem[(register_count * 2) - 1] = start_address;
+              record_buf_mem[(register_count * 2) - 2] = 4;
+              start_address = start_address + 4;
+              register_count--;
+            }
+          record_buf[0] = reg_rn;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+    }
+
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
instructions.  */
+
+static int
+thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rd, reg_rn, offset_imm;
+  uint32_t reg_dest1, reg_dest2;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2, op3;
+  LONGEST s_word;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 23, 24);
+  op2 = bits (thumb2_insn_r->arm_insn, 20, 21);
+  op3 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+    {
+      if(!(1 == op1 && 1 == op2 && (0 == op3 || 1 == op3)))
+        {
+          reg_dest1 = bits (thumb2_insn_r->arm_insn, 12, 15);
+          record_buf[0] = reg_dest1;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+
+      if (3 == op2 || (op1 & 2) || (1 == op1 && 1 == op2 && 7 == op3))
+        {
+          reg_dest2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+          record_buf[2] = reg_dest2;
+          thumb2_insn_r->reg_rec_count = 3;
+        }
+    }
+  else
+    {
+      reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+      regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+      if (0 == op1 && 0 == op2)
+        {
+          /* Handle STREX.  */
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          address = u_regval[0] + (offset_imm * 4);
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          thumb2_insn_r->mem_rec_count = 1;
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else if (1 == op1 && 0 == op2)
+        {
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+          address = u_regval[0];
+          record_buf_mem[1] = address;
+
+          if (4 == op3)
+            {
+              /* Handle STREXB.  */
+              record_buf_mem[0] = 1;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (5 == op3)
+            {
+              /* Handle STREXH.  */
+              record_buf_mem[0] = 2 ;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (7 == op3)
+            {
+              /* Handle STREXD.  */
+              address = u_regval[0];
+              record_buf_mem[0] = 4;
+              record_buf_mem[2] = 4;
+              record_buf_mem[3] = address + 4;
+              thumb2_insn_r->mem_rec_count = 2;
+            }
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+
+          if (bit (thumb2_insn_r->arm_insn, 24))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 23))
+                {
+                  offset_addr = u_regval[0] + (offset_imm * 4);
+                }
+              else
+                {
+                  offset_addr = u_regval[0] - (offset_imm * 4);
+                }
+              address = offset_addr;
+            }
+          else
+            {
+              address = u_regval[0];
+            }
+
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          record_buf_mem[2] = 4;
+          record_buf_mem[3] = address + 4;
+          thumb2_insn_r->mem_rec_count = 2;
+          record_buf[0] = reg_rn;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 data processing (shift register and modified immediate)
+   instructions.  */
+
+static int
+thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */
+  uint32_t reg_rd, op;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 21, 24);
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
+    {
+      record_buf[0] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 1;
+    }
+  else
+    {
+      record_buf[0] = reg_rd;
+      record_buf[1] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+
+  return ret;
+}
+
+/* Handler for thumb2 all unsupported instructions.  */
+
+static int
+thumb2_record_unsupported_insn (insn_decode_record *thumb2_insn_r)
+{
+  printf_unfiltered (_("Process record does not support instruction "
+                    "0x%0x at address %s.\n"),thumb2_insn_r->arm_insn,
+                    paddress (thumb2_insn_r->gdbarch,
+                    thumb2_insn_r->this_addr));
+
+  return -1;
+}
+
+/* Generic handler for thumb2 instructions which need to save PS and
+   destination registers is saved. Handles multiply, multiply accumulate,
+   and absolute difference instructions. Also handles data-processing
+   (register and plain binary immediate) instructions.  */
+
+static int
+thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */
+  uint32_t reg_rd;
+  uint32_t record_buf[8];
+
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  record_buf[0] = reg_rd;
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+
+  return ret;
+}
+
+/* Handler for thumb2 branch and miscellaneous control instructions.  */
+
+static int
+thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t op, op1, op2;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 20, 26);
+  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
+  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  /* Handle MSR insn.  */
+  if (!(op1 & 0x2) && 0x38 == op)
+    {
+      if (!(op2 & 0x3))
+        {
+          /* CPSR is going to be changed.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          /* SPSR is going to be changed.  */
+          printf_unfiltered (_("Process record does not support instruction "
+                            "0x%0x at address %s.\n"),
+                            thumb2_insn_r->arm_insn,
+                            paddress (thumb2_insn_r->gdbarch,
+                            thumb2_insn_r->this_addr));
+          return -1;
+        }
+    }
+  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
+    {
+      /* BLX.  */
+      record_buf[0] = ARM_PS_REGNUM;
+      record_buf[1] = ARM_LR_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+
+  return 0;
+}
+
+/* Handler for thumb2 store single data item instructions.  */
+
+static int
+thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
+  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+  if (bit (thumb2_insn_r->arm_insn, 23))
+    {
+      /* T2 encoding.  */
+      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
+      offset_addr = u_regval[0] + offset_imm;
+      address = offset_addr;
+    }
+  else
+    {
+      /* T3 encoding.  */
+      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
+        {
+          /* Handle STRB (register).  */
+          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
+          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
+          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
+          offset_addr = u_regval[1] << shift_imm;
+          address = u_regval[0] + offset_addr;
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          if (bit (thumb2_insn_r->arm_insn, 10))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 9))
+                {
+                  offset_addr = u_regval[0] + offset_imm;
+                }
+              else
+                {
+                  offset_addr = u_regval[0] - offset_imm;
+                }
+              address = offset_addr;
+            }
+          else
+            {
+              address = u_regval[0];
+            }
+        }
+    }
+
+  switch (op1)
+    {
+      /* Store byte instructions.  */
+      case 4:
+      case 0:
+        record_buf_mem[0] = 1;
+      break;
+      /* Store half word instructions.  */
+      case 1:
+      case 5:
+        record_buf_mem[0] = 2;
+      break;
+      /* Store word instructions.  */
+      case 2:
+      case 6:
+        record_buf_mem[0] = 4;
+      break;
+
+      default:
+        gdb_assert_not_reached ("no decoding pattern found");
+      break;
+    }
+
+  record_buf_mem[1] = address;
+  thumb2_insn_r->mem_rec_count = 1;
+  record_buf[0] = reg_rn;
+  thumb2_insn_r->reg_rec_count = 1;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 load memory hints instructions.  */
+
+static int
+thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t record_buf[8];
+  uint32_t reg_rt, reg_rn;
+
+  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+
+  if (15 != reg_rt)
+    {
+      record_buf[0] = reg_rt;
+      record_buf[1] = reg_rn;
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+
+      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+                record_buf);
+      return 0;
+    }
+
+  return -1;
+}
+
+/* Handler for thumb2 load word instructions.  */
+
+static int
+thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+
+  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+
+  return ret;
+}
+
+/* Handler for thumb2 long multiply, long multiply accumulate, and
+   divide instructions.  */
+
+static int
+thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+  uint32_t reg_src1 = 0;
+
+  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
+  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
+    {
+      /* Handle SMULL, UMULL, SMULAL.  */
+      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else if (1 == opcode1 || 3 == opcode2)
+    {
+      /* Handle SDIV and UDIV.  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else
+    {
+      ret = -1;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+
+  return ret;
+}
+
+/* Decodes thumb2 instruction type and return an instruction id.  */
+
+static unsigned int
+thumb2_record_decode_inst_id (uint32_t thumb2_insn)
+{
+  uint32_t op = 0;
+  uint32_t op1 = 0;
+  uint32_t op2 = 0;
+
+  op = bit (thumb2_insn, 15);
+  op1 = bits (thumb2_insn, 27, 28);
+  op2 = bits (thumb2_insn, 20, 26);
+
+  if (op1 == 0x01)
+    {
+      if (!(op2 & 0x64 ))
+        {
+          /* Load/store multiple instruction.  */
+          return 0;
+        }
+      else if (!((op2 & 0x64) ^ 0x04))
+        {
+          /* Load/store dual, load/store exclusive, table branch
instruction.  */
+          return 1;
+        }
+      else if (!((op2 & 0x20) ^ 0x20))
+        {
+          /* Data-processing (shifted register).  */
+          return 2;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 3;
+        }
+    }
+  else if (op1 == 0x02)
+    {
+      if (op)
+        {
+          /* Branches and miscellaneous control instructions.  */
+          return 6;
+        }
+      else if (op2 & 0x20)
+        {
+          /* Data-processing (plain binary immediate) instruction.  */
+          return 5;
+        }
+      else
+        {
+          /* Data-processing (modified immediate).  */
+          return 4;
+        }
+    }
+  else if (op1 == 0x03)
+   {
+      if (!(op2 & 0x71 ))
+        {
+          /* Store single data item.  */
+          return 7;
+        }
+      else if (!((op2 & 0x71) ^ 0x10))
+        {
+          /* Advanced SIMD element or structure load/store instructions.  */
+          return 8;
+        }
+      else if (!((op2 & 0x67) ^ 0x01))
+        {
+          /* Load byte, memory hints instruction.  */
+          return 9;
+        }
+      else if (!((op2 & 0x67) ^ 0x03))
+        {
+          /* Load halfword, memory hints instruction.  */
+          return 10;
+        }
+      else if (!((op2 & 0x67) ^ 0x05))
+        {
+          /* Load word instruction.  */
+          return 11;
+        }
+      else if (!((op2 & 0x70) ^ 0x20))
+        {
+          /* Data-processing (register) instruction.  */
+          return 12;
+        }
+      else if (!((op2 & 0x78) ^ 0x30))
+        {
+          /* Multiply, multiply accumulate, absolute difference
instruction.  */
+          return 13;
+        }
+      else if (!((op2 & 0x78) ^ 0x38))
+        {
+          /* Long multiply, long multiply accumulate, and divide.  */
+          return 14;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 15;
+        }
+   }
+
+  return -1;
+}

 /* Extracts arm/thumb/thumb2 insn depending on the size, and returns
0 on success
 and positive val on fauilure.  */
@@ -12469,6 +13110,27 @@ decode_insn (insn_decode_record *arm_rec
     thumb_record_branch                /* 111.  */
   };

+  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
instruction.  */
+  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
+  { \
+    thumb2_record_ld_st_multiple,       /* 00.  */
+    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
+    thumb2_record_unsupported_insn,     /* 03.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
+    thumb2_record_ps_dest_generic,      /* 05.  */
+    thumb2_record_branch_misc_cntrl,    /* 06.  */
+    thumb2_record_str_single_data,      /* 07.  */
+    thumb2_record_unsupported_insn,     /* 08.  */
+    thumb2_record_ld_mem_hints,         /* 09.  */
+    thumb2_record_ld_mem_hints,         /* 10.  */
+    thumb2_record_ld_word,              /* 11.  */
+    thumb2_record_ps_dest_generic,      /* 12.  */
+    thumb2_record_ps_dest_generic,      /* 13.  */
+    thumb2_record_lmul_lmla_div,        /* 14.  */
+    thumb2_record_unsupported_insn      /* 15.  */
+  };
+
   uint32_t ret = 0;    /* return value: negative:failure   0:success.  */
   uint32_t insn_id = 0;

@@ -12503,11 +13165,27 @@ decode_insn (insn_decode_record *arm_rec
     }
   else if (THUMB2_RECORD == record_type)
     {
-      printf_unfiltered (_("Process record doesnt support thumb32 instruction "
-                           "0x%0x at address %s.\n"),arm_record->arm_insn,
-                           paddress (arm_record->gdbarch,
-                           arm_record->this_addr));
-      ret = -1;
+      /* As thumb does not have condition codes, we set negative.  */
+      arm_record->cond = -1;
+
+      /* Swap first half of 32bit thumb instruction with second half.  */
+      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
+                             (arm_record->arm_insn << 16);
+
+      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
+
+      if (insn_id >= 0)
+        {
+          ret = thumb2_handle_insn[insn_id] (arm_record);
+        }
+      else
+        {
+          printf_unfiltered (_("Process record doesnt support instruction "
+                            "0x%0x at address %s.\n"),arm_record->arm_insn,
+                            paddress (arm_record->gdbarch,
+                            arm_record->this_addr));
+          ret = -1;
+        }
     }
   else
     {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-10-24  0:09 [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux* Omair Javaid
@ 2013-10-24  2:25 ` Yao Qi
  2013-11-08  3:20   ` Omair Javaid
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2013-10-24  2:25 UTC (permalink / raw)
  To: Omair Javaid; +Cc: gdb-patches, Patch Tracking

On 10/24/2013 08:09 AM, Omair Javaid wrote:
> gdb:
>
> 2013-10-24  Omair Javaid<omair.javaid@linaro.org>
>
> * arm-tdep.c (struct arm_mem_r): Update.

"Update" is too general.  Please describe what is changed.

> (arm_record_coproc_data_proc): Update.
> (thumb_record_ldm_stm_swi): Update.
> (thumb2_record_ld_st_multiple): New function.
> (thumb2_record_ld_st_dual_ex_tbb): New function.
> (thumb2_record_data_proc_sreg_mimm): New function.
> (thumb2_record_unsupported_insn): New function.
> (thumb2_record_ps_dest_generic): New function.
> (thumb2_record_branch_misc_cntrl): New function.
> (thumb2_record_str_single_data): New function.
> (thumb2_record_ld_mem_hints): New function.
> (thumb2_record_ld_word): New function.
> (thumb2_record_lmul_lmla_div): New function.
> (thumb2_record_decode_inst_id): New function.
> (decode_insn): Update.

Each line of changelog should be prefixed by tab.

> +    {
> +      if (tdep->arm_syscall_record != NULL)
> +        {
> +          ULONGEST svc_operand, svc_number;
>
> -  printf_unfiltered (_("Process record does not support instruction "
> +          svc_operand = (0x00ffffff & arm_insn_r->arm_insn);
> +
> +          if (svc_operand)  /* OABI.  */
> +            {
> +              svc_number = svc_operand - 0x900000;
> +            }

Unnecessary braces.

> +          else /* EABI.  */
> +            {
> +              regcache_raw_read_unsigned (reg_cache, 7, &svc_number);
> +            }


Unnecessary braces.

> +
> +          ret = tdep->arm_syscall_record(reg_cache, svc_number);
> +        }
> +      else
> +        {
> +          printf_unfiltered (_("no syscall record support\n"));
> +          ret = -1;
> +        }
> +    }
> +  else
> +    {
> +      printf_unfiltered (_("Process record does not support instruction "
>                           "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
>                           paddress (arm_insn_r->gdbarch, arm_insn_r->this_addr));

I find this statements appear many times, probably, we can move it into 
a function named arm_record_unsupported_insn.

> +      ret = -1;
> +    }
>     return ret;
>   }
>
> @@ -12361,9 +12377,10 @@ thumb_record_ldm_stm_swi (insn_decode_re
>     else if (0x1F == opcode1)
>       {
>           /* Handle arm syscall insn.  */
> -        if (tdep->arm_swi_record != NULL)
> +        if (tdep->arm_syscall_record != NULL)
>             {
> -            ret = tdep->arm_swi_record(reg_cache);
> +            regcache_raw_read_unsigned (reg_cache, 7, &u_regval);
> +            ret = tdep->arm_syscall_record(reg_cache, u_regval);

Space is needed before "(".  This change belongs to support recording 
syscall instructions, and IWBN to move this part to patch 2/2.

>             }
>           else
>             {
> @@ -12414,6 +12431,630 @@ thumb_record_branch (insn_decode_record
>     return 0;
>   }
>
> +/* Handler for thumb2 load/store multiple instructions.  */
> +
> +static int
> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rn, op;
> +  uint32_t register_bits = 0, register_count = 0;
> +  uint32_t index = 0, start_address = 0;
> +  uint32_t record_buf[24], record_buf_mem[48];
> +
> +  ULONGEST u_regval = 0;
> +
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +  op = bits (thumb2_insn_r->arm_insn, 23, 24);
> +
> +  if (0 == op || 3 == op)
> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle RFE instruction.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          /* Handle SRS instruction after reading banked SP.  */
> +          printf_unfiltered (_("Process record does not support instruction "
> +                            "0x%0x at address %s.\n"),
> +                            thumb2_insn_r->arm_insn,
> +                            paddress (thumb2_insn_r->gdbarch,
> +                            thumb2_insn_r->this_addr));

Use thumb2_record_unsupported_insn?

> +          return -1;
> +        }
> +    }
> +  else if(1 == op || 2 == op)
> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                {
> +                  record_buf[index++] = register_count;
> +                }

Unnecessary braces.

> +              register_count++;
> +              register_bits = register_bits >> 1;
> +            }
> +          record_buf[index++] = reg_rn;
> +          record_buf[index++] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = index;
> +        }
> +      else
> +        {
> +          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                {
> +                  register_count++;
> +                }

Unnecessary braces.

> +              register_bits = register_bits >> 1;
> +            }
> +
> +          if (1 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = u_regval;
> +            }
> +          else if (2 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = (u_regval) - (register_count * 4);
> +            }
> +
> +          thumb2_insn_r->mem_rec_count = register_count;
> +          while (register_count)
> +            {
> +              record_buf_mem[(register_count * 2) - 1] = start_address;
> +              record_buf_mem[(register_count * 2) - 2] = 4;
> +              start_address = start_address + 4;
> +              register_count--;
> +            }
> +          record_buf[0] = reg_rn;
> +          record_buf[1] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 2;
> +        }
> +    }
> +
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
> instructions.  */

Looks your mailer wraps the patch.  See 
https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches 
on how to adjust your mailer.

> +
> +/* Handler for thumb2 data processing (shift register and modified immediate)
> +   instructions.  */
> +
> +static int
> +thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */

This function always return 0.

> +  uint32_t reg_rd, op;
> +  uint32_t record_buf[8];
> +
> +  op = bits (thumb2_insn_r->arm_insn, 21, 24);
> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> +  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
> +    {
> +      record_buf[0] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 1;
> +    }
> +  else
> +    {
> +      record_buf[0] = reg_rd;
> +      record_buf[1] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 2;
> +    }
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +
> +  return ret;
> +}
> +

> +
> +/* Generic handler for thumb2 instructions which need to save PS and
> +   destination registers is saved. Handles multiply, multiply accumulate,

Two spaces after period.

> +   and absolute difference instructions. Also handles data-processing

and here.

> +   (register and plain binary immediate) instructions.  */
> +
> +static int
> +thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */

Document the meaning of return value in function header comment and 
remove this line of comment.

> +
> +/* Handler for thumb2 branch and miscellaneous control instructions.  */
> +
> +static int
> +thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t op, op1, op2;
> +  uint32_t record_buf[8];
> +
> +  op = bits (thumb2_insn_r->arm_insn, 20, 26);
> +  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
> +  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> +  /* Handle MSR insn.  */
> +  if (!(op1 & 0x2) && 0x38 == op)
> +    {
> +      if (!(op2 & 0x3))
> +        {
> +          /* CPSR is going to be changed.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          /* SPSR is going to be changed.  */
> +          printf_unfiltered (_("Process record does not support instruction "
> +                            "0x%0x at address %s.\n"),
> +                            thumb2_insn_r->arm_insn,
> +                            paddress (thumb2_insn_r->gdbarch,
> +                            thumb2_insn_r->this_addr));

Use thumb2_record_unsupported_insn?

> +          return -1;
> +        }
> +    }
> +  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
> +    {
> +      /* BLX.  */
> +      record_buf[0] = ARM_PS_REGNUM;
> +      record_buf[1] = ARM_LR_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 2;
> +    }
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +
> +  return 0;
> +}
> +
> +/* Handler for thumb2 store single data item instructions.  */
> +
> +static int
> +thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
> +  uint32_t address, offset_addr;
> +  uint32_t record_buf[8], record_buf_mem[8];
> +  uint32_t op1, op2;
> +
> +  ULONGEST u_regval[2];
> +
> +  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
> +  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
> +
> +  if (bit (thumb2_insn_r->arm_insn, 23))
> +    {
> +      /* T2 encoding.  */
> +      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
> +      offset_addr = u_regval[0] + offset_imm;
> +      address = offset_addr;
> +    }
> +  else
> +    {
> +      /* T3 encoding.  */
> +      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
> +        {
> +          /* Handle STRB (register).  */
> +          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
> +          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
> +          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
> +          offset_addr = u_regval[1] << shift_imm;
> +          address = u_regval[0] + offset_addr;
> +        }
> +      else
> +        {
> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
> +          if (bit (thumb2_insn_r->arm_insn, 10))
> +            {
> +              if (bit (thumb2_insn_r->arm_insn, 9))
> +                {
> +                  offset_addr = u_regval[0] + offset_imm;
> +                }

Unnecessary braces.

> +              else
> +                {
> +                  offset_addr = u_regval[0] - offset_imm;
> +                }
> +              address = offset_addr;
> +            }
> +          else
> +            {
> +              address = u_regval[0];
> +            }

Unnecessary braces.

> +        }
> +    }
> +
> +  switch (op1)
> +    {
> +      /* Store byte instructions.  */
> +      case 4:
> +      case 0:
> +        record_buf_mem[0] = 1;
> +      break;
> +      /* Store half word instructions.  */
> +      case 1:
> +      case 5:
> +        record_buf_mem[0] = 2;
> +      break;
> +      /* Store word instructions.  */
> +      case 2:
> +      case 6:
> +        record_buf_mem[0] = 4;
> +      break;
> +
> +      default:
> +        gdb_assert_not_reached ("no decoding pattern found");
> +      break;
> +    }
> +
> +  record_buf_mem[1] = address;
> +  thumb2_insn_r->mem_rec_count = 1;
> +  record_buf[0] = reg_rn;
> +  thumb2_insn_r->reg_rec_count = 1;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 load memory hints instructions.  */
> +
> +static int
> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t record_buf[8];
> +  uint32_t reg_rt, reg_rn;
> +
> +  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +
> +  if (15 != reg_rt)

Replace 15 with ARM_PC_REGNUM.

> +    {
> +      record_buf[0] = reg_rt;
> +      record_buf[1] = reg_rn;
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +
> +      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +                record_buf);
> +      return 0;
> +    }
> +
> +  return -1;

Is PC allowed to be used in load memory hints?  If reference manual says 
PC is not allowed, we don't have to worry about "reg_rt == 15".

> +}
> +
> +/* Handler for thumb2 load word instructions.  */
> +
> +static int
> +thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */

Looks this function always return 0.

> +  uint32_t opcode1 = 0, opcode2 = 0;
> +  uint32_t record_buf[8];
> +
> +  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +  record_buf[1] = ARM_PS_REGNUM;
> +  thumb2_insn_r->reg_rec_count = 2;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +
> +  return ret;

"return 0;"

> +}
> +
> +/* Handler for thumb2 long multiply, long multiply accumulate, and
> +   divide instructions.  */
> +
> +static int
> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;  /* Return value: -1:record failure ;  0:success.  */

Move this line of comment to function header comment.

> +  uint32_t opcode1 = 0, opcode2 = 0;
> +  uint32_t record_buf[8];
> +  uint32_t reg_src1 = 0;
> +
> +  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
> +  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
> +
> +  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
> +    {
> +      /* Handle SMULL, UMULL, SMULAL.  */
> +      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else if (1 == opcode1 || 3 == opcode2)
> +    {
> +      /* Handle SDIV and UDIV.  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else
> +    {
> +      ret = -1;
> +    }

Unncessary braces.

> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +
> +  return ret;
> +}
> +

>
>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns
> 0 on success
>   and positive val on fauilure.  */
> @@ -12469,6 +13110,27 @@ decode_insn (insn_decode_record *arm_rec
>       thumb_record_branch                /* 111.  */
>     };
>
> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
> instruction.  */
> +  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
> +  { \
> +    thumb2_record_ld_st_multiple,       /* 00.  */
> +    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
> +    thumb2_record_unsupported_insn,     /* 03.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
> +    thumb2_record_ps_dest_generic,      /* 05.  */
> +    thumb2_record_branch_misc_cntrl,    /* 06.  */
> +    thumb2_record_str_single_data,      /* 07.  */
> +    thumb2_record_unsupported_insn,     /* 08.  */
> +    thumb2_record_ld_mem_hints,         /* 09.  */
> +    thumb2_record_ld_mem_hints,         /* 10.  */
> +    thumb2_record_ld_word,              /* 11.  */
> +    thumb2_record_ps_dest_generic,      /* 12.  */
> +    thumb2_record_ps_dest_generic,      /* 13.  */
> +    thumb2_record_lmul_lmla_div,        /* 14.  */
> +    thumb2_record_unsupported_insn      /* 15.  */
> +  };
> +
>     uint32_t ret = 0;    /* return value: negative:failure   0:success.  */
>     uint32_t insn_id = 0;
>
> @@ -12503,11 +13165,27 @@ decode_insn (insn_decode_record *arm_rec
>       }
>     else if (THUMB2_RECORD == record_type)
>       {
> -      printf_unfiltered (_("Process record doesnt support thumb32 instruction "
> -                           "0x%0x at address %s.\n"),arm_record->arm_insn,
> -                           paddress (arm_record->gdbarch,
> -                           arm_record->this_addr));
> -      ret = -1;
> +      /* As thumb does not have condition codes, we set negative.  */
> +      arm_record->cond = -1;
> +
> +      /* Swap first half of 32bit thumb instruction with second half.  */
> +      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
> +                             (arm_record->arm_insn << 16);
> +
> +      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
> +
> +      if (insn_id >= 0)
> +        {
> +          ret = thumb2_handle_insn[insn_id] (arm_record);
> +        }

Unnessary braces.

> +      else
> +        {
> +          printf_unfiltered (_("Process record doesnt support instruction "
> +                            "0x%0x at address %s.\n"),arm_record->arm_insn,
> +                            paddress (arm_record->gdbarch,
> +                            arm_record->this_addr));
> +          ret = -1;
> +        }

Use thumb2_record_unsupported_insn?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-10-24  2:25 ` Yao Qi
@ 2013-11-08  3:20   ` Omair Javaid
  2013-11-11 10:53     ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2013-11-08  3:20 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Patch Tracking

On Thu 24 Oct 2013 07:24:21 AM PKT, Yao Qi wrote:
> On 10/24/2013 08:09 AM, Omair Javaid wrote:
>> gdb:
>>
>> 2013-10-24  Omair Javaid<omair.javaid@linaro.org>
>>
>> * arm-tdep.c (struct arm_mem_r): Update.
>
> "Update" is too general.  Please describe what is changed.
>
>> (arm_record_coproc_data_proc): Update.
>> (thumb_record_ldm_stm_swi): Update.
>> (thumb2_record_ld_st_multiple): New function.
>> (thumb2_record_ld_st_dual_ex_tbb): New function.
>> (thumb2_record_data_proc_sreg_mimm): New function.
>> (thumb2_record_unsupported_insn): New function.
>> (thumb2_record_ps_dest_generic): New function.
>> (thumb2_record_branch_misc_cntrl): New function.
>> (thumb2_record_str_single_data): New function.
>> (thumb2_record_ld_mem_hints): New function.
>> (thumb2_record_ld_word): New function.
>> (thumb2_record_lmul_lmla_div): New function.
>> (thumb2_record_decode_inst_id): New function.
>> (decode_insn): Update.
>
> Each line of changelog should be prefixed by tab.
>
>> +    {
>> +      if (tdep->arm_syscall_record != NULL)
>> +        {
>> +          ULONGEST svc_operand, svc_number;
>>
>> -  printf_unfiltered (_("Process record does not support instruction "
>> +          svc_operand = (0x00ffffff & arm_insn_r->arm_insn);
>> +
>> +          if (svc_operand)  /* OABI.  */
>> +            {
>> +              svc_number = svc_operand - 0x900000;
>> +            }
>
> Unnecessary braces.
>
>> +          else /* EABI.  */
>> +            {
>> +              regcache_raw_read_unsigned (reg_cache, 7, &svc_number);
>> +            }
>
>
> Unnecessary braces.
>
>> +
>> +          ret = tdep->arm_syscall_record(reg_cache, svc_number);
>> +        }
>> +      else
>> +        {
>> +          printf_unfiltered (_("no syscall record support\n"));
>> +          ret = -1;
>> +        }
>> +    }
>> +  else
>> +    {
>> +      printf_unfiltered (_("Process record does not support
>> instruction "
>>                           "0x%0x at address
>> %s.\n"),arm_insn_r->arm_insn,
>>                           paddress (arm_insn_r->gdbarch,
>> arm_insn_r->this_addr));
>
> I find this statements appear many times, probably, we can move it
> into a function named arm_record_unsupported_insn.
>
>> +      ret = -1;
>> +    }
>>     return ret;
>>   }
>>
>> @@ -12361,9 +12377,10 @@ thumb_record_ldm_stm_swi (insn_decode_re
>>     else if (0x1F == opcode1)
>>       {
>>           /* Handle arm syscall insn.  */
>> -        if (tdep->arm_swi_record != NULL)
>> +        if (tdep->arm_syscall_record != NULL)
>>             {
>> -            ret = tdep->arm_swi_record(reg_cache);
>> +            regcache_raw_read_unsigned (reg_cache, 7, &u_regval);
>> +            ret = tdep->arm_syscall_record(reg_cache, u_regval);
>
> Space is needed before "(".  This change belongs to support recording
> syscall instructions, and IWBN to move this part to patch 2/2.
>
>>             }
>>           else
>>             {
>> @@ -12414,6 +12431,630 @@ thumb_record_branch (insn_decode_record
>>     return 0;
>>   }
>>
>> +/* Handler for thumb2 load/store multiple instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
>> +  uint32_t reg_rn, op;
>> +  uint32_t register_bits = 0, register_count = 0;
>> +  uint32_t index = 0, start_address = 0;
>> +  uint32_t record_buf[24], record_buf_mem[48];
>> +
>> +  ULONGEST u_regval = 0;
>> +
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +  op = bits (thumb2_insn_r->arm_insn, 23, 24);
>> +
>> +  if (0 == op || 3 == op)
>> +    {
>> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
>> +        {
>> +          /* Handle RFE instruction.  */
>> +          record_buf[0] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +      else
>> +        {
>> +          /* Handle SRS instruction after reading banked SP.  */
>> +          printf_unfiltered (_("Process record does not support
>> instruction "
>> +                            "0x%0x at address %s.\n"),
>> +                            thumb2_insn_r->arm_insn,
>> +                            paddress (thumb2_insn_r->gdbarch,
>> +                            thumb2_insn_r->this_addr));
>
> Use thumb2_record_unsupported_insn?
>
>> +          return -1;
>> +        }
>> +    }
>> +  else if(1 == op || 2 == op)
>> +    {
>> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
>> +        {
>> +          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
>> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
>> +          while (register_bits)
>> +            {
>> +              if (register_bits & 0x00000001)
>> +                {
>> +                  record_buf[index++] = register_count;
>> +                }
>
> Unnecessary braces.
>
>> +              register_count++;
>> +              register_bits = register_bits >> 1;
>> +            }
>> +          record_buf[index++] = reg_rn;
>> +          record_buf[index++] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = index;
>> +        }
>> +      else
>> +        {
>> +          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
>> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
>> +          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
>> +          while (register_bits)
>> +            {
>> +              if (register_bits & 0x00000001)
>> +                {
>> +                  register_count++;
>> +                }
>
> Unnecessary braces.
>
>> +              register_bits = register_bits >> 1;
>> +            }
>> +
>> +          if (1 == op)
>> +            {
>> +              /* Start address calculation for LDMDB/LDMEA.  */
>> +              start_address = u_regval;
>> +            }
>> +          else if (2 == op)
>> +            {
>> +              /* Start address calculation for LDMDB/LDMEA.  */
>> +              start_address = (u_regval) - (register_count * 4);
>> +            }
>> +
>> +          thumb2_insn_r->mem_rec_count = register_count;
>> +          while (register_count)
>> +            {
>> +              record_buf_mem[(register_count * 2) - 1] = start_address;
>> +              record_buf_mem[(register_count * 2) - 2] = 4;
>> +              start_address = start_address + 4;
>> +              register_count--;
>> +            }
>> +          record_buf[0] = reg_rn;
>> +          record_buf[1] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 2;
>> +        }
>> +    }
>> +
>> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
>> +            record_buf_mem);
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
>> instructions.  */
>
> Looks your mailer wraps the patch.  See
> https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches
> on how to adjust your mailer.
>
>> +
>> +/* Handler for thumb2 data processing (shift register and modified
>> immediate)
>> +   instructions.  */
>> +
>> +static int
>> +thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t ret = 0;  /* Return value: -1:record failure ;
>> 0:success.  */
>
> This function always return 0.
>
>> +  uint32_t reg_rd, op;
>> +  uint32_t record_buf[8];
>> +
>> +  op = bits (thumb2_insn_r->arm_insn, 21, 24);
>> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +
>> +  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
>> +    {
>> +      record_buf[0] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 1;
>> +    }
>> +  else
>> +    {
>> +      record_buf[0] = reg_rd;
>> +      record_buf[1] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 2;
>> +    }
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +
>> +  return ret;
>> +}
>> +
>
>> +
>> +/* Generic handler for thumb2 instructions which need to save PS and
>> +   destination registers is saved. Handles multiply, multiply
>> accumulate,
>
> Two spaces after period.
>
>> +   and absolute difference instructions. Also handles data-processing
>
> and here.
>
>> +   (register and plain binary immediate) instructions.  */
>> +
>> +static int
>> +thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t ret = 0;  /* Return value: -1:record failure ;
>> 0:success.  */
>
> Document the meaning of return value in function header comment and
> remove this line of comment.
>
>> +
>> +/* Handler for thumb2 branch and miscellaneous control
>> instructions.  */
>> +
>> +static int
>> +thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t op, op1, op2;
>> +  uint32_t record_buf[8];
>> +
>> +  op = bits (thumb2_insn_r->arm_insn, 20, 26);
>> +  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
>> +  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +
>> +  /* Handle MSR insn.  */
>> +  if (!(op1 & 0x2) && 0x38 == op)
>> +    {
>> +      if (!(op2 & 0x3))
>> +        {
>> +          /* CPSR is going to be changed.  */
>> +          record_buf[0] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +      else
>> +        {
>> +          /* SPSR is going to be changed.  */
>> +          printf_unfiltered (_("Process record does not support
>> instruction "
>> +                            "0x%0x at address %s.\n"),
>> +                            thumb2_insn_r->arm_insn,
>> +                            paddress (thumb2_insn_r->gdbarch,
>> +                            thumb2_insn_r->this_addr));
>
> Use thumb2_record_unsupported_insn?
>
>> +          return -1;
>> +        }
>> +    }
>> +  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
>> +    {
>> +      /* BLX.  */
>> +      record_buf[0] = ARM_PS_REGNUM;
>> +      record_buf[1] = ARM_LR_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 2;
>> +    }
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 store single data item instructions.  */
>> +
>> +static int
>> +thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
>> +  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
>> +  uint32_t address, offset_addr;
>> +  uint32_t record_buf[8], record_buf_mem[8];
>> +  uint32_t op1, op2;
>> +
>> +  ULONGEST u_regval[2];
>> +
>> +  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
>> +  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
>> +
>> +  if (bit (thumb2_insn_r->arm_insn, 23))
>> +    {
>> +      /* T2 encoding.  */
>> +      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
>> +      offset_addr = u_regval[0] + offset_imm;
>> +      address = offset_addr;
>> +    }
>> +  else
>> +    {
>> +      /* T3 encoding.  */
>> +      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
>> +        {
>> +          /* Handle STRB (register).  */
>> +          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
>> +          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
>> +          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
>> +          offset_addr = u_regval[1] << shift_imm;
>> +          address = u_regval[0] + offset_addr;
>> +        }
>> +      else
>> +        {
>> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
>> +          if (bit (thumb2_insn_r->arm_insn, 10))
>> +            {
>> +              if (bit (thumb2_insn_r->arm_insn, 9))
>> +                {
>> +                  offset_addr = u_regval[0] + offset_imm;
>> +                }
>
> Unnecessary braces.
>
>> +              else
>> +                {
>> +                  offset_addr = u_regval[0] - offset_imm;
>> +                }
>> +              address = offset_addr;
>> +            }
>> +          else
>> +            {
>> +              address = u_regval[0];
>> +            }
>
> Unnecessary braces.
>
>> +        }
>> +    }
>> +
>> +  switch (op1)
>> +    {
>> +      /* Store byte instructions.  */
>> +      case 4:
>> +      case 0:
>> +        record_buf_mem[0] = 1;
>> +      break;
>> +      /* Store half word instructions.  */
>> +      case 1:
>> +      case 5:
>> +        record_buf_mem[0] = 2;
>> +      break;
>> +      /* Store word instructions.  */
>> +      case 2:
>> +      case 6:
>> +        record_buf_mem[0] = 4;
>> +      break;
>> +
>> +      default:
>> +        gdb_assert_not_reached ("no decoding pattern found");
>> +      break;
>> +    }
>> +
>> +  record_buf_mem[1] = address;
>> +  thumb2_insn_r->mem_rec_count = 1;
>> +  record_buf[0] = reg_rn;
>> +  thumb2_insn_r->reg_rec_count = 1;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
>> +            record_buf_mem);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 load memory hints instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t record_buf[8];
>> +  uint32_t reg_rt, reg_rn;
>> +
>> +  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +
>> +  if (15 != reg_rt)
>
> Replace 15 with ARM_PC_REGNUM.
>
>> +    {
>> +      record_buf[0] = reg_rt;
>> +      record_buf[1] = reg_rn;
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +
>> +      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +                record_buf);
>> +      return 0;
>> +    }
>> +
>> +  return -1;
>
> Is PC allowed to be used in load memory hints?  If reference manual
> says PC is not allowed, we don't have to worry about "reg_rt == 15".
>
>> +}
>> +
>> +/* Handler for thumb2 load word instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t ret = 0;  /* Return value: -1:record failure ;
>> 0:success.  */
>
> Looks this function always return 0.
>
>> +  uint32_t opcode1 = 0, opcode2 = 0;
>> +  uint32_t record_buf[8];
>> +
>> +  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +  record_buf[1] = ARM_PS_REGNUM;
>> +  thumb2_insn_r->reg_rec_count = 2;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +
>> +  return ret;
>
> "return 0;"
>
>> +}
>> +
>> +/* Handler for thumb2 long multiply, long multiply accumulate, and
>> +   divide instructions.  */
>> +
>> +static int
>> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t ret = 0;  /* Return value: -1:record failure ;
>> 0:success.  */
>
> Move this line of comment to function header comment.
>
>> +  uint32_t opcode1 = 0, opcode2 = 0;
>> +  uint32_t record_buf[8];
>> +  uint32_t reg_src1 = 0;
>> +
>> +  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
>> +  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
>> +
>> +  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
>> +    {
>> +      /* Handle SMULL, UMULL, SMULAL.  */
>> +      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
>> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +    }
>> +  else if (1 == opcode1 || 3 == opcode2)
>> +    {
>> +      /* Handle SDIV and UDIV.  */
>> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +    }
>> +  else
>> +    {
>> +      ret = -1;
>> +    }
>
> Unncessary braces.
>
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +
>> +  return ret;
>> +}
>> +
>
>>
>>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns
>> 0 on success
>>   and positive val on fauilure.  */
>> @@ -12469,6 +13110,27 @@ decode_insn (insn_decode_record *arm_rec
>>       thumb_record_branch                /* 111.  */
>>     };
>>
>> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
>> instruction.  */
>> +  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
>> +  { \
>> +    thumb2_record_ld_st_multiple,       /* 00.  */
>> +    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
>> +    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
>> +    thumb2_record_unsupported_insn,     /* 03.  */
>> +    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
>> +    thumb2_record_ps_dest_generic,      /* 05.  */
>> +    thumb2_record_branch_misc_cntrl,    /* 06.  */
>> +    thumb2_record_str_single_data,      /* 07.  */
>> +    thumb2_record_unsupported_insn,     /* 08.  */
>> +    thumb2_record_ld_mem_hints,         /* 09.  */
>> +    thumb2_record_ld_mem_hints,         /* 10.  */
>> +    thumb2_record_ld_word,              /* 11.  */
>> +    thumb2_record_ps_dest_generic,      /* 12.  */
>> +    thumb2_record_ps_dest_generic,      /* 13.  */
>> +    thumb2_record_lmul_lmla_div,        /* 14.  */
>> +    thumb2_record_unsupported_insn      /* 15.  */
>> +  };
>> +
>>     uint32_t ret = 0;    /* return value: negative:failure
>> 0:success.  */
>>     uint32_t insn_id = 0;
>>
>> @@ -12503,11 +13165,27 @@ decode_insn (insn_decode_record *arm_rec
>>       }
>>     else if (THUMB2_RECORD == record_type)
>>       {
>> -      printf_unfiltered (_("Process record doesnt support thumb32
>> instruction "
>> -                           "0x%0x at address
>> %s.\n"),arm_record->arm_insn,
>> -                           paddress (arm_record->gdbarch,
>> -                           arm_record->this_addr));
>> -      ret = -1;
>> +      /* As thumb does not have condition codes, we set negative.  */
>> +      arm_record->cond = -1;
>> +
>> +      /* Swap first half of 32bit thumb instruction with second
>> half.  */
>> +      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
>> +                             (arm_record->arm_insn << 16);
>> +
>> +      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
>> +
>> +      if (insn_id >= 0)
>> +        {
>> +          ret = thumb2_handle_insn[insn_id] (arm_record);
>> +        }
>
> Unnessary braces.
>
>> +      else
>> +        {
>> +          printf_unfiltered (_("Process record doesnt support
>> instruction "
>> +                            "0x%0x at address
>> %s.\n"),arm_record->arm_insn,
>> +                            paddress (arm_record->gdbarch,
>> +                            arm_record->this_addr));
>> +          ret = -1;
>> +        }
>
> Use thumb2_record_unsupported_insn?
>

Here's is an updated patch.
This patch adds support for recording thumb32 instructions. This patch 
also
fixes bugs in arm process record instruction decoding.

gdb:

2013-11-08  Omair Javaid  <omair.javaid@linaro.org>

	* arm-tdep.c (struct arm_mem_r): Use uint32_t for memory address.
	(arm_record_unsupported_insn): New function.
	(arm_record_coproc_data_proc): Removed.
	(thumb2_record_ld_st_multiple): New function.
	(thumb2_record_ld_st_dual_ex_tbb): New function.
	(thumb2_record_data_proc_sreg_mimm): New function.
	(thumb2_record_ps_dest_generic): New function.
	(thumb2_record_branch_misc_cntrl): New function.
	(thumb2_record_str_single_data): New function.
	(thumb2_record_ld_mem_hints): New function.
	(thumb2_record_ld_word): New function.
	(thumb2_record_lmul_lmla_div): New function.
	(thumb2_record_decode_inst_id): New function.
	(decode_insn): Add thumb32 instruction handlers.

Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.381
diff -u -p -r1.381 arm-tdep.c
--- gdb/arm-tdep.c	24 Jun 2013 22:18:31 -0000	1.381
+++ gdb/arm-tdep.c	7 Nov 2013 23:52:43 -0000
@@ -10618,7 +10618,7 @@ vfp - VFP co-processor."),
 struct arm_mem_r
 {
   uint32_t len;    /* Record length.  */
-  CORE_ADDR addr;  /* Memory address.  */
+  uint32_t addr;   /* Memory address.  */
 };

 /* ARM instruction record contains opcode of current insn
@@ -11919,7 +11919,7 @@ arm_record_b_bl (insn_decode_record *arm
 /* Handling opcode 110 insns.  */

 static int
-arm_record_coproc (insn_decode_record *arm_insn_r)
+arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
 {
   printf_unfiltered (_("Process record does not support instruction "
                     "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
@@ -12414,6 +12414,588 @@ thumb_record_branch (insn_decode_record
   return 0;
 }

+/* Handler for thumb2 load/store multiple instructions.  */
+
+static int
+thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, op;
+  uint32_t register_bits = 0, register_count = 0;
+  uint32_t index = 0, start_address = 0;
+  uint32_t record_buf[24], record_buf_mem[48];
+
+  ULONGEST u_regval = 0;
+
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  op = bits (thumb2_insn_r->arm_insn, 23, 24);
+
+  if (0 == op || 3 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle RFE instruction.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          /* Handle SRS instruction after reading banked SP.  */
+          return arm_record_unsupported_insn (thumb2_insn_r);
+        }
+    }
+  else if(1 == op || 2 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                record_buf[index++] = register_count;
+
+              register_count++;
+              register_bits = register_bits >> 1;
+            }
+          record_buf[index++] = reg_rn;
+          record_buf[index++] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = index;
+        }
+      else
+        {
+          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                register_count++;
+
+              register_bits = register_bits >> 1;
+            }
+
+          if (1 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = u_regval;
+            }
+          else if (2 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = (u_regval) - (register_count * 4);
+            }
+
+          thumb2_insn_r->mem_rec_count = register_count;
+          while (register_count)
+            {
+              record_buf_mem[(register_count * 2) - 1] = start_address;
+              record_buf_mem[(register_count * 2) - 2] = 4;
+              start_address = start_address + 4;
+              register_count--;
+            }
+          record_buf[0] = reg_rn;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+    }
+
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch 
instructions.  */
+
+static int
+thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rd, reg_rn, offset_imm;
+  uint32_t reg_dest1, reg_dest2;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2, op3;
+  LONGEST s_word;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 23, 24);
+  op2 = bits (thumb2_insn_r->arm_insn, 20, 21);
+  op3 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+    {
+      if(!(1 == op1 && 1 == op2 && (0 == op3 || 1 == op3)))
+        {
+          reg_dest1 = bits (thumb2_insn_r->arm_insn, 12, 15);
+          record_buf[0] = reg_dest1;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+
+      if (3 == op2 || (op1 & 2) || (1 == op1 && 1 == op2 && 7 == op3))
+        {
+          reg_dest2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+          record_buf[2] = reg_dest2;
+          thumb2_insn_r->reg_rec_count = 3;
+        }
+    }
+  else
+    {
+      reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+      regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+      if (0 == op1 && 0 == op2)
+        {
+          /* Handle STREX.  */
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          address = u_regval[0] + (offset_imm * 4);
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          thumb2_insn_r->mem_rec_count = 1;
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else if (1 == op1 && 0 == op2)
+        {
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+          address = u_regval[0];
+          record_buf_mem[1] = address;
+
+          if (4 == op3)
+            {
+              /* Handle STREXB.  */
+              record_buf_mem[0] = 1;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (5 == op3)
+            {
+              /* Handle STREXH.  */
+              record_buf_mem[0] = 2 ;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (7 == op3)
+            {
+              /* Handle STREXD.  */
+              address = u_regval[0];
+              record_buf_mem[0] = 4;
+              record_buf_mem[2] = 4;
+              record_buf_mem[3] = address + 4;
+              thumb2_insn_r->mem_rec_count = 2;
+            }
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+
+          if (bit (thumb2_insn_r->arm_insn, 24))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 23))
+                {
+                  offset_addr = u_regval[0] + (offset_imm * 4);
+                }
+              else
+                {
+                  offset_addr = u_regval[0] - (offset_imm * 4);
+                }
+              address = offset_addr;
+            }
+          else
+            {
+              address = u_regval[0];
+            }
+
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          record_buf_mem[2] = 4;
+          record_buf_mem[3] = address + 4;
+          thumb2_insn_r->mem_rec_count = 2;
+          record_buf[0] = reg_rn;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 data processing (shift register and modified 
immediate)
+   instructions.  */
+
+static int
+thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t reg_rd, op;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 21, 24);
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
+    {
+      record_buf[0] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 1;
+    }
+  else
+    {
+      record_buf[0] = reg_rd;
+      record_buf[1] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Generic handler for thumb2 instructions which effect destination 
and PS
+   registers.  */
+
+static int
+thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t reg_rd;
+  uint32_t record_buf[8];
+
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  record_buf[0] = reg_rd;
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 branch and miscellaneous control instructions.  
*/
+
+static int
+thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t op, op1, op2;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 20, 26);
+  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
+  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  /* Handle MSR insn.  */
+  if (!(op1 & 0x2) && 0x38 == op)
+    {
+      if (!(op2 & 0x3))
+        {
+          /* CPSR is going to be changed.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          arm_record_unsupported_insn(thumb2_insn_r);
+          return -1;
+        }
+    }
+  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
+    {
+      /* BLX.  */
+      record_buf[0] = ARM_PS_REGNUM;
+      record_buf[1] = ARM_LR_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 store single data item instructions.  */
+
+static int
+thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
+  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+  if (bit (thumb2_insn_r->arm_insn, 23))
+    {
+      /* T2 encoding.  */
+      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
+      offset_addr = u_regval[0] + offset_imm;
+      address = offset_addr;
+    }
+  else
+    {
+      /* T3 encoding.  */
+      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
+        {
+          /* Handle STRB (register).  */
+          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
+          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
+          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
+          offset_addr = u_regval[1] << shift_imm;
+          address = u_regval[0] + offset_addr;
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          if (bit (thumb2_insn_r->arm_insn, 10))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 9))
+                offset_addr = u_regval[0] + offset_imm;
+              else
+                offset_addr = u_regval[0] - offset_imm;
+
+              address = offset_addr;
+            }
+          else
+            address = u_regval[0];
+        }
+    }
+
+  switch (op1)
+    {
+      /* Store byte instructions.  */
+      case 4:
+      case 0:
+        record_buf_mem[0] = 1;
+      break;
+      /* Store half word instructions.  */
+      case 1:
+      case 5:
+        record_buf_mem[0] = 2;
+      break;
+      /* Store word instructions.  */
+      case 2:
+      case 6:
+        record_buf_mem[0] = 4;
+      break;
+
+      default:
+        gdb_assert_not_reached ("no decoding pattern found");
+      break;
+    }
+
+  record_buf_mem[1] = address;
+  thumb2_insn_r->mem_rec_count = 1;
+  record_buf[0] = reg_rn;
+  thumb2_insn_r->reg_rec_count = 1;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 load memory hints instructions.  */
+
+static int
+thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t record_buf[8];
+  uint32_t reg_rt, reg_rn;
+
+  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+
+  if (ARM_PC_REGNUM != reg_rt)
+    {
+      record_buf[0] = reg_rt;
+      record_buf[1] = reg_rn;
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+
+      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+                record_buf);
+      return 0;
+    }
+
+  return -1;
+}
+
+/* Handler for thumb2 load word instructions.  */
+
+static int
+thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+
+  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 long multiply, long multiply accumulate, and
+   divide instructions.  Return value: -1:record failure ;  0:success. 
 */
+
+static int
+thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+  uint32_t reg_src1 = 0;
+
+  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
+  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
+    {
+      /* Handle SMULL, UMULL, SMULAL.  */
+      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else if (1 == opcode1 || 3 == opcode2)
+    {
+      /* Handle SDIV and UDIV.  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else
+    ret = -1;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return ret;
+}
+
+/* Decodes thumb2 instruction type and return an instruction id.  */
+
+static unsigned int
+thumb2_record_decode_inst_id (uint32_t thumb2_insn)
+{
+  uint32_t op = 0;
+  uint32_t op1 = 0;
+  uint32_t op2 = 0;
+
+  op = bit (thumb2_insn, 15);
+  op1 = bits (thumb2_insn, 27, 28);
+  op2 = bits (thumb2_insn, 20, 26);
+
+  if (op1 == 0x01)
+    {
+      if (!(op2 & 0x64 ))
+        {
+          /* Load/store multiple instruction.  */
+          return 0;
+        }
+      else if (!((op2 & 0x64) ^ 0x04))
+        {
+          /* Load/store dual, load/store exclusive, table branch 
instruction.  */
+          return 1;
+        }
+      else if (!((op2 & 0x20) ^ 0x20))
+        {
+          /* Data-processing (shifted register).  */
+          return 2;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 3;
+        }
+    }
+  else if (op1 == 0x02)
+    {
+      if (op)
+        {
+          /* Branches and miscellaneous control instructions.  */
+          return 6;
+        }
+      else if (op2 & 0x20)
+        {
+          /* Data-processing (plain binary immediate) instruction.  */
+          return 5;
+        }
+      else
+        {
+          /* Data-processing (modified immediate).  */
+          return 4;
+        }
+    }
+  else if (op1 == 0x03)
+   {
+      if (!(op2 & 0x71 ))
+        {
+          /* Store single data item.  */
+          return 7;
+        }
+      else if (!((op2 & 0x71) ^ 0x10))
+        {
+          /* Advanced SIMD element or structure load/store 
instructions.  */
+          return 8;
+        }
+      else if (!((op2 & 0x67) ^ 0x01))
+        {
+          /* Load byte, memory hints instruction.  */
+          return 9;
+        }
+      else if (!((op2 & 0x67) ^ 0x03))
+        {
+          /* Load halfword, memory hints instruction.  */
+          return 10;
+        }
+      else if (!((op2 & 0x67) ^ 0x05))
+        {
+          /* Load word instruction.  */
+          return 11;
+        }
+      else if (!((op2 & 0x70) ^ 0x20))
+        {
+          /* Data-processing (register) instruction.  */
+          return 12;
+        }
+      else if (!((op2 & 0x78) ^ 0x30))
+        {
+          /* Multiply, multiply accumulate, absolute difference 
instruction.  */
+          return 13;
+        }
+      else if (!((op2 & 0x78) ^ 0x38))
+        {
+          /* Long multiply, long multiply accumulate, and divide.  */
+          return 14;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 15;
+        }
+   }
+
+  return -1;
+}

 /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0 
on success
 and positive val on fauilure.  */
@@ -12452,7 +13034,7 @@ decode_insn (insn_decode_record *arm_rec
     arm_record_ld_st_reg_offset,        /* 011.  */
     arm_record_ld_st_multiple,          /* 100.  */
     arm_record_b_bl,                    /* 101.  */
-    arm_record_coproc,                  /* 110.  */
+    arm_record_unsupported_insn,                  /* 110.  */
     arm_record_coproc_data_proc         /* 111.  */
   };

@@ -12469,6 +13051,27 @@ decode_insn (insn_decode_record *arm_rec
     thumb_record_branch                /* 111.  */
   };

+  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb 
instruction.  */
+  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
+  { \
+    thumb2_record_ld_st_multiple,       /* 00.  */
+    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
+    arm_record_unsupported_insn,        /* 03.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
+    thumb2_record_ps_dest_generic,      /* 05.  */
+    thumb2_record_branch_misc_cntrl,    /* 06.  */
+    thumb2_record_str_single_data,      /* 07.  */
+    arm_record_unsupported_insn,        /* 08.  */
+    thumb2_record_ld_mem_hints,         /* 09.  */
+    thumb2_record_ld_mem_hints,         /* 10.  */
+    thumb2_record_ld_word,              /* 11.  */
+    thumb2_record_ps_dest_generic,      /* 12.  */
+    thumb2_record_ps_dest_generic,      /* 13.  */
+    thumb2_record_lmul_lmla_div,        /* 14.  */
+    arm_record_unsupported_insn         /* 15.  */
+  };
+
   uint32_t ret = 0;    /* return value: negative:failure   0:success.  
*/
   uint32_t insn_id = 0;

@@ -12503,11 +13106,22 @@ decode_insn (insn_decode_record *arm_rec
     }
   else if (THUMB2_RECORD == record_type)
     {
-      printf_unfiltered (_("Process record doesnt support thumb32 
instruction "
-                           "0x%0x at address 
%s.\n"),arm_record->arm_insn,
-                           paddress (arm_record->gdbarch,
-                           arm_record->this_addr));
-      ret = -1;
+      /* As thumb does not have condition codes, we set negative.  */
+      arm_record->cond = -1;
+
+      /* Swap first half of 32bit thumb instruction with second half.  
*/
+      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
+                             (arm_record->arm_insn << 16);
+
+      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
+
+      if (insn_id >= 0)
+        ret = thumb2_handle_insn[insn_id] (arm_record);
+      else
+        {
+          arm_record_unsupported_insn(arm_record);
+          ret = -1;
+        }
     }
   else
     {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-08  3:20   ` Omair Javaid
@ 2013-11-11 10:53     ` Yao Qi
  2013-11-25  0:05       ` Omair Javaid
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2013-11-11 10:53 UTC (permalink / raw)
  To: Omair Javaid; +Cc: gdb-patches, Patch Tracking

Patch looks good to me, and you still need a maintainer's approval.

Some of the comments are too long.  Please make sure they don't exceed 
the limitation (around 74 characters should be fine).

On 11/08/2013 11:19 AM, Omair Javaid wrote:
> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
> instructions.  */
> +

here.

> +static int
> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +

....

> +
> +/* Decodes thumb2 instruction type and return an instruction id.  */
> +
> +static unsigned int
> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
> +{
> +  uint32_t op = 0;
> +  uint32_t op1 = 0;
> +  uint32_t op2 = 0;
> +
> +  op = bit (thumb2_insn, 15);
> +  op1 = bits (thumb2_insn, 27, 28);
> +  op2 = bits (thumb2_insn, 20, 26);
> +
> +  if (op1 == 0x01)
> +    {
> +      if (!(op2 & 0x64 ))
> +        {
> +          /* Load/store multiple instruction.  */
> +          return 0;
> +        }
> +      else if (!((op2 & 0x64) ^ 0x04))
> +        {
> +          /* Load/store dual, load/store exclusive, table branch
> instruction.  */

and here.


> +          return 1;
> +        }
> +      else if (!((op2 & 0x20) ^ 0x20))
> +        {
> +          /* Data-processing (shifted register).  */
> +          return 2;
> +        }
> +      else if (op2 & 0x40)
> +        {
> +          /* Co-processor instructions.  */
> +          return 3;
> +        }
> +    }
> +  else if (op1 == 0x02)
> +    {
> +      if (op)
> +        {
> +          /* Branches and miscellaneous control instructions.  */
> +          return 6;
> +        }
> +      else if (op2 & 0x20)
> +        {
> +          /* Data-processing (plain binary immediate) instruction.  */
> +          return 5;
> +        }
> +      else
> +        {
> +          /* Data-processing (modified immediate).  */
> +          return 4;
> +        }
> +    }
> +  else if (op1 == 0x03)
> +   {
> +      if (!(op2 & 0x71 ))
> +        {
> +          /* Store single data item.  */
> +          return 7;
> +        }
> +      else if (!((op2 & 0x71) ^ 0x10))
> +        {
> +          /* Advanced SIMD element or structure load/store
> instructions.  */

here.

> +          return 8;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x01))
> +        {
> +          /* Load byte, memory hints instruction.  */
> +          return 9;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x03))
> +        {
> +          /* Load halfword, memory hints instruction.  */
> +          return 10;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x05))
> +        {
> +          /* Load word instruction.  */
> +          return 11;
> +        }
> +      else if (!((op2 & 0x70) ^ 0x20))
> +        {
> +          /* Data-processing (register) instruction.  */
> +          return 12;
> +        }
> +      else if (!((op2 & 0x78) ^ 0x30))
> +        {
> +          /* Multiply, multiply accumulate, absolute difference
> instruction.  */

here.

> +          return 13;
> +        }
> +      else if (!((op2 & 0x78) ^ 0x38))
> +        {
> +          /* Long multiply, long multiply accumulate, and divide.  */
> +          return 14;
> +        }
> +      else if (op2 & 0x40)
> +        {
> +          /* Co-processor instructions.  */
> +          return 15;
> +        }
> +   }
> +
> +  return -1;
> +}
>
>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0
> on success
>   and positive val on fauilure.  */
> @@ -12452,7 +13034,7 @@ decode_insn (insn_decode_record *arm_rec
>       arm_record_ld_st_reg_offset,        /* 011.  */
>       arm_record_ld_st_multiple,          /* 100.  */
>       arm_record_b_bl,                    /* 101.  */
> -    arm_record_coproc,                  /* 110.  */
> +    arm_record_unsupported_insn,                  /* 110.  */
>       arm_record_coproc_data_proc         /* 111.  */
>     };
>
> @@ -12469,6 +13051,27 @@ decode_insn (insn_decode_record *arm_rec
>       thumb_record_branch                /* 111.  */
>     };
>
> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
> instruction.  */

here.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-11 10:53     ` Yao Qi
@ 2013-11-25  0:05       ` Omair Javaid
  2013-11-25 14:23         ` Oza Pawandeep
  2013-12-20 12:37         ` Pedro Alves
  0 siblings, 2 replies; 10+ messages in thread
From: Omair Javaid @ 2013-11-25  0:05 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Patch Tracking

On 11/11/2013 03:09 PM, Yao Qi wrote:
> Patch looks good to me, and you still need a maintainer's approval.
> 
> Some of the comments are too long.  Please make sure they don't exceed 
> the limitation (around 74 characters should be fine).
> 
> On 11/08/2013 11:19 AM, Omair Javaid wrote:
>> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
>> instructions.  */
>> +
> 
> here.
> 
>> +static int
>> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
> 
> ....
> 
>> +
>> +/* Decodes thumb2 instruction type and return an instruction id.  */
>> +
>> +static unsigned int
>> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
>> +{
>> +  uint32_t op = 0;
>> +  uint32_t op1 = 0;
>> +  uint32_t op2 = 0;
>> +
>> +  op = bit (thumb2_insn, 15);
>> +  op1 = bits (thumb2_insn, 27, 28);
>> +  op2 = bits (thumb2_insn, 20, 26);
>> +
>> +  if (op1 == 0x01)
>> +    {
>> +      if (!(op2 & 0x64 ))
>> +        {
>> +          /* Load/store multiple instruction.  */
>> +          return 0;
>> +        }
>> +      else if (!((op2 & 0x64) ^ 0x04))
>> +        {
>> +          /* Load/store dual, load/store exclusive, table branch
>> instruction.  */
> 
> and here.
> 
> 
>> +          return 1;
>> +        }
>> +      else if (!((op2 & 0x20) ^ 0x20))
>> +        {
>> +          /* Data-processing (shifted register).  */
>> +          return 2;
>> +        }
>> +      else if (op2 & 0x40)
>> +        {
>> +          /* Co-processor instructions.  */
>> +          return 3;
>> +        }
>> +    }
>> +  else if (op1 == 0x02)
>> +    {
>> +      if (op)
>> +        {
>> +          /* Branches and miscellaneous control instructions.  */
>> +          return 6;
>> +        }
>> +      else if (op2 & 0x20)
>> +        {
>> +          /* Data-processing (plain binary immediate) instruction.  */
>> +          return 5;
>> +        }
>> +      else
>> +        {
>> +          /* Data-processing (modified immediate).  */
>> +          return 4;
>> +        }
>> +    }
>> +  else if (op1 == 0x03)
>> +   {
>> +      if (!(op2 & 0x71 ))
>> +        {
>> +          /* Store single data item.  */
>> +          return 7;
>> +        }
>> +      else if (!((op2 & 0x71) ^ 0x10))
>> +        {
>> +          /* Advanced SIMD element or structure load/store
>> instructions.  */
> 
> here.
> 
>> +          return 8;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x01))
>> +        {
>> +          /* Load byte, memory hints instruction.  */
>> +          return 9;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x03))
>> +        {
>> +          /* Load halfword, memory hints instruction.  */
>> +          return 10;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x05))
>> +        {
>> +          /* Load word instruction.  */
>> +          return 11;
>> +        }
>> +      else if (!((op2 & 0x70) ^ 0x20))
>> +        {
>> +          /* Data-processing (register) instruction.  */
>> +          return 12;
>> +        }
>> +      else if (!((op2 & 0x78) ^ 0x30))
>> +        {
>> +          /* Multiply, multiply accumulate, absolute difference
>> instruction.  */
> 
> here.
> 
>> +          return 13;
>> +        }
>> +      else if (!((op2 & 0x78) ^ 0x38))
>> +        {
>> +          /* Long multiply, long multiply accumulate, and divide.  */
>> +          return 14;
>> +        }
>> +      else if (op2 & 0x40)
>> +        {
>> +          /* Co-processor instructions.  */
>> +          return 15;
>> +        }
>> +   }
>> +
>> +  return -1;
>> +}
>>
>>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0
>> on success
>>   and positive val on fauilure.  */
>> @@ -12452,7 +13034,7 @@ decode_insn (insn_decode_record *arm_rec
>>       arm_record_ld_st_reg_offset,        /* 011.  */
>>       arm_record_ld_st_multiple,          /* 100.  */
>>       arm_record_b_bl,                    /* 101.  */
>> -    arm_record_coproc,                  /* 110.  */
>> +    arm_record_unsupported_insn,                  /* 110.  */
>>       arm_record_coproc_data_proc         /* 111.  */
>>     };
>>
>> @@ -12469,6 +13051,27 @@ decode_insn (insn_decode_record *arm_rec
>>       thumb_record_branch                /* 111.  */
>>     };
>>
>> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
>> instruction.  */
> 
> here.
> 

Patch has been updated after incorporating suggestions. Looking for a final
commit verdict.

This patch adds support for recording thumb32 instructions. This patch also
fixes bugs in arm process record instruction decoding.

gdb:

2013-11-08  Omair Javaid  <omair.javaid@linaro.org>

	* arm-tdep.c (struct arm_mem_r): Use uint32_t for memory address.
	(arm_record_unsupported_insn): New function.
	(arm_record_coproc_data_proc): Removed.
	(thumb2_record_ld_st_multiple): New function.
	(thumb2_record_ld_st_dual_ex_tbb): New function.
	(thumb2_record_data_proc_sreg_mimm): New function.
	(thumb2_record_ps_dest_generic): New function.
	(thumb2_record_branch_misc_cntrl): New function.
	(thumb2_record_str_single_data): New function.
	(thumb2_record_ld_mem_hints): New function.
	(thumb2_record_ld_word): New function.
	(thumb2_record_lmul_lmla_div): New function.
	(thumb2_record_decode_inst_id): New function.
	(decode_insn): Add thumb32 instruction handlers.

---
 gdb/arm-tdep.c |  627 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 619 insertions(+), 8 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 7c78a61..ecaced7 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10618,7 +10618,7 @@ vfp - VFP co-processor."),
 struct arm_mem_r
 {
   uint32_t len;    /* Record length.  */
-  CORE_ADDR addr;  /* Memory address.  */
+  uint32_t addr;   /* Memory address.  */
 };
 
 /* ARM instruction record contains opcode of current insn
@@ -11919,7 +11919,7 @@ arm_record_b_bl (insn_decode_record *arm_insn_r)
 /* Handling opcode 110 insns.  */
 
 static int
-arm_record_coproc (insn_decode_record *arm_insn_r)
+arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
 {
   printf_unfiltered (_("Process record does not support instruction "
                     "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
@@ -12414,6 +12414,584 @@ thumb_record_branch (insn_decode_record *thumb_insn_r)
   return 0;     
 }
 
+/* Handler for thumb2 load/store multiple instructions.  */
+
+static int
+thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, op;
+  uint32_t register_bits = 0, register_count = 0;
+  uint32_t index = 0, start_address = 0;
+  uint32_t record_buf[24], record_buf_mem[48];
+
+  ULONGEST u_regval = 0;
+
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  op = bits (thumb2_insn_r->arm_insn, 23, 24);
+
+  if (0 == op || 3 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle RFE instruction.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          /* Handle SRS instruction after reading banked SP.  */
+          return arm_record_unsupported_insn (thumb2_insn_r);
+        }
+    }
+  else if(1 == op || 2 == op)
+    {
+      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+        {
+          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                record_buf[index++] = register_count;
+
+              register_count++;
+              register_bits = register_bits >> 1;
+            }
+          record_buf[index++] = reg_rn;
+          record_buf[index++] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = index;
+        }
+      else
+        {
+          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
+          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
+          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
+          while (register_bits)
+            {
+              if (register_bits & 0x00000001)
+                register_count++;
+
+              register_bits = register_bits >> 1;
+            }
+
+          if (1 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = u_regval;
+            }
+          else if (2 == op)
+            {
+              /* Start address calculation for LDMDB/LDMEA.  */
+              start_address = (u_regval) - (register_count * 4);
+            }
+
+          thumb2_insn_r->mem_rec_count = register_count;
+          while (register_count)
+            {
+              record_buf_mem[(register_count * 2) - 1] = start_address;
+              record_buf_mem[(register_count * 2) - 2] = 4;
+              start_address = start_address + 4;
+              register_count--;
+            }
+          record_buf[0] = reg_rn;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+    }
+
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 load/store (dual/exclusive) and table branch
+   instructions.  */
+
+static int
+thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rd, reg_rn, offset_imm;
+  uint32_t reg_dest1, reg_dest2;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2, op3;
+  LONGEST s_word;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 23, 24);
+  op2 = bits (thumb2_insn_r->arm_insn, 20, 21);
+  op3 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
+    {
+      if(!(1 == op1 && 1 == op2 && (0 == op3 || 1 == op3)))
+        {
+          reg_dest1 = bits (thumb2_insn_r->arm_insn, 12, 15);
+          record_buf[0] = reg_dest1;
+          record_buf[1] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 2;
+        }
+
+      if (3 == op2 || (op1 & 2) || (1 == op1 && 1 == op2 && 7 == op3))
+        {
+          reg_dest2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+          record_buf[2] = reg_dest2;
+          thumb2_insn_r->reg_rec_count = 3;
+        }
+    }
+  else
+    {
+      reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+      regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+      if (0 == op1 && 0 == op2)
+        {
+          /* Handle STREX.  */
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          address = u_regval[0] + (offset_imm * 4);
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          thumb2_insn_r->mem_rec_count = 1;
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else if (1 == op1 && 0 == op2)
+        {
+          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
+          record_buf[0] = reg_rd;
+          thumb2_insn_r->reg_rec_count = 1;
+          address = u_regval[0];
+          record_buf_mem[1] = address;
+
+          if (4 == op3)
+            {
+              /* Handle STREXB.  */
+              record_buf_mem[0] = 1;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (5 == op3)
+            {
+              /* Handle STREXH.  */
+              record_buf_mem[0] = 2 ;
+              thumb2_insn_r->mem_rec_count = 1;
+            }
+          else if (7 == op3)
+            {
+              /* Handle STREXD.  */
+              address = u_regval[0];
+              record_buf_mem[0] = 4;
+              record_buf_mem[2] = 4;
+              record_buf_mem[3] = address + 4;
+              thumb2_insn_r->mem_rec_count = 2;
+            }
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+
+          if (bit (thumb2_insn_r->arm_insn, 24))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 23))
+                offset_addr = u_regval[0] + (offset_imm * 4);
+              else
+                offset_addr = u_regval[0] - (offset_imm * 4);
+
+              address = offset_addr;
+            }
+          else
+            address = u_regval[0];
+
+          record_buf_mem[0] = 4;
+          record_buf_mem[1] = address;
+          record_buf_mem[2] = 4;
+          record_buf_mem[3] = address + 4;
+          thumb2_insn_r->mem_rec_count = 2;
+          record_buf[0] = reg_rn;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 data processing (shift register and modified immediate)
+   instructions.  */
+
+static int
+thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t reg_rd, op;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 21, 24);
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
+    {
+      record_buf[0] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 1;
+    }
+  else
+    {
+      record_buf[0] = reg_rd;
+      record_buf[1] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Generic handler for thumb2 instructions which effect destination and PS
+   registers.  */
+
+static int
+thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t reg_rd;
+  uint32_t record_buf[8];
+
+  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  record_buf[0] = reg_rd;
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 branch and miscellaneous control instructions.  */
+
+static int
+thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t op, op1, op2;
+  uint32_t record_buf[8];
+
+  op = bits (thumb2_insn_r->arm_insn, 20, 26);
+  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
+  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
+
+  /* Handle MSR insn.  */
+  if (!(op1 & 0x2) && 0x38 == op)
+    {
+      if (!(op2 & 0x3))
+        {
+          /* CPSR is going to be changed.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          arm_record_unsupported_insn(thumb2_insn_r);
+          return -1;
+        }
+    }
+  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
+    {
+      /* BLX.  */
+      record_buf[0] = ARM_PS_REGNUM;
+      record_buf[1] = ARM_LR_REGNUM;
+      thumb2_insn_r->reg_rec_count = 2;
+    }
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 store single data item instructions.  */
+
+static int
+thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
+{
+  struct regcache *reg_cache = thumb2_insn_r->regcache;
+
+  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
+  uint32_t address, offset_addr;
+  uint32_t record_buf[8], record_buf_mem[8];
+  uint32_t op1, op2;
+
+  ULONGEST u_regval[2];
+
+  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
+  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
+
+  if (bit (thumb2_insn_r->arm_insn, 23))
+    {
+      /* T2 encoding.  */
+      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
+      offset_addr = u_regval[0] + offset_imm;
+      address = offset_addr;
+    }
+  else
+    {
+      /* T3 encoding.  */
+      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
+        {
+          /* Handle STRB (register).  */
+          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
+          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
+          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
+          offset_addr = u_regval[1] << shift_imm;
+          address = u_regval[0] + offset_addr;
+        }
+      else
+        {
+          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
+          if (bit (thumb2_insn_r->arm_insn, 10))
+            {
+              if (bit (thumb2_insn_r->arm_insn, 9))
+                offset_addr = u_regval[0] + offset_imm;
+              else
+                offset_addr = u_regval[0] - offset_imm;
+
+              address = offset_addr;
+            }
+          else
+            address = u_regval[0];
+        }
+    }
+
+  switch (op1)
+    {
+      /* Store byte instructions.  */
+      case 4:
+      case 0:
+        record_buf_mem[0] = 1;
+      break;
+      /* Store half word instructions.  */
+      case 1:
+      case 5:
+        record_buf_mem[0] = 2;
+      break;
+      /* Store word instructions.  */
+      case 2:
+      case 6:
+        record_buf_mem[0] = 4;
+      break;
+
+      default:
+        gdb_assert_not_reached ("no decoding pattern found");
+      break;
+    }
+
+  record_buf_mem[1] = address;
+  thumb2_insn_r->mem_rec_count = 1;
+  record_buf[0] = reg_rn;
+  thumb2_insn_r->reg_rec_count = 1;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
+            record_buf_mem);
+  return 0;
+}
+
+/* Handler for thumb2 load memory hints instructions.  */
+
+static int
+thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t record_buf[8];
+  uint32_t reg_rt, reg_rn;
+
+  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
+  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
+
+  if (ARM_PC_REGNUM != reg_rt)
+    {
+      record_buf[0] = reg_rt;
+      record_buf[1] = reg_rn;
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+
+      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+                record_buf);
+      return 0;
+    }
+
+  return -1;
+}
+
+/* Handler for thumb2 load word instructions.  */
+
+static int
+thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+
+  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
+  record_buf[1] = ARM_PS_REGNUM;
+  thumb2_insn_r->reg_rec_count = 2;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return 0;
+}
+
+/* Handler for thumb2 long multiply, long multiply accumulate, and
+   divide instructions.  Return value: -1:record failure ;  0:success.  */
+
+static int
+thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
+{
+  uint32_t ret = 0;
+  uint32_t opcode1 = 0, opcode2 = 0;
+  uint32_t record_buf[8];
+  uint32_t reg_src1 = 0;
+
+  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
+  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
+
+  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
+    {
+      /* Handle SMULL, UMULL, SMULAL.  */
+      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else if (1 == opcode1 || 3 == opcode2)
+    {
+      /* Handle SDIV and UDIV.  */
+      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
+      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
+      record_buf[2] = ARM_PS_REGNUM;
+      thumb2_insn_r->reg_rec_count = 3;
+    }
+  else
+    ret = -1;
+
+  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
+            record_buf);
+  return ret;
+}
+
+/* Decodes thumb2 instruction type and return an instruction id.  */
+
+static unsigned int
+thumb2_record_decode_inst_id (uint32_t thumb2_insn)
+{
+  uint32_t op = 0;
+  uint32_t op1 = 0;
+  uint32_t op2 = 0;
+
+  op = bit (thumb2_insn, 15);
+  op1 = bits (thumb2_insn, 27, 28);
+  op2 = bits (thumb2_insn, 20, 26);
+
+  if (op1 == 0x01)
+    {
+      if (!(op2 & 0x64 ))
+        {
+          /* Load/store multiple instruction.  */
+          return 0;
+        }
+      else if (!((op2 & 0x64) ^ 0x04))
+        {
+          /* Load/store (dual/exclusive) and table branch instruction.  */
+          return 1;
+        }
+      else if (!((op2 & 0x20) ^ 0x20))
+        {
+          /* Data-processing (shifted register).  */
+          return 2;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 3;
+        }
+    }
+  else if (op1 == 0x02)
+    {
+      if (op)
+        {
+          /* Branches and miscellaneous control instructions.  */
+          return 6;
+        }
+      else if (op2 & 0x20)
+        {
+          /* Data-processing (plain binary immediate) instruction.  */
+          return 5;
+        }
+      else
+        {
+          /* Data-processing (modified immediate).  */
+          return 4;
+        }
+    }
+  else if (op1 == 0x03)
+    {
+      if (!(op2 & 0x71 ))
+        {
+          /* Store single data item.  */
+          return 7;
+        }
+      else if (!((op2 & 0x71) ^ 0x10))
+        {
+          /* Advanced SIMD or structure load/store instructions.  */
+          return 8;
+        }
+      else if (!((op2 & 0x67) ^ 0x01))
+        {
+          /* Load byte, memory hints instruction.  */
+          return 9;
+        }
+      else if (!((op2 & 0x67) ^ 0x03))
+        {
+          /* Load halfword, memory hints instruction.  */
+          return 10;
+        }
+      else if (!((op2 & 0x67) ^ 0x05))
+        {
+          /* Load word instruction.  */
+          return 11;
+        }
+      else if (!((op2 & 0x70) ^ 0x20))
+        {
+          /* Data-processing (register) instruction.  */
+          return 12;
+        }
+      else if (!((op2 & 0x78) ^ 0x30))
+        {
+          /* Multiply, multiply accumulate, abs diff instruction.  */
+          return 13;
+        }
+      else if (!((op2 & 0x78) ^ 0x38))
+        {
+          /* Long multiply, long multiply accumulate, and divide.  */
+          return 14;
+        }
+      else if (op2 & 0x40)
+        {
+          /* Co-processor instructions.  */
+          return 15;
+        }
+   }
+
+  return -1;
+}
 
 /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0 on success 
 and positive val on fauilure.  */
@@ -12452,7 +13030,7 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
     arm_record_ld_st_reg_offset,        /* 011.  */
     arm_record_ld_st_multiple,          /* 100.  */
     arm_record_b_bl,                    /* 101.  */
-    arm_record_coproc,                  /* 110.  */
+    arm_record_unsupported_insn,                  /* 110.  */
     arm_record_coproc_data_proc         /* 111.  */
   };
 
@@ -12469,6 +13047,28 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
     thumb_record_branch                /* 111.  */
   };
 
+  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
+     instruction.  */
+  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
+  { \
+    thumb2_record_ld_st_multiple,       /* 00.  */
+    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
+    arm_record_unsupported_insn,        /* 03.  */
+    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
+    thumb2_record_ps_dest_generic,      /* 05.  */
+    thumb2_record_branch_misc_cntrl,    /* 06.  */
+    thumb2_record_str_single_data,      /* 07.  */
+    arm_record_unsupported_insn,        /* 08.  */
+    thumb2_record_ld_mem_hints,         /* 09.  */
+    thumb2_record_ld_mem_hints,         /* 10.  */
+    thumb2_record_ld_word,              /* 11.  */
+    thumb2_record_ps_dest_generic,      /* 12.  */
+    thumb2_record_ps_dest_generic,      /* 13.  */
+    thumb2_record_lmul_lmla_div,        /* 14.  */
+    arm_record_unsupported_insn         /* 15.  */
+  };
+
   uint32_t ret = 0;    /* return value: negative:failure   0:success.  */
   uint32_t insn_id = 0;
 
@@ -12503,11 +13103,22 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
     }
   else if (THUMB2_RECORD == record_type)
     {
-      printf_unfiltered (_("Process record doesnt support thumb32 instruction "
-                           "0x%0x at address %s.\n"),arm_record->arm_insn,
-                           paddress (arm_record->gdbarch, 
-                           arm_record->this_addr));
-      ret = -1;
+      /* As thumb does not have condition codes, we set negative.  */
+      arm_record->cond = -1;
+
+      /* Swap first half of 32bit thumb instruction with second half.  */
+      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
+                             (arm_record->arm_insn << 16);
+
+      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
+
+      if (insn_id >= 0)
+        ret = thumb2_handle_insn[insn_id] (arm_record);
+      else
+        {
+          arm_record_unsupported_insn(arm_record);
+          ret = -1;
+        }
     }
   else
     {
-- 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-25  0:05       ` Omair Javaid
@ 2013-11-25 14:23         ` Oza Pawandeep
  2013-11-27 23:58           ` Omair Javaid
  2013-12-20 12:37         ` Pedro Alves
  1 sibling, 1 reply; 10+ messages in thread
From: Oza Pawandeep @ 2013-11-25 14:23 UTC (permalink / raw)
  To: Omair Javaid; +Cc: Yao Qi, gdb-patches, Patch Tracking

Hi Omar,

couple of comments.

1) you this part of code is still using MAGIC numbers.
+      if (!(op2 & 0x71 ))
+        {
+          /* Store single data item.  */
+          return 7;
+        }
+      else if (!((op2 & 0x71) ^ 0x10))
+        {
+          /* Advanced SIMD or structure load/store instructions.  */
+          return 8;
+        }
+      else if (!((op2 & 0x67) ^ 0x01))
+        {
+          /* Load byte, memory hints instruction.  */
+          return 9;
+        }
+      else if (!((op2 & 0x67) ^ 0x03))
+        {
+          /* Load halfword, memory hints instruction.  */
+          return 10;
+        }
+      else if (!((op2 & 0x67) ^ 0x05))
+        {
+          /* Load word instruction.  */
+          return 11;
+        }


2) following code else part is not required.

  /* Handle MSR insn.  */
+  if (!(op1 & 0x2) && 0x38 == op)
+    {
+      if (!(op2 & 0x3))
+        {
+          /* CPSR is going to be changed.  */
+          record_buf[0] = ARM_PS_REGNUM;
+          thumb2_insn_r->reg_rec_count = 1;
+        }
+      else
+        {
+          arm_record_unsupported_insn(
thumb2_insn_r);
+          return -1;
+        }

because of folowing.
// SPSRWriteByInstr()
// ==================
SPSRWriteByInstr(bits(32) value, bits(4) bytemask)
if CurrentModeIsUserOrSystem() then UNPREDICTABLE;

gdb is for user space; and use space is not allowed to use SPSR
directly using MSR instruction.
so old code base + new code base whereever we have got SPSR getting
modified we need to remove the same.

3) did you run the testsuite at whole code-base ?


ps: I am still struggling to get my public key setup at gdb git. will
try to get it done as soonas I can.

Regards,
Oza.



On Mon, Nov 25, 2013 at 5:19 AM, Omair Javaid <omair.javaid@linaro.org> wrote:
> On 11/11/2013 03:09 PM, Yao Qi wrote:
>> Patch looks good to me, and you still need a maintainer's approval.
>>
>> Some of the comments are too long.  Please make sure they don't exceed
>> the limitation (around 74 characters should be fine).
>>
>> On 11/08/2013 11:19 AM, Omair Javaid wrote:
>>> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
>>> instructions.  */
>>> +
>>
>> here.
>>
>>> +static int
>>> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
>>> +{
>>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>>> +
>>
>> ....
>>
>>> +
>>> +/* Decodes thumb2 instruction type and return an instruction id.  */
>>> +
>>> +static unsigned int
>>> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
>>> +{
>>> +  uint32_t op = 0;
>>> +  uint32_t op1 = 0;
>>> +  uint32_t op2 = 0;
>>> +
>>> +  op = bit (thumb2_insn, 15);
>>> +  op1 = bits (thumb2_insn, 27, 28);
>>> +  op2 = bits (thumb2_insn, 20, 26);
>>> +
>>> +  if (op1 == 0x01)
>>> +    {
>>> +      if (!(op2 & 0x64 ))
>>> +        {
>>> +          /* Load/store multiple instruction.  */
>>> +          return 0;
>>> +        }
>>> +      else if (!((op2 & 0x64) ^ 0x04))
>>> +        {
>>> +          /* Load/store dual, load/store exclusive, table branch
>>> instruction.  */
>>
>> and here.
>>
>>
>>> +          return 1;
>>> +        }
>>> +      else if (!((op2 & 0x20) ^ 0x20))
>>> +        {
>>> +          /* Data-processing (shifted register).  */
>>> +          return 2;
>>> +        }
>>> +      else if (op2 & 0x40)
>>> +        {
>>> +          /* Co-processor instructions.  */
>>> +          return 3;
>>> +        }
>>> +    }
>>> +  else if (op1 == 0x02)
>>> +    {
>>> +      if (op)
>>> +        {
>>> +          /* Branches and miscellaneous control instructions.  */
>>> +          return 6;
>>> +        }
>>> +      else if (op2 & 0x20)
>>> +        {
>>> +          /* Data-processing (plain binary immediate) instruction.  */
>>> +          return 5;
>>> +        }
>>> +      else
>>> +        {
>>> +          /* Data-processing (modified immediate).  */
>>> +          return 4;
>>> +        }
>>> +    }
>>> +  else if (op1 == 0x03)
>>> +   {
>>> +      if (!(op2 & 0x71 ))
>>> +        {
>>> +          /* Store single data item.  */
>>> +          return 7;
>>> +        }
>>> +      else if (!((op2 & 0x71) ^ 0x10))
>>> +        {
>>> +          /* Advanced SIMD element or structure load/store
>>> instructions.  */
>>
>> here.
>>
>>> +          return 8;
>>> +        }
>>> +      else if (!((op2 & 0x67) ^ 0x01))
>>> +        {
>>> +          /* Load byte, memory hints instruction.  */
>>> +          return 9;
>>> +        }
>>> +      else if (!((op2 & 0x67) ^ 0x03))
>>> +        {
>>> +          /* Load halfword, memory hints instruction.  */
>>> +          return 10;
>>> +        }
>>> +      else if (!((op2 & 0x67) ^ 0x05))
>>> +        {
>>> +          /* Load word instruction.  */
>>> +          return 11;
>>> +        }
>>> +      else if (!((op2 & 0x70) ^ 0x20))
>>> +        {
>>> +          /* Data-processing (register) instruction.  */
>>> +          return 12;
>>> +        }
>>> +      else if (!((op2 & 0x78) ^ 0x30))
>>> +        {
>>> +          /* Multiply, multiply accumulate, absolute difference
>>> instruction.  */
>>
>> here.
>>
>>> +          return 13;
>>> +        }
>>> +      else if (!((op2 & 0x78) ^ 0x38))
>>> +        {
>>> +          /* Long multiply, long multiply accumulate, and divide.  */
>>> +          return 14;
>>> +        }
>>> +      else if (op2 & 0x40)
>>> +        {
>>> +          /* Co-processor instructions.  */
>>> +          return 15;
>>> +        }
>>> +   }
>>> +
>>> +  return -1;
>>> +}
>>>
>>>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0
>>> on success
>>>   and positive val on fauilure.  */
>>> @@ -12452,7 +13034,7 @@ decode_insn (insn_decode_record *arm_rec
>>>       arm_record_ld_st_reg_offset,        /* 011.  */
>>>       arm_record_ld_st_multiple,          /* 100.  */
>>>       arm_record_b_bl,                    /* 101.  */
>>> -    arm_record_coproc,                  /* 110.  */
>>> +    arm_record_unsupported_insn,                  /* 110.  */
>>>       arm_record_coproc_data_proc         /* 111.  */
>>>     };
>>>
>>> @@ -12469,6 +13051,27 @@ decode_insn (insn_decode_record *arm_rec
>>>       thumb_record_branch                /* 111.  */
>>>     };
>>>
>>> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
>>> instruction.  */
>>
>> here.
>>
>
> Patch has been updated after incorporating suggestions. Looking for a final
> commit verdict.
>
> This patch adds support for recording thumb32 instructions. This patch also
> fixes bugs in arm process record instruction decoding.
>
> gdb:
>
> 2013-11-08  Omair Javaid  <omair.javaid@linaro.org>
>
>         * arm-tdep.c (struct arm_mem_r): Use uint32_t for memory address.
>         (arm_record_unsupported_insn): New function.
>         (arm_record_coproc_data_proc): Removed.
>         (thumb2_record_ld_st_multiple): New function.
>         (thumb2_record_ld_st_dual_ex_tbb): New function.
>         (thumb2_record_data_proc_sreg_mimm): New function.
>         (thumb2_record_ps_dest_generic): New function.
>         (thumb2_record_branch_misc_cntrl): New function.
>         (thumb2_record_str_single_data): New function.
>         (thumb2_record_ld_mem_hints): New function.
>         (thumb2_record_ld_word): New function.
>         (thumb2_record_lmul_lmla_div): New function.
>         (thumb2_record_decode_inst_id): New function.
>         (decode_insn): Add thumb32 instruction handlers.
>
> ---
>  gdb/arm-tdep.c |  627 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 619 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 7c78a61..ecaced7 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -10618,7 +10618,7 @@ vfp - VFP co-processor."),
>  struct arm_mem_r
>  {
>    uint32_t len;    /* Record length.  */
> -  CORE_ADDR addr;  /* Memory address.  */
> +  uint32_t addr;   /* Memory address.  */
>  };
>
>  /* ARM instruction record contains opcode of current insn
> @@ -11919,7 +11919,7 @@ arm_record_b_bl (insn_decode_record *arm_insn_r)
>  /* Handling opcode 110 insns.  */
>
>  static int
> -arm_record_coproc (insn_decode_record *arm_insn_r)
> +arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
>  {
>    printf_unfiltered (_("Process record does not support instruction "
>                      "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
> @@ -12414,6 +12414,584 @@ thumb_record_branch (insn_decode_record *thumb_insn_r)
>    return 0;
>  }
>
> +/* Handler for thumb2 load/store multiple instructions.  */
> +
> +static int
> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rn, op;
> +  uint32_t register_bits = 0, register_count = 0;
> +  uint32_t index = 0, start_address = 0;
> +  uint32_t record_buf[24], record_buf_mem[48];
> +
> +  ULONGEST u_regval = 0;
> +
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +  op = bits (thumb2_insn_r->arm_insn, 23, 24);
> +
> +  if (0 == op || 3 == op)
> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle RFE instruction.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          /* Handle SRS instruction after reading banked SP.  */
> +          return arm_record_unsupported_insn (thumb2_insn_r);
> +        }
> +    }
> +  else if(1 == op || 2 == op)
> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                record_buf[index++] = register_count;
> +
> +              register_count++;
> +              register_bits = register_bits >> 1;
> +            }
> +          record_buf[index++] = reg_rn;
> +          record_buf[index++] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = index;
> +        }
> +      else
> +        {
> +          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                register_count++;
> +
> +              register_bits = register_bits >> 1;
> +            }
> +
> +          if (1 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = u_regval;
> +            }
> +          else if (2 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = (u_regval) - (register_count * 4);
> +            }
> +
> +          thumb2_insn_r->mem_rec_count = register_count;
> +          while (register_count)
> +            {
> +              record_buf_mem[(register_count * 2) - 1] = start_address;
> +              record_buf_mem[(register_count * 2) - 2] = 4;
> +              start_address = start_address + 4;
> +              register_count--;
> +            }
> +          record_buf[0] = reg_rn;
> +          record_buf[1] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 2;
> +        }
> +    }
> +
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 load/store (dual/exclusive) and table branch
> +   instructions.  */
> +
> +static int
> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rd, reg_rn, offset_imm;
> +  uint32_t reg_dest1, reg_dest2;
> +  uint32_t address, offset_addr;
> +  uint32_t record_buf[8], record_buf_mem[8];
> +  uint32_t op1, op2, op3;
> +  LONGEST s_word;
> +
> +  ULONGEST u_regval[2];
> +
> +  op1 = bits (thumb2_insn_r->arm_insn, 23, 24);
> +  op2 = bits (thumb2_insn_r->arm_insn, 20, 21);
> +  op3 = bits (thumb2_insn_r->arm_insn, 4, 7);
> +
> +  if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +    {
> +      if(!(1 == op1 && 1 == op2 && (0 == op3 || 1 == op3)))
> +        {
> +          reg_dest1 = bits (thumb2_insn_r->arm_insn, 12, 15);
> +          record_buf[0] = reg_dest1;
> +          record_buf[1] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 2;
> +        }
> +
> +      if (3 == op2 || (op1 & 2) || (1 == op1 && 1 == op2 && 7 == op3))
> +        {
> +          reg_dest2 = bits (thumb2_insn_r->arm_insn, 8, 11);
> +          record_buf[2] = reg_dest2;
> +          thumb2_insn_r->reg_rec_count = 3;
> +        }
> +    }
> +  else
> +    {
> +      reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
> +
> +      if (0 == op1 && 0 == op2)
> +        {
> +          /* Handle STREX.  */
> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
> +          address = u_regval[0] + (offset_imm * 4);
> +          record_buf_mem[0] = 4;
> +          record_buf_mem[1] = address;
> +          thumb2_insn_r->mem_rec_count = 1;
> +          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
> +          record_buf[0] = reg_rd;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else if (1 == op1 && 0 == op2)
> +        {
> +          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
> +          record_buf[0] = reg_rd;
> +          thumb2_insn_r->reg_rec_count = 1;
> +          address = u_regval[0];
> +          record_buf_mem[1] = address;
> +
> +          if (4 == op3)
> +            {
> +              /* Handle STREXB.  */
> +              record_buf_mem[0] = 1;
> +              thumb2_insn_r->mem_rec_count = 1;
> +            }
> +          else if (5 == op3)
> +            {
> +              /* Handle STREXH.  */
> +              record_buf_mem[0] = 2 ;
> +              thumb2_insn_r->mem_rec_count = 1;
> +            }
> +          else if (7 == op3)
> +            {
> +              /* Handle STREXD.  */
> +              address = u_regval[0];
> +              record_buf_mem[0] = 4;
> +              record_buf_mem[2] = 4;
> +              record_buf_mem[3] = address + 4;
> +              thumb2_insn_r->mem_rec_count = 2;
> +            }
> +        }
> +      else
> +        {
> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
> +
> +          if (bit (thumb2_insn_r->arm_insn, 24))
> +            {
> +              if (bit (thumb2_insn_r->arm_insn, 23))
> +                offset_addr = u_regval[0] + (offset_imm * 4);
> +              else
> +                offset_addr = u_regval[0] - (offset_imm * 4);
> +
> +              address = offset_addr;
> +            }
> +          else
> +            address = u_regval[0];
> +
> +          record_buf_mem[0] = 4;
> +          record_buf_mem[1] = address;
> +          record_buf_mem[2] = 4;
> +          record_buf_mem[3] = address + 4;
> +          thumb2_insn_r->mem_rec_count = 2;
> +          record_buf[0] = reg_rn;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +    }
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 data processing (shift register and modified immediate)
> +   instructions.  */
> +
> +static int
> +thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t reg_rd, op;
> +  uint32_t record_buf[8];
> +
> +  op = bits (thumb2_insn_r->arm_insn, 21, 24);
> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> +  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
> +    {
> +      record_buf[0] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 1;
> +    }
> +  else
> +    {
> +      record_buf[0] = reg_rd;
> +      record_buf[1] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 2;
> +    }
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Generic handler for thumb2 instructions which effect destination and PS
> +   registers.  */
> +
> +static int
> +thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t reg_rd;
> +  uint32_t record_buf[8];
> +
> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> +  record_buf[0] = reg_rd;
> +  record_buf[1] = ARM_PS_REGNUM;
> +  thumb2_insn_r->reg_rec_count = 2;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 branch and miscellaneous control instructions.  */
> +
> +static int
> +thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t op, op1, op2;
> +  uint32_t record_buf[8];
> +
> +  op = bits (thumb2_insn_r->arm_insn, 20, 26);
> +  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
> +  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> +  /* Handle MSR insn.  */
> +  if (!(op1 & 0x2) && 0x38 == op)
> +    {
> +      if (!(op2 & 0x3))
> +        {
> +          /* CPSR is going to be changed.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          arm_record_unsupported_insn(thumb2_insn_r);
> +          return -1;
> +        }
> +    }
> +  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
> +    {
> +      /* BLX.  */
> +      record_buf[0] = ARM_PS_REGNUM;
> +      record_buf[1] = ARM_LR_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 2;
> +    }
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 store single data item instructions.  */
> +
> +static int
> +thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
> +  uint32_t address, offset_addr;
> +  uint32_t record_buf[8], record_buf_mem[8];
> +  uint32_t op1, op2;
> +
> +  ULONGEST u_regval[2];
> +
> +  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
> +  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
> +
> +  if (bit (thumb2_insn_r->arm_insn, 23))
> +    {
> +      /* T2 encoding.  */
> +      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
> +      offset_addr = u_regval[0] + offset_imm;
> +      address = offset_addr;
> +    }
> +  else
> +    {
> +      /* T3 encoding.  */
> +      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
> +        {
> +          /* Handle STRB (register).  */
> +          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
> +          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
> +          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
> +          offset_addr = u_regval[1] << shift_imm;
> +          address = u_regval[0] + offset_addr;
> +        }
> +      else
> +        {
> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
> +          if (bit (thumb2_insn_r->arm_insn, 10))
> +            {
> +              if (bit (thumb2_insn_r->arm_insn, 9))
> +                offset_addr = u_regval[0] + offset_imm;
> +              else
> +                offset_addr = u_regval[0] - offset_imm;
> +
> +              address = offset_addr;
> +            }
> +          else
> +            address = u_regval[0];
> +        }
> +    }
> +
> +  switch (op1)
> +    {
> +      /* Store byte instructions.  */
> +      case 4:
> +      case 0:
> +        record_buf_mem[0] = 1;
> +      break;
> +      /* Store half word instructions.  */
> +      case 1:
> +      case 5:
> +        record_buf_mem[0] = 2;
> +      break;
> +      /* Store word instructions.  */
> +      case 2:
> +      case 6:
> +        record_buf_mem[0] = 4;
> +      break;
> +
> +      default:
> +        gdb_assert_not_reached ("no decoding pattern found");
> +      break;
> +    }
> +
> +  record_buf_mem[1] = address;
> +  thumb2_insn_r->mem_rec_count = 1;
> +  record_buf[0] = reg_rn;
> +  thumb2_insn_r->reg_rec_count = 1;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 load memory hints instructions.  */
> +
> +static int
> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t record_buf[8];
> +  uint32_t reg_rt, reg_rn;
> +
> +  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +
> +  if (ARM_PC_REGNUM != reg_rt)
> +    {
> +      record_buf[0] = reg_rt;
> +      record_buf[1] = reg_rn;
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +
> +      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +                record_buf);
> +      return 0;
> +    }
> +
> +  return -1;
> +}
> +
> +/* Handler for thumb2 load word instructions.  */
> +
> +static int
> +thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t opcode1 = 0, opcode2 = 0;
> +  uint32_t record_buf[8];
> +
> +  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +  record_buf[1] = ARM_PS_REGNUM;
> +  thumb2_insn_r->reg_rec_count = 2;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +
> +/* Handler for thumb2 long multiply, long multiply accumulate, and
> +   divide instructions.  Return value: -1:record failure ;  0:success.  */
> +
> +static int
> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;
> +  uint32_t opcode1 = 0, opcode2 = 0;
> +  uint32_t record_buf[8];
> +  uint32_t reg_src1 = 0;
> +
> +  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
> +  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
> +
> +  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
> +    {
> +      /* Handle SMULL, UMULL, SMULAL.  */
> +      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else if (1 == opcode1 || 3 == opcode2)
> +    {
> +      /* Handle SDIV and UDIV.  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else
> +    ret = -1;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return ret;
> +}
> +
> +/* Decodes thumb2 instruction type and return an instruction id.  */
> +
> +static unsigned int
> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
> +{
> +  uint32_t op = 0;
> +  uint32_t op1 = 0;
> +  uint32_t op2 = 0;
> +
> +  op = bit (thumb2_insn, 15);
> +  op1 = bits (thumb2_insn, 27, 28);
> +  op2 = bits (thumb2_insn, 20, 26);
> +
> +  if (op1 == 0x01)
> +    {
> +      if (!(op2 & 0x64 ))
> +        {
> +          /* Load/store multiple instruction.  */
> +          return 0;
> +        }
> +      else if (!((op2 & 0x64) ^ 0x04))
> +        {
> +          /* Load/store (dual/exclusive) and table branch instruction.  */
> +          return 1;
> +        }
> +      else if (!((op2 & 0x20) ^ 0x20))
> +        {
> +          /* Data-processing (shifted register).  */
> +          return 2;
> +        }
> +      else if (op2 & 0x40)
> +        {
> +          /* Co-processor instructions.  */
> +          return 3;
> +        }
> +    }
> +  else if (op1 == 0x02)
> +    {
> +      if (op)
> +        {
> +          /* Branches and miscellaneous control instructions.  */
> +          return 6;
> +        }
> +      else if (op2 & 0x20)
> +        {
> +          /* Data-processing (plain binary immediate) instruction.  */
> +          return 5;
> +        }
> +      else
> +        {
> +          /* Data-processing (modified immediate).  */
> +          return 4;
> +        }
> +    }
> +  else if (op1 == 0x03)
> +    {
> +      if (!(op2 & 0x71 ))
> +        {
> +          /* Store single data item.  */
> +          return 7;
> +        }
> +      else if (!((op2 & 0x71) ^ 0x10))
> +        {
> +          /* Advanced SIMD or structure load/store instructions.  */
> +          return 8;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x01))
> +        {
> +          /* Load byte, memory hints instruction.  */
> +          return 9;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x03))
> +        {
> +          /* Load halfword, memory hints instruction.  */
> +          return 10;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x05))
> +        {
> +          /* Load word instruction.  */
> +          return 11;
> +        }
> +      else if (!((op2 & 0x70) ^ 0x20))
> +        {
> +          /* Data-processing (register) instruction.  */
> +          return 12;
> +        }
> +      else if (!((op2 & 0x78) ^ 0x30))
> +        {
> +          /* Multiply, multiply accumulate, abs diff instruction.  */
> +          return 13;
> +        }
> +      else if (!((op2 & 0x78) ^ 0x38))
> +        {
> +          /* Long multiply, long multiply accumulate, and divide.  */
> +          return 14;
> +        }
> +      else if (op2 & 0x40)
> +        {
> +          /* Co-processor instructions.  */
> +          return 15;
> +        }
> +   }
> +
> +  return -1;
> +}
>
>  /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0 on success
>  and positive val on fauilure.  */
> @@ -12452,7 +13030,7 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>      arm_record_ld_st_reg_offset,        /* 011.  */
>      arm_record_ld_st_multiple,          /* 100.  */
>      arm_record_b_bl,                    /* 101.  */
> -    arm_record_coproc,                  /* 110.  */
> +    arm_record_unsupported_insn,                  /* 110.  */
>      arm_record_coproc_data_proc         /* 111.  */
>    };
>
> @@ -12469,6 +13047,28 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>      thumb_record_branch                /* 111.  */
>    };
>
> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
> +     instruction.  */
> +  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
> +  { \
> +    thumb2_record_ld_st_multiple,       /* 00.  */
> +    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
> +    arm_record_unsupported_insn,        /* 03.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
> +    thumb2_record_ps_dest_generic,      /* 05.  */
> +    thumb2_record_branch_misc_cntrl,    /* 06.  */
> +    thumb2_record_str_single_data,      /* 07.  */
> +    arm_record_unsupported_insn,        /* 08.  */
> +    thumb2_record_ld_mem_hints,         /* 09.  */
> +    thumb2_record_ld_mem_hints,         /* 10.  */
> +    thumb2_record_ld_word,              /* 11.  */
> +    thumb2_record_ps_dest_generic,      /* 12.  */
> +    thumb2_record_ps_dest_generic,      /* 13.  */
> +    thumb2_record_lmul_lmla_div,        /* 14.  */
> +    arm_record_unsupported_insn         /* 15.  */
> +  };
> +
>    uint32_t ret = 0;    /* return value: negative:failure   0:success.  */
>    uint32_t insn_id = 0;
>
> @@ -12503,11 +13103,22 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>      }
>    else if (THUMB2_RECORD == record_type)
>      {
> -      printf_unfiltered (_("Process record doesnt support thumb32 instruction "
> -                           "0x%0x at address %s.\n"),arm_record->arm_insn,
> -                           paddress (arm_record->gdbarch,
> -                           arm_record->this_addr));
> -      ret = -1;
> +      /* As thumb does not have condition codes, we set negative.  */
> +      arm_record->cond = -1;
> +
> +      /* Swap first half of 32bit thumb instruction with second half.  */
> +      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
> +                             (arm_record->arm_insn << 16);
> +
> +      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
> +
> +      if (insn_id >= 0)
> +        ret = thumb2_handle_insn[insn_id] (arm_record);
> +      else
> +        {
> +          arm_record_unsupported_insn(arm_record);
> +          ret = -1;
> +        }
>      }
>    else
>      {
> --

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-25 14:23         ` Oza Pawandeep
@ 2013-11-27 23:58           ` Omair Javaid
  2013-11-28 12:30             ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2013-11-27 23:58 UTC (permalink / raw)
  To: Oza Pawandeep; +Cc: Yao Qi, gdb-patches, Patch Tracking


On 11/25/2013 06:07 PM, Oza Pawandeep wrote:
> Hi Omar,
> 
> couple of comments.
> 
> 1) you this part of code is still using MAGIC numbers.
> +      if (!(op2 & 0x71 ))
> +        {
> +          /* Store single data item.  */
> +          return 7;
> +        }
> +      else if (!((op2 & 0x71) ^ 0x10))
> +        {
> +          /* Advanced SIMD or structure load/store instructions.  */
> +          return 8;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x01))
> +        {
> +          /* Load byte, memory hints instruction.  */
> +          return 9;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x03))
> +        {
> +          /* Load halfword, memory hints instruction.  */
> +          return 10;
> +        }
> +      else if (!((op2 & 0x67) ^ 0x05))
> +        {
> +          /* Load word instruction.  */
> +          return 11;
> +        }
> 
In principle I agree that code must be more readable overall but that is applicable to all the code residing in arm-tdep.c. The nomenclature and bit manipulations in above code is used in line with ARM reference manual and can be understood if we have Thumb32 instruction encoding page infront.
> 
> 2) following code else part is not required.
> 
>   /* Handle MSR insn.  */
> +  if (!(op1 & 0x2) && 0x38 == op)
> +    {
> +      if (!(op2 & 0x3))
> +        {
> +          /* CPSR is going to be changed.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          arm_record_unsupported_insn(
> thumb2_insn_r);
> +          return -1;
> +        }
> 
> because of folowing.
> // SPSRWriteByInstr()
> // ==================
> SPSRWriteByInstr(bits(32) value, bits(4) bytemask)
> if CurrentModeIsUserOrSystem() then UNPREDICTABLE;
> 
> gdb is for user space; and use space is not allowed to use SPSR
> directly using MSR instruction.
> so old code base + new code base whereever we have got SPSR getting
> modified we need to remove the same.
> 
I agree with you that we have to do a lot of rework of previous process record code. But I am not sure it would be productive for us to get working code out and loose the functionality or delay its submission. As Record/Replay is pretty much functional with this set of patches I am hoping that we can do a complete rework if required later on.

> 3) did you run the testsuite at whole code-base ?
I have tested with a a fresh git checkout alongwith 4 patches I have sent. In native mode on a samsung chromebook there are only 69 failures left out of around 2500 test cases. I am working on more bug fixes and hopefully will be able to resolve rest of issues as well. 
> 
> 
> ps: I am still struggling to get my public key setup at gdb git. will
> try to get it done as soonas I can.
> 
> Regards,
> Oza.
> 
> 
> 
> On Mon, Nov 25, 2013 at 5:19 AM, Omair Javaid <omair.javaid@linaro.org> wrote:
>> On 11/11/2013 03:09 PM, Yao Qi wrote:
>>> Patch looks good to me, and you still need a maintainer's approval.
>>>
>>> Some of the comments are too long.  Please make sure they don't exceed
>>> the limitation (around 74 characters should be fine).
>>>
>>> On 11/08/2013 11:19 AM, Omair Javaid wrote:
>>>> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
>>>> instructions.  */
>>>> +
>>>
>>> here.
>>>
>>>> +static int
>>>> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
>>>> +{
>>>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>>>> +
>>>
>>> ....
>>>
>>>> +
>>>> +/* Decodes thumb2 instruction type and return an instruction id.  */
>>>> +
>>>> +static unsigned int
>>>> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
>>>> +{
>>>> +  uint32_t op = 0;
>>>> +  uint32_t op1 = 0;
>>>> +  uint32_t op2 = 0;
>>>> +
>>>> +  op = bit (thumb2_insn, 15);
>>>> +  op1 = bits (thumb2_insn, 27, 28);
>>>> +  op2 = bits (thumb2_insn, 20, 26);
>>>> +
>>>> +  if (op1 == 0x01)
>>>> +    {
>>>> +      if (!(op2 & 0x64 ))
>>>> +        {
>>>> +          /* Load/store multiple instruction.  */
>>>> +          return 0;
>>>> +        }
>>>> +      else if (!((op2 & 0x64) ^ 0x04))
>>>> +        {
>>>> +          /* Load/store dual, load/store exclusive, table branch
>>>> instruction.  */
>>>
>>> and here.
>>>
>>>
>>>> +          return 1;
>>>> +        }
>>>> +      else if (!((op2 & 0x20) ^ 0x20))
>>>> +        {
>>>> +          /* Data-processing (shifted register).  */
>>>> +          return 2;
>>>> +        }
>>>> +      else if (op2 & 0x40)
>>>> +        {
>>>> +          /* Co-processor instructions.  */
>>>> +          return 3;
>>>> +        }
>>>> +    }
>>>> +  else if (op1 == 0x02)
>>>> +    {
>>>> +      if (op)
>>>> +        {
>>>> +          /* Branches and miscellaneous control instructions.  */
>>>> +          return 6;
>>>> +        }
>>>> +      else if (op2 & 0x20)
>>>> +        {
>>>> +          /* Data-processing (plain binary immediate) instruction.  */
>>>> +          return 5;
>>>> +        }
>>>> +      else
>>>> +        {
>>>> +          /* Data-processing (modified immediate).  */
>>>> +          return 4;
>>>> +        }
>>>> +    }
>>>> +  else if (op1 == 0x03)
>>>> +   {
>>>> +      if (!(op2 & 0x71 ))
>>>> +        {
>>>> +          /* Store single data item.  */
>>>> +          return 7;
>>>> +        }
>>>> +      else if (!((op2 & 0x71) ^ 0x10))
>>>> +        {
>>>> +          /* Advanced SIMD element or structure load/store
>>>> instructions.  */
>>>
>>> here.
>>>
>>>> +          return 8;
>>>> +        }
>>>> +      else if (!((op2 & 0x67) ^ 0x01))
>>>> +        {
>>>> +          /* Load byte, memory hints instruction.  */
>>>> +          return 9;
>>>> +        }
>>>> +      else if (!((op2 & 0x67) ^ 0x03))
>>>> +        {
>>>> +          /* Load halfword, memory hints instruction.  */
>>>> +          return 10;
>>>> +        }
>>>> +      else if (!((op2 & 0x67) ^ 0x05))
>>>> +        {
>>>> +          /* Load word instruction.  */
>>>> +          return 11;
>>>> +        }
>>>> +      else if (!((op2 & 0x70) ^ 0x20))
>>>> +        {
>>>> +          /* Data-processing (register) instruction.  */
>>>> +          return 12;
>>>> +        }
>>>> +      else if (!((op2 & 0x78) ^ 0x30))
>>>> +        {
>>>> +          /* Multiply, multiply accumulate, absolute difference
>>>> instruction.  */
>>>
>>> here.
>>>
>>>> +          return 13;
>>>> +        }
>>>> +      else if (!((op2 & 0x78) ^ 0x38))
>>>> +        {
>>>> +          /* Long multiply, long multiply accumulate, and divide.  */
>>>> +          return 14;
>>>> +        }
>>>> +      else if (op2 & 0x40)
>>>> +        {
>>>> +          /* Co-processor instructions.  */
>>>> +          return 15;
>>>> +        }
>>>> +   }
>>>> +
>>>> +  return -1;
>>>> +}
>>>>
>>>>   /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0
>>>> on success
>>>>   and positive val on fauilure.  */
>>>> @@ -12452,7 +13034,7 @@ decode_insn (insn_decode_record *arm_rec
>>>>       arm_record_ld_st_reg_offset,        /* 011.  */
>>>>       arm_record_ld_st_multiple,          /* 100.  */
>>>>       arm_record_b_bl,                    /* 101.  */
>>>> -    arm_record_coproc,                  /* 110.  */
>>>> +    arm_record_unsupported_insn,                  /* 110.  */
>>>>       arm_record_coproc_data_proc         /* 111.  */
>>>>     };
>>>>
>>>> @@ -12469,6 +13051,27 @@ decode_insn (insn_decode_record *arm_rec
>>>>       thumb_record_branch                /* 111.  */
>>>>     };
>>>>
>>>> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
>>>> instruction.  */
>>>
>>> here.
>>>
>>
>> Patch has been updated after incorporating suggestions. Looking for a final
>> commit verdict.
>>
>> This patch adds support for recording thumb32 instructions. This patch also
>> fixes bugs in arm process record instruction decoding.
>>
>> gdb:
>>
>> 2013-11-08  Omair Javaid  <omair.javaid@linaro.org>
>>
>>         * arm-tdep.c (struct arm_mem_r): Use uint32_t for memory address.
>>         (arm_record_unsupported_insn): New function.
>>         (arm_record_coproc_data_proc): Removed.
>>         (thumb2_record_ld_st_multiple): New function.
>>         (thumb2_record_ld_st_dual_ex_tbb): New function.
>>         (thumb2_record_data_proc_sreg_mimm): New function.
>>         (thumb2_record_ps_dest_generic): New function.
>>         (thumb2_record_branch_misc_cntrl): New function.
>>         (thumb2_record_str_single_data): New function.
>>         (thumb2_record_ld_mem_hints): New function.
>>         (thumb2_record_ld_word): New function.
>>         (thumb2_record_lmul_lmla_div): New function.
>>         (thumb2_record_decode_inst_id): New function.
>>         (decode_insn): Add thumb32 instruction handlers.
>>
>> ---
>>  gdb/arm-tdep.c |  627 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 619 insertions(+), 8 deletions(-)
>>
>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> index 7c78a61..ecaced7 100644
>> --- a/gdb/arm-tdep.c
>> +++ b/gdb/arm-tdep.c
>> @@ -10618,7 +10618,7 @@ vfp - VFP co-processor."),
>>  struct arm_mem_r
>>  {
>>    uint32_t len;    /* Record length.  */
>> -  CORE_ADDR addr;  /* Memory address.  */
>> +  uint32_t addr;   /* Memory address.  */
>>  };
>>
>>  /* ARM instruction record contains opcode of current insn
>> @@ -11919,7 +11919,7 @@ arm_record_b_bl (insn_decode_record *arm_insn_r)
>>  /* Handling opcode 110 insns.  */
>>
>>  static int
>> -arm_record_coproc (insn_decode_record *arm_insn_r)
>> +arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
>>  {
>>    printf_unfiltered (_("Process record does not support instruction "
>>                      "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
>> @@ -12414,6 +12414,584 @@ thumb_record_branch (insn_decode_record *thumb_insn_r)
>>    return 0;
>>  }
>>
>> +/* Handler for thumb2 load/store multiple instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
>> +  uint32_t reg_rn, op;
>> +  uint32_t register_bits = 0, register_count = 0;
>> +  uint32_t index = 0, start_address = 0;
>> +  uint32_t record_buf[24], record_buf_mem[48];
>> +
>> +  ULONGEST u_regval = 0;
>> +
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +  op = bits (thumb2_insn_r->arm_insn, 23, 24);
>> +
>> +  if (0 == op || 3 == op)
>> +    {
>> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
>> +        {
>> +          /* Handle RFE instruction.  */
>> +          record_buf[0] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +      else
>> +        {
>> +          /* Handle SRS instruction after reading banked SP.  */
>> +          return arm_record_unsupported_insn (thumb2_insn_r);
>> +        }
>> +    }
>> +  else if(1 == op || 2 == op)
>> +    {
>> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
>> +        {
>> +          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
>> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
>> +          while (register_bits)
>> +            {
>> +              if (register_bits & 0x00000001)
>> +                record_buf[index++] = register_count;
>> +
>> +              register_count++;
>> +              register_bits = register_bits >> 1;
>> +            }
>> +          record_buf[index++] = reg_rn;
>> +          record_buf[index++] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = index;
>> +        }
>> +      else
>> +        {
>> +          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
>> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
>> +          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
>> +          while (register_bits)
>> +            {
>> +              if (register_bits & 0x00000001)
>> +                register_count++;
>> +
>> +              register_bits = register_bits >> 1;
>> +            }
>> +
>> +          if (1 == op)
>> +            {
>> +              /* Start address calculation for LDMDB/LDMEA.  */
>> +              start_address = u_regval;
>> +            }
>> +          else if (2 == op)
>> +            {
>> +              /* Start address calculation for LDMDB/LDMEA.  */
>> +              start_address = (u_regval) - (register_count * 4);
>> +            }
>> +
>> +          thumb2_insn_r->mem_rec_count = register_count;
>> +          while (register_count)
>> +            {
>> +              record_buf_mem[(register_count * 2) - 1] = start_address;
>> +              record_buf_mem[(register_count * 2) - 2] = 4;
>> +              start_address = start_address + 4;
>> +              register_count--;
>> +            }
>> +          record_buf[0] = reg_rn;
>> +          record_buf[1] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 2;
>> +        }
>> +    }
>> +
>> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
>> +            record_buf_mem);
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 load/store (dual/exclusive) and table branch
>> +   instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_st_dual_ex_tbb (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
>> +  uint32_t reg_rd, reg_rn, offset_imm;
>> +  uint32_t reg_dest1, reg_dest2;
>> +  uint32_t address, offset_addr;
>> +  uint32_t record_buf[8], record_buf_mem[8];
>> +  uint32_t op1, op2, op3;
>> +  LONGEST s_word;
>> +
>> +  ULONGEST u_regval[2];
>> +
>> +  op1 = bits (thumb2_insn_r->arm_insn, 23, 24);
>> +  op2 = bits (thumb2_insn_r->arm_insn, 20, 21);
>> +  op3 = bits (thumb2_insn_r->arm_insn, 4, 7);
>> +
>> +  if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
>> +    {
>> +      if(!(1 == op1 && 1 == op2 && (0 == op3 || 1 == op3)))
>> +        {
>> +          reg_dest1 = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +          record_buf[0] = reg_dest1;
>> +          record_buf[1] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 2;
>> +        }
>> +
>> +      if (3 == op2 || (op1 & 2) || (1 == op1 && 1 == op2 && 7 == op3))
>> +        {
>> +          reg_dest2 = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +          record_buf[2] = reg_dest2;
>> +          thumb2_insn_r->reg_rec_count = 3;
>> +        }
>> +    }
>> +  else
>> +    {
>> +      reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +      regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
>> +
>> +      if (0 == op1 && 0 == op2)
>> +        {
>> +          /* Handle STREX.  */
>> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
>> +          address = u_regval[0] + (offset_imm * 4);
>> +          record_buf_mem[0] = 4;
>> +          record_buf_mem[1] = address;
>> +          thumb2_insn_r->mem_rec_count = 1;
>> +          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
>> +          record_buf[0] = reg_rd;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +      else if (1 == op1 && 0 == op2)
>> +        {
>> +          reg_rd = bits (thumb2_insn_r->arm_insn, 0, 3);
>> +          record_buf[0] = reg_rd;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +          address = u_regval[0];
>> +          record_buf_mem[1] = address;
>> +
>> +          if (4 == op3)
>> +            {
>> +              /* Handle STREXB.  */
>> +              record_buf_mem[0] = 1;
>> +              thumb2_insn_r->mem_rec_count = 1;
>> +            }
>> +          else if (5 == op3)
>> +            {
>> +              /* Handle STREXH.  */
>> +              record_buf_mem[0] = 2 ;
>> +              thumb2_insn_r->mem_rec_count = 1;
>> +            }
>> +          else if (7 == op3)
>> +            {
>> +              /* Handle STREXD.  */
>> +              address = u_regval[0];
>> +              record_buf_mem[0] = 4;
>> +              record_buf_mem[2] = 4;
>> +              record_buf_mem[3] = address + 4;
>> +              thumb2_insn_r->mem_rec_count = 2;
>> +            }
>> +        }
>> +      else
>> +        {
>> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
>> +
>> +          if (bit (thumb2_insn_r->arm_insn, 24))
>> +            {
>> +              if (bit (thumb2_insn_r->arm_insn, 23))
>> +                offset_addr = u_regval[0] + (offset_imm * 4);
>> +              else
>> +                offset_addr = u_regval[0] - (offset_imm * 4);
>> +
>> +              address = offset_addr;
>> +            }
>> +          else
>> +            address = u_regval[0];
>> +
>> +          record_buf_mem[0] = 4;
>> +          record_buf_mem[1] = address;
>> +          record_buf_mem[2] = 4;
>> +          record_buf_mem[3] = address + 4;
>> +          thumb2_insn_r->mem_rec_count = 2;
>> +          record_buf[0] = reg_rn;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +    }
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
>> +            record_buf_mem);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 data processing (shift register and modified immediate)
>> +   instructions.  */
>> +
>> +static int
>> +thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t reg_rd, op;
>> +  uint32_t record_buf[8];
>> +
>> +  op = bits (thumb2_insn_r->arm_insn, 21, 24);
>> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +
>> +  if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
>> +    {
>> +      record_buf[0] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 1;
>> +    }
>> +  else
>> +    {
>> +      record_buf[0] = reg_rd;
>> +      record_buf[1] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 2;
>> +    }
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Generic handler for thumb2 instructions which effect destination and PS
>> +   registers.  */
>> +
>> +static int
>> +thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t reg_rd;
>> +  uint32_t record_buf[8];
>> +
>> +  reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +
>> +  record_buf[0] = reg_rd;
>> +  record_buf[1] = ARM_PS_REGNUM;
>> +  thumb2_insn_r->reg_rec_count = 2;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 branch and miscellaneous control instructions.  */
>> +
>> +static int
>> +thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t op, op1, op2;
>> +  uint32_t record_buf[8];
>> +
>> +  op = bits (thumb2_insn_r->arm_insn, 20, 26);
>> +  op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
>> +  op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
>> +
>> +  /* Handle MSR insn.  */
>> +  if (!(op1 & 0x2) && 0x38 == op)
>> +    {
>> +      if (!(op2 & 0x3))
>> +        {
>> +          /* CPSR is going to be changed.  */
>> +          record_buf[0] = ARM_PS_REGNUM;
>> +          thumb2_insn_r->reg_rec_count = 1;
>> +        }
>> +      else
>> +        {
>> +          arm_record_unsupported_insn(thumb2_insn_r);
>> +          return -1;
>> +        }
>> +    }
>> +  else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
>> +    {
>> +      /* BLX.  */
>> +      record_buf[0] = ARM_PS_REGNUM;
>> +      record_buf[1] = ARM_LR_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 2;
>> +    }
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 store single data item instructions.  */
>> +
>> +static int
>> +thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
>> +{
>> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
>> +
>> +  uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
>> +  uint32_t address, offset_addr;
>> +  uint32_t record_buf[8], record_buf_mem[8];
>> +  uint32_t op1, op2;
>> +
>> +  ULONGEST u_regval[2];
>> +
>> +  op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
>> +  op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +  regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
>> +
>> +  if (bit (thumb2_insn_r->arm_insn, 23))
>> +    {
>> +      /* T2 encoding.  */
>> +      offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
>> +      offset_addr = u_regval[0] + offset_imm;
>> +      address = offset_addr;
>> +    }
>> +  else
>> +    {
>> +      /* T3 encoding.  */
>> +      if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
>> +        {
>> +          /* Handle STRB (register).  */
>> +          reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
>> +          regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
>> +          shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
>> +          offset_addr = u_regval[1] << shift_imm;
>> +          address = u_regval[0] + offset_addr;
>> +        }
>> +      else
>> +        {
>> +          offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
>> +          if (bit (thumb2_insn_r->arm_insn, 10))
>> +            {
>> +              if (bit (thumb2_insn_r->arm_insn, 9))
>> +                offset_addr = u_regval[0] + offset_imm;
>> +              else
>> +                offset_addr = u_regval[0] - offset_imm;
>> +
>> +              address = offset_addr;
>> +            }
>> +          else
>> +            address = u_regval[0];
>> +        }
>> +    }
>> +
>> +  switch (op1)
>> +    {
>> +      /* Store byte instructions.  */
>> +      case 4:
>> +      case 0:
>> +        record_buf_mem[0] = 1;
>> +      break;
>> +      /* Store half word instructions.  */
>> +      case 1:
>> +      case 5:
>> +        record_buf_mem[0] = 2;
>> +      break;
>> +      /* Store word instructions.  */
>> +      case 2:
>> +      case 6:
>> +        record_buf_mem[0] = 4;
>> +      break;
>> +
>> +      default:
>> +        gdb_assert_not_reached ("no decoding pattern found");
>> +      break;
>> +    }
>> +
>> +  record_buf_mem[1] = address;
>> +  thumb2_insn_r->mem_rec_count = 1;
>> +  record_buf[0] = reg_rn;
>> +  thumb2_insn_r->reg_rec_count = 1;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
>> +            record_buf_mem);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 load memory hints instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t record_buf[8];
>> +  uint32_t reg_rt, reg_rn;
>> +
>> +  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +
>> +  if (ARM_PC_REGNUM != reg_rt)
>> +    {
>> +      record_buf[0] = reg_rt;
>> +      record_buf[1] = reg_rn;
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +
>> +      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +                record_buf);
>> +      return 0;
>> +    }
>> +
>> +  return -1;
>> +}
>> +
>> +/* Handler for thumb2 load word instructions.  */
>> +
>> +static int
>> +thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t opcode1 = 0, opcode2 = 0;
>> +  uint32_t record_buf[8];
>> +
>> +  record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +  record_buf[1] = ARM_PS_REGNUM;
>> +  thumb2_insn_r->reg_rec_count = 2;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return 0;
>> +}
>> +
>> +/* Handler for thumb2 long multiply, long multiply accumulate, and
>> +   divide instructions.  Return value: -1:record failure ;  0:success.  */
>> +
>> +static int
>> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
>> +{
>> +  uint32_t ret = 0;
>> +  uint32_t opcode1 = 0, opcode2 = 0;
>> +  uint32_t record_buf[8];
>> +  uint32_t reg_src1 = 0;
>> +
>> +  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
>> +  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
>> +
>> +  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
>> +    {
>> +      /* Handle SMULL, UMULL, SMULAL.  */
>> +      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
>> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +    }
>> +  else if (1 == opcode1 || 3 == opcode2)
>> +    {
>> +      /* Handle SDIV and UDIV.  */
>> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
>> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
>> +      record_buf[2] = ARM_PS_REGNUM;
>> +      thumb2_insn_r->reg_rec_count = 3;
>> +    }
>> +  else
>> +    ret = -1;
>> +
>> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
>> +            record_buf);
>> +  return ret;
>> +}
>> +
>> +/* Decodes thumb2 instruction type and return an instruction id.  */
>> +
>> +static unsigned int
>> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
>> +{
>> +  uint32_t op = 0;
>> +  uint32_t op1 = 0;
>> +  uint32_t op2 = 0;
>> +
>> +  op = bit (thumb2_insn, 15);
>> +  op1 = bits (thumb2_insn, 27, 28);
>> +  op2 = bits (thumb2_insn, 20, 26);
>> +
>> +  if (op1 == 0x01)
>> +    {
>> +      if (!(op2 & 0x64 ))
>> +        {
>> +          /* Load/store multiple instruction.  */
>> +          return 0;
>> +        }
>> +      else if (!((op2 & 0x64) ^ 0x04))
>> +        {
>> +          /* Load/store (dual/exclusive) and table branch instruction.  */
>> +          return 1;
>> +        }
>> +      else if (!((op2 & 0x20) ^ 0x20))
>> +        {
>> +          /* Data-processing (shifted register).  */
>> +          return 2;
>> +        }
>> +      else if (op2 & 0x40)
>> +        {
>> +          /* Co-processor instructions.  */
>> +          return 3;
>> +        }
>> +    }
>> +  else if (op1 == 0x02)
>> +    {
>> +      if (op)
>> +        {
>> +          /* Branches and miscellaneous control instructions.  */
>> +          return 6;
>> +        }
>> +      else if (op2 & 0x20)
>> +        {
>> +          /* Data-processing (plain binary immediate) instruction.  */
>> +          return 5;
>> +        }
>> +      else
>> +        {
>> +          /* Data-processing (modified immediate).  */
>> +          return 4;
>> +        }
>> +    }
>> +  else if (op1 == 0x03)
>> +    {
>> +      if (!(op2 & 0x71 ))
>> +        {
>> +          /* Store single data item.  */
>> +          return 7;
>> +        }
>> +      else if (!((op2 & 0x71) ^ 0x10))
>> +        {
>> +          /* Advanced SIMD or structure load/store instructions.  */
>> +          return 8;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x01))
>> +        {
>> +          /* Load byte, memory hints instruction.  */
>> +          return 9;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x03))
>> +        {
>> +          /* Load halfword, memory hints instruction.  */
>> +          return 10;
>> +        }
>> +      else if (!((op2 & 0x67) ^ 0x05))
>> +        {
>> +          /* Load word instruction.  */
>> +          return 11;
>> +        }
>> +      else if (!((op2 & 0x70) ^ 0x20))
>> +        {
>> +          /* Data-processing (register) instruction.  */
>> +          return 12;
>> +        }
>> +      else if (!((op2 & 0x78) ^ 0x30))
>> +        {
>> +          /* Multiply, multiply accumulate, abs diff instruction.  */
>> +          return 13;
>> +        }
>> +      else if (!((op2 & 0x78) ^ 0x38))
>> +        {
>> +          /* Long multiply, long multiply accumulate, and divide.  */
>> +          return 14;
>> +        }
>> +      else if (op2 & 0x40)
>> +        {
>> +          /* Co-processor instructions.  */
>> +          return 15;
>> +        }
>> +   }
>> +
>> +  return -1;
>> +}
>>
>>  /* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0 on success
>>  and positive val on fauilure.  */
>> @@ -12452,7 +13030,7 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>>      arm_record_ld_st_reg_offset,        /* 011.  */
>>      arm_record_ld_st_multiple,          /* 100.  */
>>      arm_record_b_bl,                    /* 101.  */
>> -    arm_record_coproc,                  /* 110.  */
>> +    arm_record_unsupported_insn,                  /* 110.  */
>>      arm_record_coproc_data_proc         /* 111.  */
>>    };
>>
>> @@ -12469,6 +13047,28 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>>      thumb_record_branch                /* 111.  */
>>    };
>>
>> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
>> +     instruction.  */
>> +  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
>> +  { \
>> +    thumb2_record_ld_st_multiple,       /* 00.  */
>> +    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
>> +    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
>> +    arm_record_unsupported_insn,        /* 03.  */
>> +    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
>> +    thumb2_record_ps_dest_generic,      /* 05.  */
>> +    thumb2_record_branch_misc_cntrl,    /* 06.  */
>> +    thumb2_record_str_single_data,      /* 07.  */
>> +    arm_record_unsupported_insn,        /* 08.  */
>> +    thumb2_record_ld_mem_hints,         /* 09.  */
>> +    thumb2_record_ld_mem_hints,         /* 10.  */
>> +    thumb2_record_ld_word,              /* 11.  */
>> +    thumb2_record_ps_dest_generic,      /* 12.  */
>> +    thumb2_record_ps_dest_generic,      /* 13.  */
>> +    thumb2_record_lmul_lmla_div,        /* 14.  */
>> +    arm_record_unsupported_insn         /* 15.  */
>> +  };
>> +
>>    uint32_t ret = 0;    /* return value: negative:failure   0:success.  */
>>    uint32_t insn_id = 0;
>>
>> @@ -12503,11 +13103,22 @@ decode_insn (insn_decode_record *arm_record, record_type_t record_type,
>>      }
>>    else if (THUMB2_RECORD == record_type)
>>      {
>> -      printf_unfiltered (_("Process record doesnt support thumb32 instruction "
>> -                           "0x%0x at address %s.\n"),arm_record->arm_insn,
>> -                           paddress (arm_record->gdbarch,
>> -                           arm_record->this_addr));
>> -      ret = -1;
>> +      /* As thumb does not have condition codes, we set negative.  */
>> +      arm_record->cond = -1;
>> +
>> +      /* Swap first half of 32bit thumb instruction with second half.  */
>> +      arm_record->arm_insn = (arm_record->arm_insn >> 16) |
>> +                             (arm_record->arm_insn << 16);
>> +
>> +      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
>> +
>> +      if (insn_id >= 0)
>> +        ret = thumb2_handle_insn[insn_id] (arm_record);
>> +      else
>> +        {
>> +          arm_record_unsupported_insn(arm_record);
>> +          ret = -1;
>> +        }
>>      }
>>    else
>>      {
>> --
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-27 23:58           ` Omair Javaid
@ 2013-11-28 12:30             ` Yao Qi
  2013-12-17 10:22               ` Omair Javaid
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2013-11-28 12:30 UTC (permalink / raw)
  To: Omair Javaid; +Cc: Oza Pawandeep, gdb-patches, Patch Tracking

On 11/28/2013 06:04 AM, Omair Javaid wrote:
>> gdb is for user space; and use space is not allowed to use SPSR
>> >directly using MSR instruction.
>> >so old code base + new code base whereever we have got SPSR getting
>> >modified we need to remove the same.
>> >
> I agree with you that we have to do a lot of rework of previous process record code. But I am not sure it would be productive for us to get working code out and loose the functionality or delay its submission. As Record/Replay is pretty much functional with this set of patches I am hoping that we can do a complete rework if required later on.
>

If there is something broken related to the submissions, the common 
practise could be one of them below:

  1. Fix them first,
  2. Document the existing limitations/problems or write a test case to 
kfail it.  In this way, people are aware of this.
  3. Continue the submission and revisit the problems later.

Personally, I choose one of them, depending on the complexity of fixing 
the existing problems.  This patch series is a large one, and based on 
some existing code.  I would like to fix existing problems first, before 
doing something new, if it doesn't take much time on fixing existing 
problems.  These patches are preparatory, and usually simple.  so they 
have more chances to be reviewed in a timely manner.  These preparatory 
patches set up a context for your large patch series, and the context is 
helpful to understanding your patch series.  As a result, the review 
process may be shortened.

I don't want you to fix existing problems first, or take other actions.
Just let you know something submitters can do to help maintainers to 
approve patches.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-28 12:30             ` Yao Qi
@ 2013-12-17 10:22               ` Omair Javaid
  0 siblings, 0 replies; 10+ messages in thread
From: Omair Javaid @ 2013-12-17 10:22 UTC (permalink / raw)
  To: Yao Qi; +Cc: Oza Pawandeep, gdb-patches, Patch Tracking

On Thu 28 Nov 2013 05:19:15 PM PKT, Yao Qi wrote:
> On 11/28/2013 06:04 AM, Omair Javaid wrote:
>>> gdb is for user space; and use space is not allowed to use SPSR
>>>> directly using MSR instruction.
>>>> so old code base + new code base whereever we have got SPSR getting
>>>> modified we need to remove the same.
>>>>
>> I agree with you that we have to do a lot of rework of previous process record code. But I am not sure it would be productive for us to get working code out and loose the functionality or delay its submission. As Record/Replay is pretty much functional with this set of patches I am hoping that we can do a complete rework if required later on.
>>
>
> If there is something broken related to the submissions, the common
> practise could be one of them below:
>
>   1. Fix them first,
>   2. Document the existing limitations/problems or write a test case to
> kfail it.  In this way, people are aware of this.
>   3. Continue the submission and revisit the problems later.
>
> Personally, I choose one of them, depending on the complexity of fixing
> the existing problems.  This patch series is a large one, and based on
> some existing code.  I would like to fix existing problems first, before
> doing something new, if it doesn't take much time on fixing existing
> problems.  These patches are preparatory, and usually simple.  so they
> have more chances to be reviewed in a timely manner.  These preparatory
> patches set up a context for your large patch series, and the context is
> helpful to understanding your patch series.  As a result, the review
> process may be shortened.
>
> I don't want you to fix existing problems first, or take other actions.
> Just let you know something submitters can do to help maintainers to
> approve patches.
>

Ping! Looking for maintainer's approval for arm process record/replay 
improvement patches.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
  2013-11-25  0:05       ` Omair Javaid
  2013-11-25 14:23         ` Oza Pawandeep
@ 2013-12-20 12:37         ` Pedro Alves
  1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2013-12-20 12:37 UTC (permalink / raw)
  To: Omair Javaid; +Cc: Yao Qi, gdb-patches, Patch Tracking

On 11/24/2013 11:49 PM, Omair Javaid wrote:

> Patch has been updated after incorporating suggestions. Looking for a final
> commit verdict.
> 
> This patch adds support for recording thumb32 instructions. This patch also
> fixes bugs in arm process record instruction decoding.

It's not clear what this "also fixes bugs" refers to.  Could that part be
something that could be split into it's own patch?

> gdb:
> 
> 2013-11-08  Omair Javaid  <omair.javaid@linaro.org>
> 
> 	* arm-tdep.c (struct arm_mem_r): Use uint32_t for memory address.
> 	(arm_record_unsupported_insn): New function.
> 	(arm_record_coproc_data_proc): Removed.
> 	(thumb2_record_ld_st_multiple): New function.
> 	(thumb2_record_ld_st_dual_ex_tbb): New function.
> 	(thumb2_record_data_proc_sreg_mimm): New function.
> 	(thumb2_record_ps_dest_generic): New function.
> 	(thumb2_record_branch_misc_cntrl): New function.
> 	(thumb2_record_str_single_data): New function.
> 	(thumb2_record_ld_mem_hints): New function.
> 	(thumb2_record_ld_word): New function.
> 	(thumb2_record_lmul_lmla_div): New function.
> 	(thumb2_record_decode_inst_id): New function.
> 	(decode_insn): Add thumb32 instruction handlers.
> 


> +/* Handler for thumb2 load/store multiple instructions.  */
> +
> +static int
> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
> +{
> +  struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> +  uint32_t reg_rn, op;
> +  uint32_t register_bits = 0, register_count = 0;
> +  uint32_t index = 0, start_address = 0;
> +  uint32_t record_buf[24], record_buf_mem[48];
> +
> +  ULONGEST u_regval = 0;
> +
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +  op = bits (thumb2_insn_r->arm_insn, 23, 24);
> +
> +  if (0 == op || 3 == op)
> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle RFE instruction.  */
> +          record_buf[0] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 1;
> +        }
> +      else
> +        {
> +          /* Handle SRS instruction after reading banked SP.  */
> +          return arm_record_unsupported_insn (thumb2_insn_r);
> +        }
> +    }
> +  else if(1 == op || 2 == op)

Missing space before parens.

> +    {
> +      if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> +        {
> +          /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                record_buf[index++] = register_count;
> +
> +              register_count++;
> +              register_bits = register_bits >> 1;
> +            }
> +          record_buf[index++] = reg_rn;
> +          record_buf[index++] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = index;
> +        }
> +      else
> +        {
> +          /* Handle STM/STMIA/STMEA and STMDB/STMFD.  */
> +          register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> +          regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> +          while (register_bits)
> +            {
> +              if (register_bits & 0x00000001)
> +                register_count++;
> +
> +              register_bits = register_bits >> 1;
> +            }
> +
> +          if (1 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = u_regval;
> +            }
> +          else if (2 == op)
> +            {
> +              /* Start address calculation for LDMDB/LDMEA.  */
> +              start_address = (u_regval) - (register_count * 4);

Unnecessary parens.

> +            }
> +
> +          thumb2_insn_r->mem_rec_count = register_count;
> +          while (register_count)

          while (register_count > 0)

> +            {
> +              record_buf_mem[(register_count * 2) - 1] = start_address;
> +              record_buf_mem[(register_count * 2) - 2] = 4;

Unnecessary parens.

> +              start_address = start_address + 4;
> +              register_count--;
> +            }
> +          record_buf[0] = reg_rn;
> +          record_buf[1] = ARM_PS_REGNUM;
> +          thumb2_insn_r->reg_rec_count = 2;
> +        }
> +    }
> +
> +  MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> +            record_buf_mem);
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return 0;
> +}
> +



> +/* Handler for thumb2 load memory hints instructions.  */
> +
> +static int
> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t record_buf[8];
> +  uint32_t reg_rt, reg_rn;
> +
> +  reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
> +  reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +
> +  if (ARM_PC_REGNUM != reg_rt)
> +    {
> +      record_buf[0] = reg_rt;
> +      record_buf[1] = reg_rn;
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +
> +      REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +                record_buf);
> +      return 0;
> +    }
> +
> +  return -1;
> +}
> +

> +/* Handler for thumb2 long multiply, long multiply accumulate, and
> +   divide instructions.  Return value: -1:record failure ;  0:success.  */

"Returns -1 on failure, 0 on success."

Most of the other functions lack describing the return.  Please update
them too.

> +
> +static int
> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
> +{
> +  uint32_t ret = 0;
> +  uint32_t opcode1 = 0, opcode2 = 0;
> +  uint32_t record_buf[8];
> +  uint32_t reg_src1 = 0;
> +
> +  opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
> +  opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
> +
> +  if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
> +    {
> +      /* Handle SMULL, UMULL, SMULAL.  */
> +      /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S).  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else if (1 == opcode1 || 3 == opcode2)
> +    {
> +      /* Handle SDIV and UDIV.  */
> +      record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> +      record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> +      record_buf[2] = ARM_PS_REGNUM;
> +      thumb2_insn_r->reg_rec_count = 3;
> +    }
> +  else
> +    ret = -1;
> +
> +  REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> +            record_buf);
> +  return ret;
> +}
> +
> +/* Decodes thumb2 instruction type and return an instruction id.  */

Mention also the -1 on failure.  But, where are these instruction ids
described?  Can this be an enum?

> +
> +static unsigned int
> +thumb2_record_decode_inst_id (uint32_t thumb2_insn)
> +{

> +  return -1;
> +}
>  

> +  /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
> +     instruction.  */
> +  static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
> +  { \

No need for that \.

> +    thumb2_record_ld_st_multiple,       /* 00.  */
> +    thumb2_record_ld_st_dual_ex_tbb,    /* 01.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 02.  */
> +    arm_record_unsupported_insn,        /* 03.  */
> +    thumb2_record_data_proc_sreg_mimm,  /* 04.  */
> +    thumb2_record_ps_dest_generic,      /* 05.  */
> +    thumb2_record_branch_misc_cntrl,    /* 06.  */
> +    thumb2_record_str_single_data,      /* 07.  */
> +    arm_record_unsupported_insn,        /* 08.  */
> +    thumb2_record_ld_mem_hints,         /* 09.  */
> +    thumb2_record_ld_mem_hints,         /* 10.  */
> +    thumb2_record_ld_word,              /* 11.  */
> +    thumb2_record_ps_dest_generic,      /* 12.  */
> +    thumb2_record_ps_dest_generic,      /* 13.  */
> +    thumb2_record_lmul_lmla_div,        /* 14.  */
> +    arm_record_unsupported_insn         /* 15.  */
> +  };

So, that number was an index into this table?  Why not simply
have thumb2_record_decode_inst_id return a sti_arm_hdl_fp_t directly?
Or better, just have thumb2_record_decode_inst_id call the function
directly (renamed to thumb2_record_handle_insn)?  AFAICS, this
is only used in one place:

> +
> +      insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
> +
> +      if (insn_id >= 0)
> +        ret = thumb2_handle_insn[insn_id] (arm_record);
> +      else
> +        {
> +          arm_record_unsupported_insn(arm_record);
> +          ret = -1;
> +        }

That could then just be:

 	 ret = thumb2_record_handle_insn (arm_record->arm_insn);

BTW, "insn" is much more common in GDB as short for instruction
than inst.

> +
>    uint32_t ret = 0;    /* return value: negative:failure   0:success.  */

Please use full sentences instead of that ':' syntax.  Document the
return code in the function's intro instead, and then you don't need
to document it here.  A variable called 'ret' becomes self describing.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-12-20 12:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-24  0:09 [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux* Omair Javaid
2013-10-24  2:25 ` Yao Qi
2013-11-08  3:20   ` Omair Javaid
2013-11-11 10:53     ` Yao Qi
2013-11-25  0:05       ` Omair Javaid
2013-11-25 14:23         ` Oza Pawandeep
2013-11-27 23:58           ` Omair Javaid
2013-11-28 12:30             ` Yao Qi
2013-12-17 10:22               ` Omair Javaid
2013-12-20 12:37         ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).