[RFC] mips-tdep.c: Fix mips16 bit rot

Kevin Buettner kevinb@redhat.com
Tue Dec 14 00:06:00 GMT 2010


In the course of some recent mips testing, I noticed that GDB's mips16
support has been non-functional for quite a while.

A change which occurred between "2007-01-21 09:49" and
"2007-01-21 09:50" (use those strings as arguments to cvs's -D
switches...) caused all line number information in mips16 programs to
be marked using the mips16 convention of setting the low bit of the
address.  Prior to that patch, the behavior was mixed - function entry
points would have the mips16 bit stripped, whereas other lines would
have it in place.  This change caused a search for a function, e.g
main(), without the mips16 bit set, to be considered as part of
whatever function preceded main().  As a consequence,
skip_prologue_using_sal() would return the address of the first
instruction in main() rather than the address just past the prologue.

(I don't see anything wrong with the patch that was committed in Jan
2007, and, quite frankly, I'm surprised that it had this affect on
mips16 behavior at all!)

Note that gdbarch_addr_bits_remove() is invoked in
dwarf_decode_lines() when determining the address associated with a
line.  The fact that mips_addr_bits_remove(), in the patch below,
strips the low bit of the address is what causes the address to now be
stored in the line table without the low bit.

The changes to mips_read_pc() and mips_unwind_pc() were needed to
correct potential mismatches between addresses that GDB stores
internally and actual PC values.  (My recollection is that when GDB
was stopping at a breakpoint, it was comparing an non-mips16 address
with a mips16 address and then delivering a message to the user about
a SIGTRAP occurring.  (They didn't compare as equal due to being off
by one bit.)

And, likewise, when writing the PC, or when writing a function
argument for an inferior call, we must take care to set the mips16
address bit as needed.  If we do not do so, the processor attempts to
execute mips16 code as ordinary mips code, with predictably disastrous
results.

In the course of making these changes, I studied what arm-tdep.c
does for THUMB mode and made analogous changes to mips-tdep.c.

I've tested this patch using various mips16 multilibs and it vastly
improves the test results.  (There are roughly 1600 fewer failures
per multilib.)

Comments?

	* mips-tdep.c (make_mips16_addr): New function.
	(mips_elf_make_msymbol_special): Don't set the low bit in the
	symbol's address.
	(mips_read_pc, mips_unwind_pc, mips_addr_bits_remove): Strip bit
	indicating mips16 address, if present.
	(mips_write_pc): Set bit indicating mips16 address when in a mips16
	function.
	(mips_eabi_push_dummy_call, mips_o64_push_dummy_call): Likewise,
	but for each function pointer argument to inferior function call.

--- ../../../sourceware-mips/src/gdb/mips-tdep.c	2010-12-13 11:54:02.775400717 -0700
+++ mips-tdep.c	2010-12-13 15:49:30.386337794 -0700
@@ -205,6 +205,12 @@ unmake_mips16_addr (CORE_ADDR addr)
   return ((addr) & ~(CORE_ADDR) 1);
 }
 
+static CORE_ADDR
+make_mips16_addr (CORE_ADDR addr)
+{
+  return ((addr) | (CORE_ADDR) 1);
+}
+
 /* Return the MIPS ABI associated with GDBARCH.  */
 enum mips_abi
 mips_abi (struct gdbarch *gdbarch)
@@ -264,7 +270,6 @@ mips_elf_make_msymbol_special (asymbol *
   if (((elf_symbol_type *) (sym))->internal_elf_sym.st_other == STO_MIPS16)
     {
       MSYMBOL_TARGET_FLAG_1 (msym) = 1;
-      SYMBOL_VALUE_ADDRESS (msym) |= 1;
     }
 }
 
@@ -931,14 +936,21 @@ mips_read_pc (struct regcache *regcache)
   ULONGEST pc;
   int regnum = mips_regnum (get_regcache_arch (regcache))->pc;
   regcache_cooked_read_signed (regcache, regnum, &pc);
+  if (is_mips16_addr (pc))
+    pc = unmake_mips16_addr (pc);
   return pc;
 }
 
 static CORE_ADDR
 mips_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
-  return frame_unwind_register_signed
-	   (next_frame, gdbarch_num_regs (gdbarch) + mips_regnum (gdbarch)->pc);
+  ULONGEST pc;
+
+  pc = frame_unwind_register_signed
+	 (next_frame, gdbarch_num_regs (gdbarch) + mips_regnum (gdbarch)->pc);
+  if (is_mips16_addr (pc))
+    pc = unmake_mips16_addr (pc);
+  return pc;
 }
 
 static CORE_ADDR
@@ -967,7 +979,10 @@ static void
 mips_write_pc (struct regcache *regcache, CORE_ADDR pc)
 {
   int regnum = mips_regnum (get_regcache_arch (regcache))->pc;
-  regcache_cooked_write_unsigned (regcache, regnum, pc);
+  if (mips_pc_is_mips16 (pc))
+    regcache_cooked_write_unsigned (regcache, regnum, make_mips16_addr (pc));
+  else
+    regcache_cooked_write_unsigned (regcache, regnum, pc);
 }
 
 /* Fetch and return instruction from the specified location.  If the PC
@@ -2450,6 +2465,10 @@ static CORE_ADDR
 mips_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (is_mips16_addr (addr))
+    addr = unmake_mips16_addr (addr);
+
   if (mips_mask_address_p (tdep) && (((ULONGEST) addr) >> 32 == 0xffffffffUL))
     /* This hack is a work-around for existing boards using PMON, the
        simulator, and any other 64-bit targets that doesn't have true
@@ -2869,9 +2888,25 @@ mips_eabi_push_dummy_call (struct gdbarc
 			    "mips_eabi_push_dummy_call: %d len=%d type=%d",
 			    argnum + 1, len, (int) typecode);
 
+      /* Function pointer arguments to mips16 code need to be made into
+         mips16 pointers.  */
+      if (typecode == TYPE_CODE_PTR
+          && TYPE_CODE (TYPE_TARGET_TYPE (arg_type)) == TYPE_CODE_FUNC)
+	{
+	  CORE_ADDR addr = extract_signed_integer (value_contents (arg),
+						   len, byte_order);
+	  if (mips_pc_is_mips16 (addr))
+	    {
+	      store_signed_integer (valbuf, len, byte_order, 
+				    make_mips16_addr (addr));
+	      val = valbuf;
+	    }
+	  else
+	    val = value_contents (arg);
+	}
       /* The EABI passes structures that do not fit in a register by
          reference.  */
-      if (len > regsize
+      else if (len > regsize
 	  && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
 	{
 	  store_unsigned_integer (valbuf, regsize, byte_order,
@@ -4159,6 +4194,7 @@ mips_o64_push_dummy_call (struct gdbarch
   for (argnum = 0; argnum < nargs; argnum++)
     {
       const gdb_byte *val;
+      gdb_byte valbuf[MAX_REGISTER_SIZE];
       struct value *arg = args[argnum];
       struct type *arg_type = check_typedef (value_type (arg));
       int len = TYPE_LENGTH (arg_type);
@@ -4171,6 +4207,21 @@ mips_o64_push_dummy_call (struct gdbarch
 
       val = value_contents (arg);
 
+      /* Function pointer arguments to mips16 code need to be made into
+         mips16 pointers.  */
+      if (typecode == TYPE_CODE_PTR
+          && TYPE_CODE (TYPE_TARGET_TYPE (arg_type)) == TYPE_CODE_FUNC)
+	{
+	  CORE_ADDR addr = extract_signed_integer (value_contents (arg),
+						   len, byte_order);
+	  if (mips_pc_is_mips16 (addr))
+	    {
+	      store_signed_integer (valbuf, len, byte_order, 
+				    make_mips16_addr (addr));
+	      val = valbuf;
+	    }
+	}
+
       /* Floating point arguments passed in registers have to be
          treated specially.  On 32-bit architectures, doubles
          are passed in register pairs; the even register gets



More information about the Gdb-patches mailing list