This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


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

Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*


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 (éå)


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