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]

[PATCH, v2] Fix ARM machine state testcase failures


Hi,

This is v2 of the original patch. Compared to v1, i've taken Yao's suggestion and dropped special handling of push/pop instructions.

While at it, i noticed the original functions had some issues like unused variables and broken identation, so i went ahead and completely rewrote the functions handling ARM mode opcodes 010 and 100.

I see the same results as before, with no regressions.

How does it look now?

Luis
2014-10-21  Luis Machado  <lgustavo@codesourcery.com>

	* arm-tdep.c (INSN_S_L_BIT_NUM): Document.
	(arm_record_ld_st_imm_offset): Reimplement to cover all
	load/store cases for ARM opcode 010.
	(arm_record_ld_st_multiple): Reimplement to cover all
	load/store cases for ARM opcode 100.

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index e2559ec..3a75d3f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10676,6 +10676,8 @@ vfp - VFP co-processor."),
 #define THUMB2_INSN_SIZE_BYTES 4
 
 
+/* Position of the bit within a 32-bit ARM instruction
+   that defines whether the instruction is a load or store.  */
 #define INSN_S_L_BIT_NUM 20
 
 #define REG_ALLOC(REGS, LENGTH, RECORD_BUF) \
@@ -11473,110 +11475,94 @@ arm_record_data_proc_imm (insn_decode_record *arm_insn_r)
   return 0;
 }
 
-/* Handling opcode 010 insns.  */
+/* Handle ARM mode instructions with opcode 010.  */
 
 static int
 arm_record_ld_st_imm_offset (insn_decode_record *arm_insn_r)
 {
   struct regcache *reg_cache = arm_insn_r->regcache;
 
-  uint32_t reg_src1 = 0 , reg_dest = 0;
-  uint32_t offset_12 = 0, tgt_mem_addr = 0;
+  uint32_t reg_base , reg_dest;
+  uint32_t offset_12, tgt_mem_addr;
   uint32_t record_buf[8], record_buf_mem[8];
+  unsigned char wback;
+  ULONGEST u_regval;
 
-  ULONGEST u_regval = 0;
+  /* Calculate wback.  */
+  wback = (bit (arm_insn_r->arm_insn, 24) == 0)
+	  || (bit (arm_insn_r->arm_insn, 21) == 1);
 
-  arm_insn_r->opcode = bits (arm_insn_r->arm_insn, 21, 24);
-  arm_insn_r->decode = bits (arm_insn_r->arm_insn, 4, 7);
+  arm_insn_r->reg_rec_count = 0;
+  reg_base = bits (arm_insn_r->arm_insn, 16, 19);
 
   if (bit (arm_insn_r->arm_insn, INSN_S_L_BIT_NUM))
     {
+      /* LDR (immediate), LDR (literal), LDRB (immediate), LDRB (literal), LDRBT
+	 and LDRT.  */
+
       reg_dest = bits (arm_insn_r->arm_insn, 12, 15);
-      /* LDR insn has a capability to do branching, if
-         MOV LR, PC is precedded by LDR insn having Rn as R15
-         in that case, it emulates branch and link insn, and hence we
-         need to save CSPR and PC as well.  */
-      if (ARM_PC_REGNUM != reg_dest)
-        {
-          record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
-          arm_insn_r->reg_rec_count = 1;
-        }
-      else
-        {
-          record_buf[0] = reg_dest;
-          record_buf[1] = ARM_PS_REGNUM;
-          arm_insn_r->reg_rec_count = 2;
-        }
+      record_buf[arm_insn_r->reg_rec_count++] = reg_dest;
+
+      /* The LDR instruction is capable of doing branching.  If MOV LR, PC
+	 preceeds a LDR instruction having R15 as reg_base, it
+	 emulates a branch and link instruction, and hence we need to save
+	 CPSR and PC as well.  */
+      if (ARM_PC_REGNUM == reg_dest)
+	record_buf[arm_insn_r->reg_rec_count++] = ARM_PS_REGNUM;
+
+      /* If wback is true, also save the base register, which is going to be
+	 written to.  */
+      if (wback)
+	record_buf[arm_insn_r->reg_rec_count++] = reg_base;
     }
   else
     {
-      /* Store, immediate offset, immediate pre-indexed,
-         immediate post-indexed.  */
-      reg_src1 = bits (arm_insn_r->arm_insn, 16, 19);
+      /* STR (immediate), STRB (immediate), STRBT and STRT.  */
+
       offset_12 = bits (arm_insn_r->arm_insn, 0, 11);
-      regcache_raw_read_unsigned (reg_cache, reg_src1, &u_regval);
-      /* U == 1 */
+      regcache_raw_read_unsigned (reg_cache, reg_base, &u_regval);
+
+      /* Handle bit U.  */
       if (bit (arm_insn_r->arm_insn, 23))
-        {
-          tgt_mem_addr = u_regval + offset_12;
-        }
+	{
+	  /* U == 1: Add the offset. */
+	  tgt_mem_addr = (uint32_t) u_regval + offset_12;
+	}
       else
-        {
-          tgt_mem_addr = u_regval - offset_12;
-        }
+	{
+	  /* U == 0: subtract the offset. */
+	  tgt_mem_addr = (uint32_t) u_regval - offset_12;
+	}
+
+      /* Bit 22 tells us whether the store instruction writes 1 byte or 4
+	 bytes.  */
+      if (bit (arm_insn_r->arm_insn, 22))
+	{
+	  /* STRB and STRBT: 1 byte.  */
+	  record_buf_mem[0] = 1;
+	}
+      else
+	{
+	  /* STR and STRT: 4 bytes.  */
+	  record_buf_mem[0] = 4;
+	}
+
+      /* Handle bit P.  */
+      if (bit (arm_insn_r->arm_insn, 24))
+	{
+	  record_buf_mem[1] = tgt_mem_addr;
+	}
+      else
+	{
+	   record_buf_mem[1] = (uint32_t) u_regval;
+	}
 
-      switch (arm_insn_r->opcode)
-        {
-          /* STR.  */
-          case 8:
-          case 12:
-          /* STR.  */
-          case 9:
-          case 13:
-          /* STRT.  */    
-          case 1:
-          case 5:
-          /* STR.  */    
-          case 4:
-          case 0:
-            record_buf_mem[0] = 4;
-          break;
-
-          /* STRB.  */
-          case 10:
-          case 14:
-          /* STRB.  */    
-          case 11:
-          case 15:
-          /* STRBT.  */    
-          case 3:
-          case 7:
-          /* STRB.  */    
-          case 2:
-          case 6:
-            record_buf_mem[0] = 1;
-          break;
-
-          default:
-            gdb_assert_not_reached ("no decoding pattern found");
-          break;
-        }
-      record_buf_mem[1] = tgt_mem_addr;
       arm_insn_r->mem_rec_count = 1;
 
-      if (9 == arm_insn_r->opcode || 11 == arm_insn_r->opcode
-          || 13 == arm_insn_r->opcode || 15 == arm_insn_r->opcode
-          || 0 == arm_insn_r->opcode || 2 == arm_insn_r->opcode
-          || 4 == arm_insn_r->opcode || 6 == arm_insn_r->opcode
-          || 1 == arm_insn_r->opcode || 3 == arm_insn_r->opcode
-          || 5 == arm_insn_r->opcode || 7 == arm_insn_r->opcode
-         )
-        {
-          /* We are handling pre-indexed mode; post-indexed mode;
-             where Rn is going to be changed.  */
-          record_buf[0] = reg_src1;
-          arm_insn_r->reg_rec_count = 1;
-        }
+      /* If wback is true, also save the base register, which is going to be
+	 written to.  */
+      if (wback)
+	record_buf[arm_insn_r->reg_rec_count++] = reg_base;
     }
 
   REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);
@@ -11847,134 +11833,95 @@ arm_record_ld_st_reg_offset (insn_decode_record *arm_insn_r)
   return 0;
 }
 
-/* Handling opcode 100 insns.  */
+/* Handle ARM mode instructions with opcode 100.  */
 
 static int
 arm_record_ld_st_multiple (insn_decode_record *arm_insn_r)
 {
   struct regcache *reg_cache = arm_insn_r->regcache;
-
-  uint32_t register_list[16] = {0}, register_count = 0, register_bits = 0;
-  uint32_t reg_src1 = 0, addr_mode = 0, no_of_regs = 0;
-  uint32_t start_address = 0, index = 0;
+  uint32_t register_count = 0, register_bits;
+  uint32_t reg_base, addr_mode;
   uint32_t record_buf[24], record_buf_mem[48];
+  uint32_t wback;
+  ULONGEST u_regval;
 
-  ULONGEST u_regval[2] = {0};
+  /* Fetch the list of registers.  */
+  register_bits = bits (arm_insn_r->arm_insn, 0, 15);
+  arm_insn_r->reg_rec_count = 0;
+
+  /* Fetch the base register that contains the address we are loading data
+     to.  */
+  reg_base = bits (arm_insn_r->arm_insn, 16, 19);
 
-  /* This mode is exclusively for load and store multiple.  */
-  /* Handle incremenrt after/before and decrment after.before mode;
-     Rn is changing depending on W bit, but as of now we store Rn too
-     without optimization.  */
+  /* Calculate wback.  */
+  wback = (bit (arm_insn_r->arm_insn, 24) == 0)
+	  || (bit (arm_insn_r->arm_insn, 21) == 1);
 
   if (bit (arm_insn_r->arm_insn, INSN_S_L_BIT_NUM))
     {
-      /* LDM  (1,2,3) where LDM  (3) changes CPSR too.  */
+      /* LDM/LDMIA/LDMFD, LDMDA/LDMFA, LDMDB and LDMIB.  */
 
-      if (bit (arm_insn_r->arm_insn, 20) && !bit (arm_insn_r->arm_insn, 22))
-        {
-          register_bits = bits (arm_insn_r->arm_insn, 0, 15);
-          no_of_regs = 15;
-        }
-      else
-        {
-          register_bits = bits (arm_insn_r->arm_insn, 0, 14);
-          no_of_regs = 14;
-        }
-      /* Get Rn.  */
-      reg_src1 = bits (arm_insn_r->arm_insn, 16, 19);
+      /* Find out which registers are going to be loaded from memory.  */
       while (register_bits)
-      {
-        if (register_bits & 0x00000001)
-          record_buf[index++] = register_count;
-        register_bits = register_bits >> 1;
-        register_count++;
-      }
+	{
+	  if (register_bits & 0x00000001)
+	    record_buf[arm_insn_r->reg_rec_count++] = register_count;
+	  register_bits = register_bits >> 1;
+	  register_count++;
+	}
 
-        /* Extra space for Base Register and CPSR; wihtout optimization.  */
-        record_buf[index++] = reg_src1;
-        record_buf[index++] = ARM_PS_REGNUM;
-        arm_insn_r->reg_rec_count = index;
+      /* Extra space for Base Register and CPSR; wihtout optimization.  */
+      record_buf[arm_insn_r->reg_rec_count++] = reg_base;
+      record_buf[arm_insn_r->reg_rec_count++] = ARM_PS_REGNUM;
     }
   else
     {
-      /* It handles both STM(1) and STM(2).  */
-      addr_mode = bits (arm_insn_r->arm_insn, 23, 24);    
+      /* STM (STMIA, STMEA), STMDA (STMED), STMDB (STMFD) and STMIB (STMFA).  */
 
-      register_bits = bits (arm_insn_r->arm_insn, 0, 15);
-      /* Get Rn.  */
-      reg_src1 = bits (arm_insn_r->arm_insn, 16, 19);
-      regcache_raw_read_unsigned (reg_cache, reg_src1, &u_regval[0]);
+      addr_mode = bits (arm_insn_r->arm_insn, 23, 24); 
+
+      regcache_raw_read_unsigned (reg_cache, reg_base, &u_regval);
+
+      /* Find out how many registers are going to be stored to memory.  */
       while (register_bits)
-        {
-          if (register_bits & 0x00000001)
-            register_count++;
-          register_bits = register_bits >> 1;
-        }
+	{
+	  if (register_bits & 0x00000001)
+	    register_count++;
+	  register_bits = register_bits >> 1;
+	}
 
       switch (addr_mode)
-        {
-          /* Decrement after.  */
-          case 0:                          
-            start_address = (u_regval[0]) - (register_count * 4) + 4;
-            arm_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--;
-              }
-          break;    
-
-          /* Increment after.  */
-          case 1:
-            start_address = u_regval[0];
-            arm_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--;
-              }
-          break;    
-
-          /* Decrement before.  */
-          case 2:
-
-            start_address = (u_regval[0]) - (register_count * 4);
-            arm_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--;
-              }
-          break;    
-
-          /* Increment before.  */
-          case 3:
-            start_address = u_regval[0] + 4;
-            arm_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--;
-              }
-          break;    
-
-          default:
-            gdb_assert_not_reached ("no decoding pattern found");
-          break;    
-        }
+	{
+	  /* STMDA (STMED): Decrement after.  */
+	  case 0:
+	  record_buf_mem[1] = (uint32_t) u_regval
+			      - register_count * INT_REGISTER_SIZE + 4;
+	  break;
+	  /* STM (STMIA, STMEA): Increment after.  */
+	  case 1:
+	  record_buf_mem[1] = (uint32_t) u_regval;
+	  break;
+	  /* STMDB (STMFD): Decrement before.  */
+	  case 2:
+	  record_buf_mem[1] = (uint32_t) u_regval
+			      - register_count * INT_REGISTER_SIZE;
+	  break;
+	  /* STMIB (STMFA): Increment before.  */
+	  case 3:
+	  record_buf_mem[1] = (uint32_t) u_regval + INT_REGISTER_SIZE;
+	  break;
+	  default:
+	    gdb_assert_not_reached ("no decoding pattern found");
+	  break;
+	}
 
-      /* Base register also changes; based on condition and W bit.  */
-      /* We save it anyway without optimization.  */
-      record_buf[0] = reg_src1;
-      arm_insn_r->reg_rec_count = 1;
+      record_buf_mem[0] = register_count * INT_REGISTER_SIZE;
+      arm_insn_r->mem_rec_count = 1;
+
+      /* If wback is true, also save the base register, which is going to be
+	 written to.  */
+      if (wback)
+	record_buf[arm_insn_r->reg_rec_count++] = reg_base;
     }
 
   REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);

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