Re: [PATCH] microMIPS support (Linux signal trampolines)

["Maciej W. Rozycki" <macro@codesourcery.com>]

Hi,

On Tue, 24 Apr 2012, Maciej W. Rozycki wrote:

>  Here's a change that adds support for debugging binaries containing 
> microMIPS code.  There's not much to say about it except that it works. ;)  

 And with the update below it works even better. :)

 The necessity for this change was revealed in the course of investigation 
related to my recent ISA bit submission.  It adds support for Linux signal 
trampolines encoded with the microMIPS instruction set.  Such trampolines 
are used by the Linux kernel if compiled as a microMIPS binary (even if 
the binary run/debugged itself contains no microMIPS code at all).  Until 
recently the reliability of such kernel binaries was not good enough for 
regression testing as they would typically not get through at all.  
Therefore the need for this change was not covered and escaped testing.  
It was revealed by code inspection.

 To see if we need to check whether the execution mode selected matches 
the given trampoline I have checked what the bit patterns of all the 
trampoline sequences decode to in the opposite instruction set.  This 
produced useless or at least unusual code in most cases, for example:

microMIPS/EB, o32 sigreturn, decoded as MIPS:
	30401017 	andi	zero,v0,0x1017
	00008b7c 	dsll32	s1,zero,0xd

MIPS/EL, o32 sigreturn, decoded as microMIPS:
	1017 2402 	addi	zero,s7,9218
	000c 0000 	sll	zero,t0,0x0

However in some corner cases reasonable code can mimic a trampoline, for 
example:

MIPS/EB, n32 rt_sigreturn, decoded as microMIPS:
	2402      	sll	s0,s0,1
	1843 0000 	sb	v0,0(v1)
	000c 0f3c 	jr	t0

-- here the first instruction is a 16-bit one making things nastier even 
as there are some other microMIPS instructions whose first 16-bit halfword 
is 0x000c and therefore matches this whole trampoline pattern.

 To overcome this problem I have decided the signal trampoline unwinder 
has to ask the platform backend whether it can apply a given trampoline 
pattern to the code location being concerned or not.  Anticipating the 
acceptance of the ISA bit proposal I decided the handler not to merely be 
a predicate, but also to be able to provide an adjusted PC if required.  
I decided that returning zero will mean that the trampoline pattern is not 
applicable and any other value is the adjusted PC to use; a handler may 
return the value requested if the trampoline pattern and the PC requested 
as-is are both accepted.

 This changes the semantics of the trampoline unwinder a bit in that the 
zero PC now has a special value.  I think this should be safe as a NULL 
pointer is generally supposed to be invalid.  Let me know if you think 
that could cause any troubles (the zero PC serves as a frame chain 
termination mark in some ABIs, including the MIPS ABI; I have a vague 
recollection that it is actually already caught elsewhere, but couldn't 
track that place down easily).

 A corresponding change will be required to the ISA bit proposal that I 
shall send separately -- the signal trampoline unwinder is a piece of 
generic code that interpretes the instruction stream (hopefully this is 
the only such place across GDB), so it has to see the ISA bit stripped off 
which the change below doesn't do for obvious reasons.

 Also obviously, if the ISA bit was stripped off from the PC as execution 
stopped in an actual trampoline, then we'll never have a chance to know 
that we stopped in the microMIPS mode in the first place as the trampoline 
will not have any symbol information associated and by now the ISA bit has 
been lost.  Which really means the change below will only make sense once 
the ISA bit proposal has got in.

 I'll be pushing this update through testing now.  I don't expect any 
problems, the change is mostly mechanical (as noted before microMIPS 
instructions, similarly to MIPS16 ones, are sequences of halfwords, hence 
the different arrangement of the instruction patterns used; POOL32A is the 
name of the major opcode the SYSCALL instruction is assigned to, that 
takes the upper six bits and happens to be all-zeros, the rest of the 
halfword is the SYSCALL immediate "argument" that is all-zeros too).

 Given that this change touches generic parts, the note about the ISA bit 
above, and that the original microMIPS change has worked correctly for 
many months now (especially given that microMIPS Linux kernel binaries may 
have only recently become usable), I decided I would be treating these two 
patches as separate and will therefore commit the microMIPS patch in its 
original form straight away.

 Meanwhile I'll be happy to answer any questions.

2011-05-18  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* tramp-frame.h (tramp_frame): Add validate member.
	* tramp-frame.c (tramp_frame_start): Validate trampoline before
	scanning.
	* mips-linux-tdep.c (MICROMIPS_INST_LI_V0): New macro.
	(MICROMIPS_INST_POOL32A, MICROMIPS_INST_SYSCALL): Likewise.
	(mips_linux_o32_sigframe): Initialize validate member.
	(mips_linux_o32_rt_sigframe): Likewise.
	(mips_linux_n32_rt_sigframe): Likewise.
	(mips_linux_n64_rt_sigframe): Likewise.
	(micromips_linux_o32_sigframe): New variable.
	(micromips_linux_o32_rt_sigframe): Likewise.
	(micromips_linux_n32_rt_sigframe): Likewise.
	(micromips_linux_n64_rt_sigframe): Likewise.
	(mips_linux_o32_sigframe_init): Handle microMIPS trampolines.
	(mips_linux_n32n64_sigframe_init): Likewise.
	(mips_linux_sigframe_validate): New function.
	(micromips_linux_sigframe_validate): Likewise.
	(mips_linux_init_abi): Install microMIPS trampoline unwinders.

  Maciej

gdb-micromips-linux-sigtramp.diff
Index: gdb-fsf-trunk-quilt/gdb/mips-linux-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-linux-tdep.c	2012-05-18 19:46:29.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/mips-linux-tdep.c	2012-05-18 20:41:04.625560181 +0100
@@ -800,6 +800,14 @@ static void mips_linux_n32n64_sigframe_i
 					     struct trad_frame_cache *this_cache,
 					     CORE_ADDR func);
 
+static CORE_ADDR mips_linux_sigframe_validate (const struct tramp_frame *self,
+					       struct frame_info *this_frame,
+					       CORE_ADDR pc);
+
+static CORE_ADDR micromips_linux_sigframe_validate (const struct tramp_frame *self,
+						    struct frame_info *this_frame,
+						    CORE_ADDR pc);
+
 #define MIPS_NR_LINUX 4000
 #define MIPS_NR_N64_LINUX 5000
 #define MIPS_NR_N32_LINUX 6000
@@ -815,6 +823,10 @@ static void mips_linux_n32n64_sigframe_i
 #define MIPS_INST_LI_V0_N32_RT_SIGRETURN 0x24020000 + MIPS_NR_N32_rt_sigreturn
 #define MIPS_INST_SYSCALL 0x0000000c
 
+#define MICROMIPS_INST_LI_V0 0x3040
+#define MICROMIPS_INST_POOL32A 0x0000
+#define MICROMIPS_INST_SYSCALL 0x8b7c
+
 static const struct tramp_frame mips_linux_o32_sigframe = {
   SIGTRAMP_FRAME,
   4,
@@ -823,7 +835,8 @@ static const struct tramp_frame mips_lin
     { MIPS_INST_SYSCALL, -1 },
     { TRAMP_SENTINEL_INSN, -1 }
   },
-  mips_linux_o32_sigframe_init
+  mips_linux_o32_sigframe_init,
+  mips_linux_sigframe_validate
 };
 
 static const struct tramp_frame mips_linux_o32_rt_sigframe = {
@@ -833,7 +846,8 @@ static const struct tramp_frame mips_lin
     { MIPS_INST_LI_V0_RT_SIGRETURN, -1 },
     { MIPS_INST_SYSCALL, -1 },
     { TRAMP_SENTINEL_INSN, -1 } },
-  mips_linux_o32_sigframe_init
+  mips_linux_o32_sigframe_init,
+  mips_linux_sigframe_validate
 };
 
 static const struct tramp_frame mips_linux_n32_rt_sigframe = {
@@ -844,7 +858,8 @@ static const struct tramp_frame mips_lin
     { MIPS_INST_SYSCALL, -1 },
     { TRAMP_SENTINEL_INSN, -1 }
   },
-  mips_linux_n32n64_sigframe_init
+  mips_linux_n32n64_sigframe_init,
+  mips_linux_sigframe_validate
 };
 
 static const struct tramp_frame mips_linux_n64_rt_sigframe = {
@@ -855,7 +870,63 @@ static const struct tramp_frame mips_lin
     { MIPS_INST_SYSCALL, -1 },
     { TRAMP_SENTINEL_INSN, -1 }
   },
-  mips_linux_n32n64_sigframe_init
+  mips_linux_n32n64_sigframe_init,
+  mips_linux_sigframe_validate
+};
+
+static const struct tramp_frame micromips_linux_o32_sigframe = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { MICROMIPS_INST_LI_V0, -1 },
+    { MIPS_NR_sigreturn, -1 },
+    { MICROMIPS_INST_POOL32A, -1 },
+    { MICROMIPS_INST_SYSCALL, -1 },
+    { TRAMP_SENTINEL_INSN, -1 }
+  },
+  mips_linux_o32_sigframe_init,
+  micromips_linux_sigframe_validate
+};
+
+static const struct tramp_frame micromips_linux_o32_rt_sigframe = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { MICROMIPS_INST_LI_V0, -1 },
+    { MIPS_NR_rt_sigreturn, -1 },
+    { MICROMIPS_INST_POOL32A, -1 },
+    { MICROMIPS_INST_SYSCALL, -1 },
+    { TRAMP_SENTINEL_INSN, -1 } },
+  mips_linux_o32_sigframe_init,
+  micromips_linux_sigframe_validate
+};
+
+static const struct tramp_frame micromips_linux_n32_rt_sigframe = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { MICROMIPS_INST_LI_V0, -1 },
+    { MIPS_NR_N32_rt_sigreturn, -1 },
+    { MICROMIPS_INST_POOL32A, -1 },
+    { MICROMIPS_INST_SYSCALL, -1 },
+    { TRAMP_SENTINEL_INSN, -1 }
+  },
+  mips_linux_n32n64_sigframe_init,
+  micromips_linux_sigframe_validate
+};
+
+static const struct tramp_frame micromips_linux_n64_rt_sigframe = {
+  SIGTRAMP_FRAME,
+  2,
+  {
+    { MICROMIPS_INST_LI_V0, -1 },
+    { MIPS_NR_N64_rt_sigreturn, -1 },
+    { MICROMIPS_INST_POOL32A, -1 },
+    { MICROMIPS_INST_SYSCALL, -1 },
+    { TRAMP_SENTINEL_INSN, -1 }
+  },
+  mips_linux_n32n64_sigframe_init,
+  micromips_linux_sigframe_validate
 };
 
 /* *INDENT-OFF* */
@@ -942,7 +1013,8 @@ mips_linux_o32_sigframe_init (const stru
   const struct mips_regnum *regs = mips_regnum (gdbarch);
   CORE_ADDR regs_base;
 
-  if (self == &mips_linux_o32_sigframe)
+  if (self == &mips_linux_o32_sigframe
+      || self == &micromips_linux_o32_sigframe)
     sigcontext_base = frame_sp + SIGFRAME_SIGCONTEXT_OFFSET;
   else
     sigcontext_base = frame_sp + RTSIGFRAME_SIGCONTEXT_OFFSET;
@@ -1107,7 +1179,8 @@ mips_linux_n32n64_sigframe_init (const s
   CORE_ADDR sigcontext_base;
   const struct mips_regnum *regs = mips_regnum (gdbarch);
 
-  if (self == &mips_linux_n32_rt_sigframe)
+  if (self == &mips_linux_n32_rt_sigframe
+      || self == &micromips_linux_n32_rt_sigframe)
     sigcontext_base = frame_sp + N32_SIGFRAME_SIGCONTEXT_OFFSET;
   else
     sigcontext_base = frame_sp + N64_SIGFRAME_SIGCONTEXT_OFFSET;
@@ -1151,6 +1224,22 @@ mips_linux_n32n64_sigframe_init (const s
   trad_frame_set_id (this_cache, frame_id_build (frame_sp, func));
 }
 
+static CORE_ADDR
+mips_linux_sigframe_validate (const struct tramp_frame *self,
+			      struct frame_info *this_frame,
+			      CORE_ADDR pc)
+{
+  return mips_pc_is_mips (pc) ? pc : 0;
+}
+
+static CORE_ADDR
+micromips_linux_sigframe_validate (const struct tramp_frame *self,
+				   struct frame_info *this_frame,
+				   CORE_ADDR pc)
+{
+  return mips_pc_is_micromips (get_frame_arch (this_frame), pc) ? pc : 0;
+}
+
 /* Implement the "write_pc" gdbarch method.  */
 
 static void
@@ -1254,6 +1343,9 @@ mips_linux_init_abi (struct gdbarch_info
 	                                mips_linux_get_longjmp_target);
 	set_solib_svr4_fetch_link_map_offsets
 	  (gdbarch, svr4_ilp32_fetch_link_map_offsets);
+	tramp_frame_prepend_unwinder (gdbarch, &micromips_linux_o32_sigframe);
+	tramp_frame_prepend_unwinder (gdbarch,
+				      &micromips_linux_o32_rt_sigframe);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_o32_sigframe);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_o32_rt_sigframe);
 	set_xml_syscall_file_name ("syscalls/mips-o32-linux.xml");
@@ -1269,6 +1361,8 @@ mips_linux_init_abi (struct gdbarch_info
 	   except that the quiet/signalling NaN bit is reversed (GDB
 	   does not distinguish between quiet and signalling NaNs).  */
 	set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
+	tramp_frame_prepend_unwinder (gdbarch,
+				      &micromips_linux_n32_rt_sigframe);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_n32_rt_sigframe);
 	set_xml_syscall_file_name ("syscalls/mips-n32-linux.xml");
 	break;
@@ -1283,6 +1377,8 @@ mips_linux_init_abi (struct gdbarch_info
 	   except that the quiet/signalling NaN bit is reversed (GDB
 	   does not distinguish between quiet and signalling NaNs).  */
 	set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
+	tramp_frame_prepend_unwinder (gdbarch,
+				      &micromips_linux_n64_rt_sigframe);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_n64_rt_sigframe);
 	set_xml_syscall_file_name ("syscalls/mips-n64-linux.xml");
 	break;
Index: gdb-fsf-trunk-quilt/gdb/tramp-frame.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/tramp-frame.c	2012-02-24 15:23:42.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/tramp-frame.c	2012-05-18 20:03:53.775469792 +0100
@@ -87,6 +87,12 @@ tramp_frame_start (const struct tramp_fr
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int ti;
 
+  /* Check if we can use this trampoline.  */
+  if (tramp->validate)
+    pc = tramp->validate (tramp, this_frame, pc);
+  if (pc == 0)
+    return 0;
+
   /* Search through the trampoline for one that matches the
      instruction sequence around PC.  */
   for (ti = 0; tramp->insn[ti].bytes != TRAMP_SENTINEL_INSN; ti++)
Index: gdb-fsf-trunk-quilt/gdb/tramp-frame.h
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/tramp-frame.h	2012-02-24 15:23:42.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/tramp-frame.h	2012-05-18 19:56:11.745630232 +0100
@@ -69,6 +69,12 @@ struct tramp_frame
 		struct frame_info *this_frame,
 		struct trad_frame_cache *this_cache,
 		CORE_ADDR func);
+  /* Check if the tramp-frame is valid for the PC requested.  Return
+     the address to check the instruction sequence against if so,
+     otherwise zero.  If this is NULL, then any PC is valid.  */
+  CORE_ADDR (*validate) (const struct tramp_frame *self,
+			 struct frame_info *this_frame,
+			 CORE_ADDR pc);
 };
 
 void tramp_frame_prepend_unwinder (struct gdbarch *gdbarch,