get_frame_func() VS get_frame_id().code

get_frame_func() VS get_frame_id().code

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

Hello,

The current get_frame_func() is implemented as roughly:

       fi->prev_func.addr = get_pc_function_start (addr_in_block);

Unfortunatly this isn't valid for a signal trampoline (or at least the 
evil ones that consist of random bytes in a random memory location). 
For such trampolines, get_pc_function_start [rightly] fails and "func" 
ends up as zero -- not good -- a properly constructed frame ID requires 
non-zero code and stack addresses.

Fortunatly, with a bit of extra instruction pattern matching, it is 
possible to identify the first instruction of a signal trampoline and 
hence correctly compute the trampolines "func" address.  Similarly, more 
normal frames can determine the function start using the symbol table's 
get_pc_function_start.

Consequently, I think there should be mechanism for obtaining both the 
symbol table and frame's idea of a function's start address.  This would 
mean introducing:

- get_frame_func_by_symtab
Returns the function start according to the symbol table.  Much of the 
existing code (especially unwinders) would need to be updated to use this.

- get_frame_func_by_id
Returns the function start based on the frame ID.  With the first change 
made, this could even be called get_frame_func.

I've attached a proof-of-concept and as such the patch points to a 
number of additional changes (the most obvious being the need for a 
"tramp-frame" that generalizes the technique).

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 6297 bytes --]

2004-03-03  Andrew Cagney  <cagney@redhat.com>

	* ppcnbsd-tdep.c: Include "objfiles.h", "trad-frame.h", and
	"frame-unwind.h".
	(struct ppcnbsd_sigtramp_cache, ppcnbsd_sigtramp_this_id)
	(ppcnbsd_sigtramp_prev_register, ppcnbsd_sigtramp_cache)
	(ppcnbsd_sigtramp_sniffer, ppcnbsd_sigtramp_unwind)
	(ppcnbsd_sigtramp_start, ppcnbsd_init_abi): Implement a NetBSD/PPC
	signal trampline unwinder, register.

Index: ppcnbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppcnbsd-tdep.c,v
retrieving revision 1.11
diff -u -r1.11 ppcnbsd-tdep.c
--- ppcnbsd-tdep.c	10 Nov 2003 22:47:28 -0000	1.11
+++ ppcnbsd-tdep.c	5 Mar 2004 15:39:35 -0000
@@ -26,6 +26,9 @@
 #include "breakpoint.h"
 #include "value.h"
 #include "osabi.h"
+#include "trad-frame.h"
+#include "frame-unwind.h"
+#include "objfiles.h"
 
 #include "ppc-tdep.h"
 #include "ppcnbsd-tdep.h"
@@ -227,6 +230,165 @@
 					     readbuf, writebuf);
 }
 
+/* Given the NEXTE frame, examine the instructions at and around this
+   frame's resume address (aka PC) to see of they look like a signal
+   trampoline.  Return the address of the trampolines first
+   instruction, or zero if it isn't a signal trampoline.  */
+
+static CORE_ADDR
+ppcnbsd_sigtramp_start (struct frame_info *next_frame)
+{
+  static const long sigtramp[] = {
+    0x38610018, /* addi r3,r1,24 */
+    0x38000127, /* li r0,295 */
+    0x44000002, /* sc */
+    0x38000001, /* li r0,1 */
+    0x44000002, /* sc */
+  };
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  long pcinsn;
+  int pci;
+
+  /* Fetch the instruction at PC and try to find it in the
+     sigtramp.  */
+  pcinsn = read_memory_unsigned_integer (pc, 4);
+  for (pci = 0; pci < ARRAY_SIZE (sigtramp); pci++)
+    {
+      if (sigtramp[pci] == pcinsn)
+	{
+	  /* With the PC's instruction found, check that the remaining
+	     instructions also match.  Because there are two SC's in
+	     the above, this part may need to be executed more than
+	     once.  */
+	  int i;
+	  long insn;
+	  for (i = 0; i < ARRAY_SIZE (sigtramp); i++)
+	    {
+	      if (i == pci) continue;
+	      insn = read_memory_unsigned_integer (pc + 4 * (i - pci), 4);
+	      if (sigtramp[i] != insn)
+		break;
+	    }
+	  /* All matched, return the start of the signal
+	     trampoline.  */
+	  if (i >= ARRAY_SIZE (sigtramp))
+	    return pc - 4 * pci;
+	}
+    }
+  /* Not a sigtramp.  */
+  return 0;
+}
+
+struct ppcnbsd_sigtramp_cache
+{
+  struct frame_id this_id;
+  CORE_ADDR this_base;
+  struct trad_frame_saved_reg *saved_regs;
+};
+
+static struct ppcnbsd_sigtramp_cache *
+ppcnbsd_sigtramp_cache (struct frame_info *next_frame, void **this_cache)
+{
+  CORE_ADDR offset;
+  int i;
+  struct ppcnbsd_sigtramp_cache *cache;
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if ((*this_cache) != NULL)
+    return (*this_cache);
+  cache = FRAME_OBSTACK_ZALLOC (struct ppcnbsd_sigtramp_cache);
+  (*this_cache) = cache;
+  cache->saved_regs = trad_frame_alloc_saved_regs (next_frame);
+
+  cache->this_base = frame_unwind_register_unsigned (next_frame, SP_REGNUM);
+  offset = cache->this_base + 0x18 + 2 * tdep->wordsize;
+  for (i = 0; i < 32; i++)
+    {
+      int regnum = i + tdep->ppc_gp0_regnum;
+      cache->saved_regs[regnum].addr = offset;
+      offset += tdep->wordsize;
+    }
+  cache->saved_regs[tdep->ppc_lr_regnum].addr = offset;
+  offset += tdep->wordsize;
+  cache->saved_regs[tdep->ppc_cr_regnum].addr = offset;
+  offset += tdep->wordsize;
+  cache->saved_regs[tdep->ppc_xer_regnum].addr = offset;
+  offset += tdep->wordsize;
+  cache->saved_regs[tdep->ppc_ctr_regnum].addr = offset;
+  offset += tdep->wordsize;
+  cache->saved_regs[PC_REGNUM].addr = offset; /* SRR0? */
+  offset += tdep->wordsize;
+
+  /* Construct the frame ID using the function start.  */
+  {
+    CORE_ADDR func = frame_func_unwind (next_frame);
+    if (func == 0)
+      cache->this_id = frame_id_build (cache->this_base,
+				       ppcnbsd_sigtramp_start (next_frame));
+    else
+      cache->this_id = frame_id_build (cache->this_base, func);
+  }
+
+  return cache;
+}
+
+static void
+ppcnbsd_sigtramp_this_id (struct frame_info *next_frame, void **this_cache,
+			  struct frame_id *this_id)
+{
+  struct ppcnbsd_sigtramp_cache *info = ppcnbsd_sigtramp_cache (next_frame,
+								this_cache);
+  (*this_id) = info->this_id;
+}
+
+static void
+ppcnbsd_sigtramp_prev_register (struct frame_info *next_frame,
+				void **this_cache,
+				int regnum, int *optimizedp,
+				enum lval_type *lvalp, CORE_ADDR *addrp,
+				int *realnump, void *valuep)
+{
+  struct ppcnbsd_sigtramp_cache *info = ppcnbsd_sigtramp_cache (next_frame,
+								this_cache);
+  trad_frame_prev_register (next_frame, info->saved_regs, regnum,
+			    optimizedp, lvalp, addrp, realnump, valuep);
+}
+
+static const struct frame_unwind ppcnbsd_sigtramp_unwind =
+{
+  SIGTRAMP_FRAME,
+  ppcnbsd_sigtramp_this_id,
+  ppcnbsd_sigtramp_prev_register
+};
+
+static const struct frame_unwind *
+ppcnbsd_sigtramp_sniffer (struct frame_info *next_frame)
+{
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  char *name;
+  find_pc_partial_function (pc, &name, NULL, NULL);
+  if (name != NULL)
+    {
+      /* A modern unwinder (post 1.6.x) that lives in libc.  It can be
+	 identified by its name.  */
+      if (nbsd_pc_in_sigtramp (pc, name))
+	return &ppcnbsd_sigtramp_unwind;
+      else
+	return NULL;
+    }
+  if (find_pc_section (pc) != NULL)
+    /* Old style signal trampolines (which lived on the stack) never
+       had a section, so if one is found, it can't be a sigtramp.  */
+    return NULL;
+  /* Examine instructions around PC to see if they match the sigtramp,
+     if they do ok.  */
+  if (ppcnbsd_sigtramp_start (next_frame) != 0)
+    return &ppcnbsd_sigtramp_unwind;
+  else
+    return NULL;
+}
+
 static void
 ppcnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
@@ -237,6 +399,7 @@
   set_gdbarch_return_value (gdbarch, ppcnbsd_return_value);
   set_solib_svr4_fetch_link_map_offsets (gdbarch,
                                 nbsd_ilp32_solib_svr4_fetch_link_map_offsets);
+  frame_unwind_append_sniffer (gdbarch, ppcnbsd_sigtramp_sniffer);
 }
 
 void

Re: get_frame_func() VS get_frame_id().code

[Daniel Jacobowitz]

On Fri, Mar 05, 2004 at 11:05:54AM -0500, Andrew Cagney wrote:
> Hello,
> 
> The current get_frame_func() is implemented as roughly:
> 
>       fi->prev_func.addr = get_pc_function_start (addr_in_block);
> 
> Unfortunatly this isn't valid for a signal trampoline (or at least the 
> evil ones that consist of random bytes in a random memory location). 
> For such trampolines, get_pc_function_start [rightly] fails and "func" 
> ends up as zero -- not good -- a properly constructed frame ID requires 
> non-zero code and stack addresses.
> 
> Fortunatly, with a bit of extra instruction pattern matching, it is 
> possible to identify the first instruction of a signal trampoline and 
> hence correctly compute the trampolines "func" address.  Similarly, more 
> normal frames can determine the function start using the symbol table's 
> get_pc_function_start.
> 
> Consequently, I think there should be mechanism for obtaining both the 
> symbol table and frame's idea of a function's start address.  This would 
> mean introducing:
> 
> - get_frame_func_by_symtab
> Returns the function start according to the symbol table.  Much of the 
> existing code (especially unwinders) would need to be updated to use this.

How about just leaving this as frame_func_unwind?  The only current use
of frame_func_unwind that I see outside of unwinders is the
implementation of get_frame_func.  So we could define frame_func_unwind
to attempt to find the beginning of the function at the unwound PC
using the symtab.

It is already the unwinder's duty to propogate the function address
into the ID.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: get_frame_func() VS get_frame_id().code

[Andrew Cagney]

> On Fri, Mar 05, 2004 at 11:05:54AM -0500, Andrew Cagney wrote:
> 
>>> Hello,
>>> 
>>> The current get_frame_func() is implemented as roughly:
>>> 
>>>       fi->prev_func.addr = get_pc_function_start (addr_in_block);
>>> 
>>> Unfortunatly this isn't valid for a signal trampoline (or at least the 
>>> evil ones that consist of random bytes in a random memory location). 
>>> For such trampolines, get_pc_function_start [rightly] fails and "func" 
>>> ends up as zero -- not good -- a properly constructed frame ID requires 
>>> non-zero code and stack addresses.
>>> 
>>> Fortunatly, with a bit of extra instruction pattern matching, it is 
>>> possible to identify the first instruction of a signal trampoline and 
>>> hence correctly compute the trampolines "func" address.  Similarly, more 
>>> normal frames can determine the function start using the symbol table's 
>>> get_pc_function_start.
>>> 
>>> Consequently, I think there should be mechanism for obtaining both the 
>>> symbol table and frame's idea of a function's start address.  This would 
>>> mean introducing:
>>> 
>>> - get_frame_func_by_symtab
>>> Returns the function start according to the symbol table.  Much of the 
>>> existing code (especially unwinders) would need to be updated to use this.
> 
> 
> How about just leaving this as frame_func_unwind?

Because the func, based on the frame ID, is more likely to be correct. 
The current get_frame_func and frame_func_unwind can return 0 or the 
wrong function :-(

BTW, there is get_frame_function() which returns the symbol for the 
function.

 >  The only current use
 > of frame_func_unwind that I see outside of unwinders is the
 > implementation of get_frame_func.

... get_frame_func is hardly called ...

 >  So we could define frame_func_unwind
 > to attempt to find the beginning of the function at the unwound PC
 > using the symtab.

... so it may even be possible to simply pull get_frame_func from the 
frame interface.

> It is already the unwinder's duty to propogate the function address
> into the ID.

Andrew