Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch/rfc] signal trampoline frames
@ 2004-03-22  3:14 Andrew Cagney
  2004-03-23 14:13 ` Andrew Cagney
  2004-03-23 23:09 ` Daniel Jacobowitz
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Cagney @ 2004-03-22  3:14 UTC (permalink / raw)
  To: gdb-patches

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

Hello,

This adds "tramp-frame.[hc]" as a generic signal trampoline frame 
unwinder.  The client architectures, per the revised change to 
ppcnbsd-tdep.c (it superseeds my original), just needs to provide the 
signal trampoline instruction sequence and a method to initialize the 
trad_frame_cache.

Thoughts, I'll look to commit the tramp-frame parts in a few days.

committed to my tramp branch,
Andrew

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

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

	* tramp-frame.h, tramp-frame.h: New files.
	* Makefile.in (SFILES, tramp_frame_h, COMMON_OBS, tramp-frame.o):
	Update rules to include "tramp-frame.h" and "tramp-frame.c".

	* Makefile.in (ppcnbsd-tdep.o): Update dependencies.
	* ppcnbsd-tdep.c: Include "tramp-frame.h" and "trad-frame.h".
	(ppcnbsd_sigtramp_cache_init, ppcnbsd_sigtramp): Handle signal
	trampoline.
	(ppcnbsd_init_abi): Call tramp_frame_append with ppcnbsd_sigtramp.
	
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	22 Mar 2004 01:40:26 -0000
@@ -30,6 +30,8 @@
 #include "ppc-tdep.h"
 #include "ppcnbsd-tdep.h"
 #include "nbsd-tdep.h"
+#include "tramp-frame.h"
+#include "trad-frame.h"
 
 #include "solib-svr4.h"
 
@@ -228,6 +230,58 @@
 }
 
 static void
+ppcnbsd_sigtramp_cache_init (const struct tramp_frame *self,
+			     struct frame_info *next_frame,
+			     struct trad_frame_cache *this_cache,
+			     CORE_ADDR func)
+{
+  CORE_ADDR base;
+  CORE_ADDR offset;
+  int i;
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  base = frame_unwind_register_unsigned (next_frame, SP_REGNUM);
+  offset = base + 0x18 + 2 * tdep->wordsize;
+  for (i = 0; i < 32; i++)
+    {
+      int regnum = i + tdep->ppc_gp0_regnum;
+      trad_frame_set_reg_addr (this_cache, regnum, offset);
+      offset += tdep->wordsize;
+    }
+  trad_frame_set_reg_addr (this_cache, tdep->ppc_lr_regnum, offset);
+  offset += tdep->wordsize;
+  trad_frame_set_reg_addr (this_cache, tdep->ppc_cr_regnum, offset);
+  offset += tdep->wordsize;
+  trad_frame_set_reg_addr (this_cache, tdep->ppc_xer_regnum, offset);
+  offset += tdep->wordsize;
+  trad_frame_set_reg_addr (this_cache, tdep->ppc_ctr_regnum, offset);
+  offset += tdep->wordsize;
+  trad_frame_set_reg_addr (this_cache, PC_REGNUM, offset); /* SRR0? */
+  offset += tdep->wordsize;
+
+  /* Construct the frame ID using the function start.  */
+  trad_frame_set_id (this_cache, frame_id_build (base, func));
+}
+
+/* Given the NEXT 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 const struct tramp_frame ppcnbsd_sigtramp = {
+  4, /* insn size */
+  { /* insn */
+    0x38610018, /* addi r3,r1,24 */
+    0x38000127, /* li r0,295 */
+    0x44000002, /* sc */
+    0x38000001, /* li r0,1 */
+    0x44000002, /* sc */
+  },
+  ppcnbsd_sigtramp_cache_init
+};
+
+static void
 ppcnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
 {
@@ -237,6 +291,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);
+  tramp_frame_append (gdbarch, &ppcnbsd_sigtramp);
 }
 
 void
Index: tramp-frame.c
===================================================================
RCS file: tramp-frame.c
diff -N tramp-frame.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tramp-frame.c	22 Mar 2004 01:40:27 -0000
@@ -0,0 +1,162 @@
+/* Signal trampoline unwinder, for GDB the GNU Debugger.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include "defs.h"
+#include "tramp-frame.h"
+#include "frame-unwind.h"
+#include "gdbcore.h"
+#include "symtab.h"
+#include "objfiles.h"
+#include "target.h"
+#include "trad-frame.h"
+#include "frame-base.h"
+
+struct frame_data
+{
+  const struct tramp_frame *tramp_frame;
+};
+
+struct tramp_frame_cache
+{
+  CORE_ADDR func;
+  const struct tramp_frame *tramp_frame;
+  struct trad_frame_cache *trad_cache;
+};
+
+static struct trad_frame_cache *
+tramp_frame_cache (struct frame_info *next_frame,
+		   void **this_cache)
+{
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  struct tramp_frame_cache *tramp_cache = (*this_cache);
+  if (tramp_cache->trad_cache == NULL)
+    {
+      tramp_cache->trad_cache = trad_frame_cache_zalloc (next_frame);
+      tramp_cache->tramp_frame->init (tramp_cache->tramp_frame,
+				      next_frame,
+				      tramp_cache->trad_cache,
+				      tramp_cache->func);
+    }
+  return tramp_cache->trad_cache;
+}
+
+static void
+tramp_frame_this_id (struct frame_info *next_frame,
+		     void **this_cache,
+		     struct frame_id *this_id)
+{
+  struct trad_frame_cache *trad_cache
+    = tramp_frame_cache (next_frame, this_cache);
+  trad_frame_get_id (trad_cache, this_id);
+}
+
+static void
+tramp_frame_prev_register (struct frame_info *next_frame,
+			   void **this_cache,
+			   int prev_regnum,
+			   int *optimizedp,
+			   enum lval_type * lvalp,
+			   CORE_ADDR *addrp,
+			   int *realnump, void *valuep)
+{
+  struct trad_frame_cache *trad_cache
+    = tramp_frame_cache (next_frame, this_cache);
+  trad_frame_get_register (trad_cache, next_frame, prev_regnum, optimizedp,
+			   lvalp, addrp, realnump, valuep);
+}
+
+static CORE_ADDR
+tramp_frame_start (CORE_ADDR pc, const struct tramp_frame *tramp)
+{
+  int ti;
+  /* Search through the trampoline for one that matches the
+     instruction sequence around PC.  */
+  for (ti = 0; tramp->insn[ti] != 0; ti++)
+    {
+      CORE_ADDR func = pc - tramp->insn_size * ti;
+      int i;
+      for (i = 0; 1; i++)
+	{
+	  bfd_byte buf[sizeof (LONGEST)];
+	  CORE_ADDR insn;
+	  if (tramp->insn[i] == 0)
+	    return func;
+	  if (target_read_memory (func + i * tramp->insn_size, buf,
+				  tramp->insn_size) != 0)
+	    break;
+	  insn = extract_unsigned_integer (buf, tramp->insn_size);
+	  if (tramp->insn[i] != insn)
+	    break;
+	}
+    }
+  /* Trampoline doesn't match.  */
+  return 0;
+}
+
+static int
+tramp_frame_sniffer (const struct frame_unwind *self,
+		     struct frame_info *next_frame,
+		     void **this_cache)
+{
+  const struct tramp_frame *tramp = self->unwind_data->tramp_frame;
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  CORE_ADDR func;
+  char *name;
+  struct tramp_frame_cache *tramp_cache;
+
+  /* If the function has a valid symbol name, it isn't a
+     trampoline.  */
+  find_pc_partial_function (pc, &name, NULL, NULL);
+  if (name != NULL)
+    return 0;
+  /* If the function lives in a valid section (even without a starting
+     point) it isn't a trampoline.  */
+  if (find_pc_section (pc) != NULL)
+    return 0;
+  /* Finally, check that the trampoline matches at PC.  */
+  func = tramp_frame_start (pc, tramp);
+  if (func == 0)
+    return 0;
+  tramp_cache = FRAME_OBSTACK_ZALLOC (struct tramp_frame_cache);
+  tramp_cache->func = func;
+  tramp_cache->tramp_frame = tramp;
+  (*this_cache) = tramp_cache;
+  return 1;
+}
+
+void
+tramp_frame_append (struct gdbarch *gdbarch,
+		    const struct tramp_frame *tramp_frame)
+{
+  struct frame_data *data;
+  struct frame_unwind *unwinder;
+
+  data = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_data);
+  unwinder = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_unwind);
+
+  data->tramp_frame = tramp_frame;
+  unwinder->type = SIGTRAMP_FRAME;
+  unwinder->unwind_data = data;
+  unwinder->sniffer = tramp_frame_sniffer;
+  unwinder->this_id = tramp_frame_this_id;
+  unwinder->prev_register = tramp_frame_prev_register;
+  frame_unwind_register_unwinder (gdbarch, unwinder);
+}
Index: tramp-frame.h
===================================================================
RCS file: tramp-frame.h
diff -N tramp-frame.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tramp-frame.h	21 Mar 2004 23:57:38 -0000	1.1.2.2
@@ -0,0 +1,64 @@
+/* Signal trampoline unwinder, for GDB the GNU Debugger.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#ifndef TRAMP_FRAME_H
+#define TRAMP_FRAME_H
+
+struct trad_frame;
+struct frame_info;
+struct trad_frame_cache;
+
+/* A trampoline consists of a small sequence of instructions placed at
+   an unspecified location in the inferior's address space.  The only
+   identifying attribute of the trampoline's address is that it does
+   not fall inside an object file's section.
+
+   The only way to identify a trampoline is to perform a brute force
+   examination of the instructions at and around the PC.
+
+   This module provides a convent interface for performing that
+   operation.  */
+
+/* A trampoline descriptor.  */
+
+struct tramp_frame
+{
+  /* The trampoline's entire instruction sequence.  Search for this in
+     the inferior at or around the frame's PC.  It is assumed that the
+     PC is INSN_SIZE aligned, and that each element of TRAMP contains
+     one INSN_SIZE instruction.  It is also assumed that TRAMP[0]
+     contains the first instruction of the trampoline and hence the
+     address of the instruction matching TRAMP[0] is the trampoline's
+     "func" address.  */
+  int insn_size;
+  ULONGEST insn[8];
+  /* Initialize a trad-frame cache corresponding to the tramp-frame.
+     FUNC is the address of the instruction TRAMP[0] in memory.  */
+  void (*init) (const struct tramp_frame *self,
+		struct frame_info *next_frame,
+		struct trad_frame_cache *this_cache,
+		CORE_ADDR func);
+};
+
+void tramp_frame_append (struct gdbarch *gdbarch,
+			 const struct tramp_frame *tramp);
+
+#endif

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch/rfc] signal trampoline frames
  2004-03-22  3:14 [patch/rfc] signal trampoline frames Andrew Cagney
@ 2004-03-23 14:13 ` Andrew Cagney
  2004-03-23 23:09 ` Daniel Jacobowitz
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Cagney @ 2004-03-23 14:13 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

> Hello,
> 
> This adds "tramp-frame.[hc]" as a generic signal trampoline frame unwinder.  The client architectures, per the revised change to ppcnbsd-tdep.c (it superseeds my original), just needs to provide the signal trampoline instruction sequence and a method to initialize the trad_frame_cache.
> 
> Thoughts, I'll look to commit the tramp-frame parts in a few days.

I've committed this part:

> 2004-03-21  Andrew Cagney  <cagney@redhat.com>
> 
> 	* tramp-frame.h, tramp-frame.h: New files.
> 	* Makefile.in (SFILES, tramp_frame_h, COMMON_OBS, tramp-frame.o):
> 	Update rules to include "tramp-frame.h" and "tramp-frame.c".

Andrew



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch/rfc] signal trampoline frames
  2004-03-22  3:14 [patch/rfc] signal trampoline frames Andrew Cagney
  2004-03-23 14:13 ` Andrew Cagney
@ 2004-03-23 23:09 ` Daniel Jacobowitz
  2004-03-24  0:02   ` Andrew Cagney
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2004-03-23 23:09 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Sun, Mar 21, 2004 at 10:14:17PM -0500, Andrew Cagney wrote:
> Hello,
> 
> This adds "tramp-frame.[hc]" as a generic signal trampoline frame 
> unwinder.  The client architectures, per the revised change to 
> ppcnbsd-tdep.c (it superseeds my original), just needs to provide the 
> signal trampoline instruction sequence and a method to initialize the 
> trad_frame_cache.
> 
> Thoughts, I'll look to commit the tramp-frame parts in a few days.

Some minor comments.

> +static CORE_ADDR
> +tramp_frame_start (CORE_ADDR pc, const struct tramp_frame *tramp)
> +{
> +  int ti;
> +  /* Search through the trampoline for one that matches the
> +     instruction sequence around PC.  */
> +  for (ti = 0; tramp->insn[ti] != 0; ti++)
> +    {
> +      CORE_ADDR func = pc - tramp->insn_size * ti;
> +      int i;
> +      for (i = 0; 1; i++)
> +	{
> +	  bfd_byte buf[sizeof (LONGEST)];
> +	  CORE_ADDR insn;

tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
also.

> +	  if (tramp->insn[i] == 0)
> +	    return func;

So zeros in tramp->insn mark the end of the sequence?  Should document
that, zeros are valid instructions and some bizarre architecture might
use one as a syscall trap.

> +  /* If the function has a valid symbol name, it isn't a
> +     trampoline.  */
> +  find_pc_partial_function (pc, &name, NULL, NULL);
> +  if (name != NULL)
> +    return 0;
> +  /* If the function lives in a valid section (even without a starting
> +     point) it isn't a trampoline.  */
> +  if (find_pc_section (pc) != NULL)
> +    return 0;

I believe the first check is redundant to the second check; can we have
names without sections?  I may be wrong about this, remembering the
problem on IRIX with absolute sections.



-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch/rfc] signal trampoline frames
  2004-03-23 23:09 ` Daniel Jacobowitz
@ 2004-03-24  0:02   ` Andrew Cagney
  2004-03-24 17:15     ` Daniel Jacobowitz
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cagney @ 2004-03-24  0:02 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

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

see attached?

> 
>>> +static CORE_ADDR
>>> +tramp_frame_start (CORE_ADDR pc, const struct tramp_frame *tramp)
>>> +{
>>> +  int ti;
>>> +  /* Search through the trampoline for one that matches the
>>> +     instruction sequence around PC.  */
>>> +  for (ti = 0; tramp->insn[ti] != 0; ti++)
>>> +    {
>>> +      CORE_ADDR func = pc - tramp->insn_size * ti;
>>> +      int i;
>>> +      for (i = 0; 1; i++)
>>> +	{
>>> +	  bfd_byte buf[sizeof (LONGEST)];
>>> +	  CORE_ADDR insn;
> 
> 
> tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
> also.

changed to ->insn[0]

> 
>>> +	  if (tramp->insn[i] == 0)
>>> +	    return func;
> 
> 
> So zeros in tramp->insn mark the end of the sequence?  Should document
> that, zeros are valid instructions and some bizarre architecture might
> use one as a syscall trap.

Added TRAMP_SENTINEL_INSN, it _isn't_ zero.

>>> +  /* If the function has a valid symbol name, it isn't a
>>> +     trampoline.  */
>>> +  find_pc_partial_function (pc, &name, NULL, NULL);
>>> +  if (name != NULL)
>>> +    return 0;
>>> +  /* If the function lives in a valid section (even without a starting
>>> +     point) it isn't a trampoline.  */
>>> +  if (find_pc_section (pc) != NULL)
>>> +    return 0;
> 
> 
> I believe the first check is redundant to the second check; can we have
> names without sections?  I may be wrong about this, remembering the
> problem on IRIX with absolute sections.

I don't know.

Andrew


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

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

	* tramp-frame.h (TRAMP_SENTINEL_INSN): Define, document.
	* tramp-frame.c: Include "gdb_assert.h".
	(tramp_frame_start): Use TRAMP_SENTINEL_INSN.
	(tramp_frame_append): Assert the presence of TRAMP_SENTINEL_INSN.
	* Makefile.in (tramp-frame.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.529
diff -u -r1.529 Makefile.in
--- Makefile.in	23 Mar 2004 14:12:30 -0000	1.529
+++ Makefile.in	23 Mar 2004 23:58:05 -0000
@@ -2449,7 +2449,7 @@
 	$(regcache_h)
 tramp-frame.o: tramp-frame.c $(defs_h) $(tramp_frame_h) $(frame_unwind_h) \
 	$(gdbcore_h) $(symtab_h) $(objfiles_h) $(target_h) $(trad_frame_h) \
-	$(frame_base_h)
+	$(frame_base_h) $(gdb_assert_h)
 typeprint.o: typeprint.c $(defs_h) $(gdb_obstack_h) $(bfd_h) $(symtab_h) \
 	$(gdbtypes_h) $(expression_h) $(value_h) $(gdbcore_h) $(command_h) \
 	$(gdbcmd_h) $(target_h) $(language_h) $(cp_abi_h) $(typeprint_h) \
Index: tramp-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.c,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.c
--- tramp-frame.c	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.c	23 Mar 2004 23:58:06 -0000
@@ -28,6 +28,7 @@
 #include "target.h"
 #include "trad-frame.h"
 #include "frame-base.h"
+#include "gdb_assert.h"
 
 struct frame_data
 {
@@ -89,15 +90,15 @@
   int ti;
   /* Search through the trampoline for one that matches the
      instruction sequence around PC.  */
-  for (ti = 0; tramp->insn[ti] != 0; ti++)
+  for (ti = 0; tramp->insn[ti] != TRAMP_SENTINEL_INSN; ti++)
     {
       CORE_ADDR func = pc - tramp->insn_size * ti;
       int i;
       for (i = 0; 1; i++)
 	{
-	  bfd_byte buf[sizeof (LONGEST)];
+	  bfd_byte buf[sizeof (tramp->insn[0])];
 	  CORE_ADDR insn;
-	  if (tramp->insn[i] == 0)
+	  if (tramp->insn[i] == TRAMP_SENTINEL_INSN)
 	    return func;
 	  if (target_read_memory (func + i * tramp->insn_size, buf,
 				  tramp->insn_size) != 0)
@@ -148,6 +149,15 @@
 {
   struct frame_data *data;
   struct frame_unwind *unwinder;
+  int i;
+
+  /* Check that the instruction sequence contains a sentinel.  */
+  for (i = 0; i < ARRAY_SIZE (tramp_frame->insn); i++)
+    {
+      if (tramp_frame->insn[i] == TRAMP_SENTINEL_INSN)
+	break;
+    }
+  gdb_assert (i < ARRAY_SIZE (tramp_frame->insn));
 
   data = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_data);
   unwinder = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_unwind);
Index: tramp-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.h,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.h
--- tramp-frame.h	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.h	23 Mar 2004 23:58:06 -0000
@@ -39,6 +39,10 @@
 
 /* A trampoline descriptor.  */
 
+/* Magic instruction that to mark the end of the signal trampoline
+   instruction sequence (zero for the moment).  */
+#define TRAMP_SENTINEL_INSN ((LONGEST) -1)
+
 struct tramp_frame
 {
   /* The trampoline's entire instruction sequence.  Search for this in
@@ -47,7 +51,8 @@
      one INSN_SIZE instruction.  It is also assumed that TRAMP[0]
      contains the first instruction of the trampoline and hence the
      address of the instruction matching TRAMP[0] is the trampoline's
-     "func" address.  */
+     "func" address.  The instruction sequence shall be terminated by
+     TRAMP_SENTINEL_INSN.  */
   int insn_size;
   ULONGEST insn[8];
   /* Initialize a trad-frame cache corresponding to the tramp-frame.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch/rfc] signal trampoline frames
  2004-03-24  0:02   ` Andrew Cagney
@ 2004-03-24 17:15     ` Daniel Jacobowitz
  2004-03-24 23:18       ` Andrew Cagney
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2004-03-24 17:15 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Tue, Mar 23, 2004 at 07:02:45PM -0500, Andrew Cagney wrote:
> see attached?
> 
> >
> >>>+static CORE_ADDR
> >>>+tramp_frame_start (CORE_ADDR pc, const struct tramp_frame *tramp)
> >>>+{
> >>>+  int ti;
> >>>+  /* Search through the trampoline for one that matches the
> >>>+     instruction sequence around PC.  */
> >>>+  for (ti = 0; tramp->insn[ti] != 0; ti++)
> >>>+    {
> >>>+      CORE_ADDR func = pc - tramp->insn_size * ti;
> >>>+      int i;
> >>>+      for (i = 0; 1; i++)
> >>>+	{
> >>>+	  bfd_byte buf[sizeof (LONGEST)];
> >>>+	  CORE_ADDR insn;
> >
> >
> >tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
> >also.
> 
> changed to ->insn[0]

I was more concerned about the CORE_ADDR.

> >>>+	  if (tramp->insn[i] == 0)
> >>>+	    return func;
> >
> >
> >So zeros in tramp->insn mark the end of the sequence?  Should document
> >that, zeros are valid instructions and some bizarre architecture might
> >use one as a syscall trap.
> 
> Added TRAMP_SENTINEL_INSN, it _isn't_ zero.

The comment in the attached still says it's zero.  Zero and -1 are just
about equally likely/unlikely, so I don't think it matters what the
value is; I like having TRAMP_SENTINEL_INSN though.

> I don't know.

Doesn't much matter then.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch/rfc] signal trampoline frames
  2004-03-24 17:15     ` Daniel Jacobowitz
@ 2004-03-24 23:18       ` Andrew Cagney
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cagney @ 2004-03-24 23:18 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

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


>>>> >tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
>>>> >also.
>>
>>> 
>>> changed to ->insn[0]
> 
> 
> I was more concerned about the CORE_ADDR.

Ah, "both".

>>>>>> >>>+	  if (tramp->insn[i] == 0)
>>>>>> >>>+	    return func;
>>>
>>>> >
>>>> >
>>>> >So zeros in tramp->insn mark the end of the sequence?  Should document
>>>> >that, zeros are valid instructions and some bizarre architecture might
>>>> >use one as a syscall trap.
>>
>>> 
>>> Added TRAMP_SENTINEL_INSN, it _isn't_ zero.
> 
> 
> The comment in the attached still says it's zero.  Zero and -1 are just
> about equally likely/unlikely, so I don't think it matters what the
> value is; I like having TRAMP_SENTINEL_INSN though.

(LONGEST)-1 is typically 0xffffffffffffffffLL so unless an ISA has a 
64-bit insn there won't be a clash.

>>> I don't know.
> 
> 
> Doesn't much matter then.

I committed the attached,

Andrew


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

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

	* tramp-frame.h (TRAMP_SENTINEL_INSN): Define, document.
	* tramp-frame.c: Include "gdb_assert.h".
	(tramp_frame_start): Use TRAMP_SENTINEL_INSN.  Use ULONGEST and
	correct sizeof.
	(tramp_frame_append): Validate the tramp frame's instructions.
	* Makefile.in (tramp-frame.o): Update dependencies.

Index: tramp-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.c,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.c
--- tramp-frame.c	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.c	24 Mar 2004 23:13:23 -0000
@@ -28,6 +28,7 @@
 #include "target.h"
 #include "trad-frame.h"
 #include "frame-base.h"
+#include "gdb_assert.h"
 
 struct frame_data
 {
@@ -89,15 +90,15 @@
   int ti;
   /* Search through the trampoline for one that matches the
      instruction sequence around PC.  */
-  for (ti = 0; tramp->insn[ti] != 0; ti++)
+  for (ti = 0; tramp->insn[ti] != TRAMP_SENTINEL_INSN; ti++)
     {
       CORE_ADDR func = pc - tramp->insn_size * ti;
       int i;
       for (i = 0; 1; i++)
 	{
-	  bfd_byte buf[sizeof (LONGEST)];
-	  CORE_ADDR insn;
-	  if (tramp->insn[i] == 0)
+	  bfd_byte buf[sizeof (tramp->insn[0])];
+	  ULONGEST insn;
+	  if (tramp->insn[i] == TRAMP_SENTINEL_INSN)
 	    return func;
 	  if (target_read_memory (func + i * tramp->insn_size, buf,
 				  tramp->insn_size) != 0)
@@ -148,6 +149,16 @@
 {
   struct frame_data *data;
   struct frame_unwind *unwinder;
+  int i;
+
+  /* Check that the instruction sequence contains a sentinel.  */
+  for (i = 0; i < ARRAY_SIZE (tramp_frame->insn); i++)
+    {
+      if (tramp_frame->insn[i] == TRAMP_SENTINEL_INSN)
+	break;
+    }
+  gdb_assert (i < ARRAY_SIZE (tramp_frame->insn));
+  gdb_assert (tramp_frame->insn_size <= sizeof (tramp_frame->insn[0]));
 
   data = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_data);
   unwinder = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_unwind);
Index: tramp-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.h,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.h
--- tramp-frame.h	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.h	24 Mar 2004 23:13:23 -0000
@@ -39,6 +39,10 @@
 
 /* A trampoline descriptor.  */
 
+/* Magic instruction that to mark the end of the signal trampoline
+   instruction sequence.  */
+#define TRAMP_SENTINEL_INSN ((LONGEST) -1)
+
 struct tramp_frame
 {
   /* The trampoline's entire instruction sequence.  Search for this in
@@ -47,7 +51,8 @@
      one INSN_SIZE instruction.  It is also assumed that TRAMP[0]
      contains the first instruction of the trampoline and hence the
      address of the instruction matching TRAMP[0] is the trampoline's
-     "func" address.  */
+     "func" address.  The instruction sequence shall be terminated by
+     TRAMP_SENTINEL_INSN.  */
   int insn_size;
   ULONGEST insn[8];
   /* Initialize a trad-frame cache corresponding to the tramp-frame.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-03-24 23:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-22  3:14 [patch/rfc] signal trampoline frames Andrew Cagney
2004-03-23 14:13 ` Andrew Cagney
2004-03-23 23:09 ` Daniel Jacobowitz
2004-03-24  0:02   ` Andrew Cagney
2004-03-24 17:15     ` Daniel Jacobowitz
2004-03-24 23:18       ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox