Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch, rfc, 6.0] Change frame predicates to sniffers
@ 2003-07-11 21:57 Andrew Cagney
  2003-07-15 17:41 ` Andrew Cagney
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cagney @ 2003-07-11 21:57 UTC (permalink / raw)
  To: gdb-patches

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

Hello,

This patch adds frame sniffers (name stolen from other sniffer code) to 
the frame code.  Unlike the existing frame predicate methods that take a 
PC parameter, these take a full NEXT_FRAME.  It leaves the old frame 
predicates alone (making this safe for the branch).

I figure that should be enough to keep any unwinder, trying to decide if 
the frame is good, happy :-)

I'll look to commit this to both the trunk and 6.0 branch early next 
week.  I'll then follow up with patches to:

- rips out the old predicate methods (mainline only)

- replace the dwarf2 predicate with a dwarf2 sniffer
Will apply to mainline and branch.  Should fix weird edge case where a 
function has no cfi but is doing a tail call to a no-return function, 
but the code following the function does have CFI.

- (I guess) something to work around the dwarf2 unwind info (need to 
look at Mark's old code)

I've so far tested it on a RH9 system (with the sniffer enabled) and it 
hasn't shown a sign of regressing.

Can anyone point me at testcases that will magically start passing with 
this fixed?

Andrew

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

2003-07-11  Andrew Cagney  <cagney@redhat.com>

	* frame.c (get_frame_id): Use frame_unwind_find_by_frame.
	(frame_register_unwind, create_new_frame): Ditto.
	(legacy_get_prev_frame, get_frame_type): Ditto.
	(get_frame_base_address): Use frame_base_find_by_frame.
	(get_frame_locals_address): Use frame_base_find_by_frame.
	(get_frame_args_address): Use frame_base_find_by_frame.
	* frame-base.h (frame_base_sniffer_ftype): Declare.
	(frame_base_append_sniffer): Declare.
	(frame_base_find_by_frame): Replace frame_base_find_by_pc.
	* frame-base.c (append_predicate): Add a "sniffer" parameter.
	(frame_base_append_sniffer): New function.
	(frame_base_append_predicate): Add a NULL sniffer.
	(frame_base_find_by_frame): Replace "frame_base_find_by_pc".
	(struct frame_base_table): Add "sniffer".
	(frame_base_free): Free the "sniffer" table.
	* frame-unwind.h (frame_unwind_sniffer_ftype): Define.
	(frame_unwind_append_sniffer): Declare.
	(frame_unwind_find_by_frame): Replace frame_unwind_find_by_pc.
	* frame-unwind.c (frame_unwind_free): Free the "sniffer" table.
	(struct frame_unwind_table): Add "sniffer", delete "middle".
	(append_predicate): Add "sniffer" parameter, append the sniffer.
	(frame_unwind_init): Update append_predicate call.
	(frame_unwind_append_sniffer): New function.
	(frame_unwind_append_predicate): Update append_predicate call.
	(frame_unwind_find_by_frame): Replace frame_unwind_find_by_pc.

Index: frame-base.c
===================================================================
RCS file: /cvs/src/src/gdb/frame-base.c,v
retrieving revision 1.5
diff -u -r1.5 frame-base.c
--- frame-base.c	26 Jun 2003 17:18:41 -0000	1.5
+++ frame-base.c	11 Jul 2003 21:40:14 -0000
@@ -71,6 +71,7 @@
 struct frame_base_table
 {
   frame_base_p_ftype **p;
+  frame_base_sniffer_ftype **sniffer;
   const struct frame_base *default_base;
   int nr;
 };
@@ -89,6 +90,7 @@
   struct frame_base_table *table =
     gdbarch_data (gdbarch, frame_base_data);
   xfree (table->p);
+  xfree (table->sniffer);
   xfree (table);
 }
 
@@ -108,11 +110,16 @@
 
 /* Append a predicate to the end of the table.  */
 static void
-append_predicate (struct frame_base_table *table, frame_base_p_ftype *p)
+append_predicate (struct frame_base_table *table, frame_base_p_ftype *p,
+		  frame_base_sniffer_ftype *sniffer)
 {
   table->p = xrealloc (table->p, ((table->nr + 1)
 				  * sizeof (frame_base_p_ftype *)));
+  table->sniffer = xrealloc (table->sniffer,
+			     ((table->nr + 1)
+			      * sizeof (frame_base_sniffer_ftype *)));
   table->p[table->nr] = p;
+  table->sniffer[table->nr] = sniffer;
   table->nr++;
 }
 
@@ -121,7 +128,15 @@
 			     frame_base_p_ftype *p)
 {
   struct frame_base_table *table = frame_base_table (gdbarch);
-  append_predicate (table, p);
+  append_predicate (table, p, NULL);
+}
+
+void
+frame_base_append_sniffer (struct gdbarch *gdbarch,
+			   frame_base_sniffer_ftype *sniffer)
+{
+  struct frame_base_table *table = frame_base_table (gdbarch);
+  append_predicate (table, NULL, sniffer);
 }
 
 void
@@ -133,13 +148,18 @@
 }
 
 const struct frame_base *
-frame_base_find_by_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
+frame_base_find_by_frame (struct frame_info *next_frame)
 {
-  int i;
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
   struct frame_base_table *table = frame_base_table (gdbarch);
+  int i;
   for (i = 0; i < table->nr; i++)
     {
-      const struct frame_base *desc = table->p[i] (pc);
+      const struct frame_base *desc = NULL;
+      if (table->p[i] != NULL)
+	desc = table->p[i] (frame_pc_unwind (next_frame));
+      else if (table->sniffer[i] != NULL)
+	desc = table->sniffer[i] (next_frame);
       if (desc != NULL)
 	return desc;
     }
Index: frame-base.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-base.h,v
retrieving revision 1.1
diff -u -r1.1 frame-base.h
--- frame-base.h	1 Apr 2003 19:11:01 -0000	1.1
+++ frame-base.h	11 Jul 2003 21:40:14 -0000
@@ -29,30 +29,12 @@
 struct gdbarch;
 struct regcache;
 
-/* Return the frame base methods for the function that contains PC, or
-   NULL if it can't handle this frame.  */
+/* For compatibility.  */
 
 typedef const struct frame_base *(frame_base_p_ftype) (CORE_ADDR pc);
-
-/* Add a frame base handler to the list.  The predicates are polled in
-   the order that they are appended.  */
-
 extern void frame_base_append_predicate (struct gdbarch *gdbarch,
 					 frame_base_p_ftype *p);
 
-/* Set the default frame base.  If all else fails, this one is
-   returned.  If this isn't set, the default is to use legacy code
-   that uses things like the frame ID's base (ulgh!).  */
-
-extern void frame_base_set_default (struct gdbarch *gdbarch,
-				    const struct frame_base *def);
-
-/* Iterate through the list of frame base handlers until one returns
-   an implementation.  */
-
-extern const struct frame_base *frame_base_find_by_pc (struct gdbarch *gdbarch,
-						       CORE_ADDR pc);
-
 /* Assuming the frame chain: (outer) prev <-> this <-> next (inner);
    and that this is a `normal frame'; use the NEXT frame, and its
    register unwind method, to determine the address of THIS frame's
@@ -90,5 +72,28 @@
   frame_this_locals_ftype *this_locals;
   frame_this_args_ftype *this_args;
 };
+
+/* Given the NEXT frame, return the frame base methods for THIS frame,
+   or NULL if it can't handle THIS frame.  */
+
+typedef const struct frame_base *(frame_base_sniffer_ftype) (struct frame_info *next_frame);
+
+/* Append a frame base sniffer to the list.  The sniffers are polled
+   in the order that they are appended.  */
+
+extern void frame_base_append_sniffer (struct gdbarch *gdbarch,
+				       frame_base_sniffer_ftype *sniffer);
+
+/* Set the default frame base.  If all else fails, this one is
+   returned.  If this isn't set, the default is to use legacy code
+   that uses things like the frame ID's base (ulgh!).  */
+
+extern void frame_base_set_default (struct gdbarch *gdbarch,
+				    const struct frame_base *def);
+
+/* Iterate through the list of frame base handlers until one returns
+   an implementation.  */
+
+extern const struct frame_base *frame_base_find_by_frame (struct frame_info *next_frame);
 
 #endif
Index: frame-unwind.c
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.c,v
retrieving revision 1.4
diff -u -r1.4 frame-unwind.c
--- frame-unwind.c	8 Jun 2003 18:27:13 -0000	1.4
+++ frame-unwind.c	11 Jul 2003 21:40:14 -0000
@@ -30,17 +30,21 @@
 struct frame_unwind_table
 {
   frame_unwind_p_ftype **p;
-  int middle;
+  frame_unwind_sniffer_ftype **sniffer;
   int nr;
 };
 
 /* Append a predicate to the end of the table.  */
 static void
-append_predicate (struct frame_unwind_table *table, frame_unwind_p_ftype *p)
+append_predicate (struct frame_unwind_table *table, frame_unwind_p_ftype *p,
+		  frame_unwind_sniffer_ftype *sniffer)
 {
   table->p = xrealloc (table->p, ((table->nr + 1)
 				  * sizeof (frame_unwind_p_ftype *)));
+  table->sniffer = xrealloc (table->sniffer, ((table->nr + 1)
+					      * sizeof (frame_unwind_sniffer_ftype *)));
   table->p[table->nr] = p;
+  table->sniffer[table->nr] = sniffer;
   table->nr++;
 }
 
@@ -48,7 +52,7 @@
 frame_unwind_init (struct gdbarch *gdbarch)
 {
   struct frame_unwind_table *table = XCALLOC (1, struct frame_unwind_table);
-  append_predicate (table, dummy_frame_p);
+  append_predicate (table, dummy_frame_p, NULL);
   return table;
 }
 
@@ -58,6 +62,7 @@
   struct frame_unwind_table *table =
     gdbarch_data (gdbarch, frame_unwind_data);
   xfree (table->p);
+  xfree (table->sniffer);
   xfree (table);
 }
 
@@ -74,15 +79,31 @@
       table = frame_unwind_init (gdbarch);
       set_gdbarch_data (gdbarch, frame_unwind_data, table);
     }
-  append_predicate (table, p);
+  append_predicate (table, p, NULL);
 }
 
-const struct frame_unwind *
-frame_unwind_find_by_pc (struct gdbarch *gdbarch, CORE_ADDR pc)
+void
+frame_unwind_append_sniffer (struct gdbarch *gdbarch,
+			     frame_unwind_sniffer_ftype *sniffer)
 {
-  int i;
   struct frame_unwind_table *table =
     gdbarch_data (gdbarch, frame_unwind_data);
+  if (table == NULL)
+    {
+      /* ULGH, called during architecture initialization.  Patch
+         things up.  */
+      table = frame_unwind_init (gdbarch);
+      set_gdbarch_data (gdbarch, frame_unwind_data, table);
+    }
+  append_predicate (table, NULL, sniffer);
+}
+
+const struct frame_unwind *
+frame_unwind_find_by_frame (struct frame_info *next_frame)
+{
+  int i;
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  struct frame_unwind_table *table = gdbarch_data (gdbarch, frame_unwind_data);
   if (!DEPRECATED_USE_GENERIC_DUMMY_FRAMES)
     /* Seriously old code.  Don't even try to use this new mechanism.
        (Note: The variable USE_GENERIC_DUMMY_FRAMES is deprecated, not
@@ -91,7 +112,13 @@
     return legacy_saved_regs_unwind;
   for (i = 0; i < table->nr; i++)
     {
-      const struct frame_unwind *desc = table->p[i] (pc);
+      const struct frame_unwind *desc;
+      if (table->p[i] != NULL)
+	desc = table->p[i] (frame_pc_unwind (next_frame));
+      else if (table->sniffer[i] != NULL)
+	desc = table->sniffer[i] (next_frame);
+      else
+	internal_error (__FILE__, __LINE__, "Missing sniffer?");
       if (desc != NULL)
 	return desc;
     }
Index: frame-unwind.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.h,v
retrieving revision 1.7
diff -u -r1.7 frame-unwind.h
--- frame-unwind.h	5 Apr 2003 03:55:59 -0000	1.7
+++ frame-unwind.h	11 Jul 2003 21:40:14 -0000
@@ -30,25 +30,12 @@
 
 #include "frame.h"		/* For enum frame_type.  */
 
-/* Return the frame unwind methods for the function that contains PC,
-   or NULL if this this unwinder can't handle this frame.  */
-
+/* For compatibility with the old frame code.  See end of header for
+   new methods.  */
 typedef const struct frame_unwind *(frame_unwind_p_ftype) (CORE_ADDR pc);
-
-/* Add a frame unwinder to the list.  The predicates are polled in the
-   order that they are appended.  The initial list contains the dummy
-   frame's predicate.  */
-
 extern void frame_unwind_append_predicate (struct gdbarch *gdbarch,
 					   frame_unwind_p_ftype *p);
 
-/* Iterate through the list of frame unwinders until one returns an
-   implementation.  */
-
-extern const struct frame_unwind *frame_unwind_find_by_pc (struct gdbarch
-							   *gdbarch,
-							   CORE_ADDR pc);
-
 /* The following unwind functions assume a chain of frames forming the
    sequence: (outer) prev <-> this <-> next (inner).  All the
    functions are called with called with the next frame's `struct
@@ -138,5 +125,23 @@
   frame_this_id_ftype *this_id;
   frame_prev_register_ftype *prev_register;
 };
+
+/* Given the NEXT frame, take a wiff of THIS frame's registers (namely
+   the PC and attributes) and if it is the applicable unwinder return
+   the unwind methods, or NULL if it is not.  */
+
+typedef const struct frame_unwind *(frame_unwind_sniffer_ftype) (struct frame_info *next_frame);
+
+/* Add a frame sniffer to the list.  The predicates are polled in the
+   order that they are appended.  The initial list contains the dummy
+   frame sniffer.  */
+
+extern void frame_unwind_append_sniffer (struct gdbarch *gdbarch,
+					 frame_unwind_sniffer_ftype *sniffer);
+
+/* Iterate through the next frame's sniffers until one returns with an
+   unwinder implementation.  */
+
+extern const struct frame_unwind *frame_unwind_find_by_frame (struct frame_info *next_frame);
 
 #endif
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.131
diff -u -r1.131 frame.c
--- frame.c	11 Jul 2003 14:52:17 -0000	1.131
+++ frame.c	11 Jul 2003 21:40:16 -0000
@@ -229,8 +229,7 @@
       /* Find the unwinder.  */
       if (fi->unwind == NULL)
 	{
-	  fi->unwind = frame_unwind_find_by_pc (current_gdbarch,
-						get_frame_pc (fi));
+	  fi->unwind = frame_unwind_find_by_frame (fi->next);
 	  /* FIXME: cagney/2003-04-02: Rather than storing the frame's
 	     type in the frame, the unwinder's type should be returned
 	     directly.  Unfortunatly, legacy code, called by
@@ -517,8 +516,7 @@
   /* Find the unwinder.  */
   if (frame->unwind == NULL)
     {
-      frame->unwind = frame_unwind_find_by_pc (current_gdbarch,
-					       get_frame_pc (frame));
+      frame->unwind = frame_unwind_find_by_frame (frame->next);
       /* FIXME: cagney/2003-04-02: Rather than storing the frame's
 	 type in the frame, the unwinder's type should be returned
 	 directly.  Unfortunatly, legacy code, called by
@@ -1206,7 +1204,7 @@
 
   /* Select/initialize both the unwind function and the frame's type
      based on the PC.  */
-  fi->unwind = frame_unwind_find_by_pc (current_gdbarch, pc);
+  fi->unwind = frame_unwind_find_by_frame (fi->next);
   if (fi->unwind->type != UNKNOWN_FRAME)
     fi->type = fi->unwind->type;
   else
@@ -1365,8 +1363,7 @@
 
       /* Set the unwind functions based on that identified PC.  Ditto
          for the "type" but strongly prefer the unwinder's frame type.  */
-      prev->unwind = frame_unwind_find_by_pc (current_gdbarch,
-					      get_frame_pc (prev));
+      prev->unwind = frame_unwind_find_by_frame (prev->next);
       if (prev->unwind->type == UNKNOWN_FRAME)
 	prev->type = frame_type_from_pc (get_frame_pc (prev));
       else
@@ -1514,8 +1511,7 @@
              to the new frame code.  Implement FRAME_CHAIN the way the
              new frame will.  */
 	  /* Find PREV frame's unwinder.  */
-	  prev->unwind = frame_unwind_find_by_pc (current_gdbarch,
-						  frame_pc_unwind (this_frame));
+	  prev->unwind = frame_unwind_find_by_frame (this_frame->next);
 	  /* FIXME: cagney/2003-04-02: Rather than storing the frame's
 	     type in the frame, the unwinder's type should be returned
 	     directly.  Unfortunatly, legacy code, called by
@@ -1676,8 +1672,7 @@
      If there isn't a FRAME_CHAIN, the code above will have already
      done this.  */
   if (prev->unwind == NULL)
-    prev->unwind = frame_unwind_find_by_pc (current_gdbarch,
-					    get_frame_pc (prev));
+    prev->unwind = frame_unwind_find_by_frame (prev->next);
 
   /* If the unwinder provides a frame type, use it.  Otherwize
      continue on to that heuristic mess.  */
@@ -2072,7 +2067,7 @@
   if (get_frame_type (fi) != NORMAL_FRAME)
     return 0;
   if (fi->base == NULL)
-    fi->base = frame_base_find_by_pc (current_gdbarch, get_frame_pc (fi));
+    fi->base = frame_base_find_by_frame (fi->next);
   /* Sneaky: If the low-level unwind and high-level base code share a
      common unwinder, let them share the prologue cache.  */
   if (fi->base->unwind == fi->unwind)
@@ -2088,7 +2083,7 @@
     return 0;
   /* If there isn't a frame address method, find it.  */
   if (fi->base == NULL)
-    fi->base = frame_base_find_by_pc (current_gdbarch, get_frame_pc (fi));
+    fi->base = frame_base_find_by_frame (fi->next);
   /* Sneaky: If the low-level unwind and high-level base code share a
      common unwinder, let them share the prologue cache.  */
   if (fi->base->unwind == fi->unwind)
@@ -2106,7 +2101,7 @@
     return 0;
   /* If there isn't a frame address method, find it.  */
   if (fi->base == NULL)
-    fi->base = frame_base_find_by_pc (current_gdbarch, get_frame_pc (fi));
+    fi->base = frame_base_find_by_frame (fi->next);
   /* Sneaky: If the low-level unwind and high-level base code share a
      common unwinder, let them share the prologue cache.  */
   if (fi->base->unwind == fi->unwind)
@@ -2145,8 +2140,7 @@
     {
       /* Initialize the frame's unwinder because it is that which
          provides the frame's type.  */
-      frame->unwind = frame_unwind_find_by_pc (current_gdbarch,
-					       get_frame_pc (frame));
+      frame->unwind = frame_unwind_find_by_frame (frame->next);
       /* FIXME: cagney/2003-04-02: Rather than storing the frame's
 	 type in the frame, the unwinder's type should be returned
 	 directly.  Unfortunatly, legacy code, called by

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch, rfc, 6.0] Change frame predicates to sniffers
@ 2003-07-12 19:19 Michael Elizabeth Chastain
  2003-07-13 16:55 ` Mark Kettenis
  2003-07-15 15:56 ` Andrew Cagney
  0 siblings, 2 replies; 6+ messages in thread
From: Michael Elizabeth Chastain @ 2003-07-12 19:19 UTC (permalink / raw)
  To: ac131313, gdb-patches

My test bed says: no regressions, no improvements.

Tested on native i686-pc-linux-gnu, gcc v2 and v3 (several flavors
of v3), -gdwarf-2 and -gstabs+.

> Can anyone point me at testcases that will magically start passing with 
> this fixed?

If you want test scripts, try:

  gdb.base/corefile.exp: backtrace in corefile.exp
  gdb.base/corefile.exp: print func2::coremaker_local
  gdb.mi/mi-syn-frame.exp: 407-stack-list-frames
  gdb.threads/pthreads.exp: check backtrace from main thread
  gdb.threads/pthreads.exp: apply backtrace command to all three threads
  gdb.threads/pthreads.exp: check backtrace from thread 2

Or download the attachments from pr gdb/1250, pr gdb/1253,
and pr gdb/1255, and play with those.

Michael C

===

2003-07-11  Andrew Cagney  <cagney@redhat.com>

	* frame.c (get_frame_id): Use frame_unwind_find_by_frame.
	(frame_register_unwind, create_new_frame): Ditto.
	(legacy_get_prev_frame, get_frame_type): Ditto.
	(get_frame_base_address): Use frame_base_find_by_frame.
	(get_frame_locals_address): Use frame_base_find_by_frame.
	(get_frame_args_address): Use frame_base_find_by_frame.
	* frame-base.h (frame_base_sniffer_ftype): Declare.
	(frame_base_append_sniffer): Declare.
	(frame_base_find_by_frame): Replace frame_base_find_by_pc.
	* frame-base.c (append_predicate): Add a "sniffer" parameter.
	(frame_base_append_sniffer): New function.
	(frame_base_append_predicate): Add a NULL sniffer.
	(frame_base_find_by_frame): Replace "frame_base_find_by_pc".
	(struct frame_base_table): Add "sniffer".
	(frame_base_free): Free the "sniffer" table.
	* frame-unwind.h (frame_unwind_sniffer_ftype): Define.
	(frame_unwind_append_sniffer): Declare.
	(frame_unwind_find_by_frame): Replace frame_unwind_find_by_pc.
	* frame-unwind.c (frame_unwind_free): Free the "sniffer" table.
	(struct frame_unwind_table): Add "sniffer", delete "middle".
	(append_predicate): Add "sniffer" parameter, append the sniffer.
	(frame_unwind_init): Update append_predicate call.
	(frame_unwind_append_sniffer): New function.
	(frame_unwind_append_predicate): Update append_predicate call.
	(frame_unwind_find_by_frame): Replace frame_unwind_find_by_pc.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch, rfc, 6.0] Change frame predicates to sniffers
@ 2003-07-14  5:35 Michael Elizabeth Chastain
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Elizabeth Chastain @ 2003-07-14  5:35 UTC (permalink / raw)
  To: kettenis; +Cc: ac131313, gdb-patches

Mark Kettenis writes:
> For your testcases, the instructions after the function call are
> optimized away.  This means that the return address of the called
> frame points to the first instruction of the next function.  The
> prologue analysis is done for this function instead of the function
> that actually made the call.

Gotcha.

> The new prologue analyzer notices that at the first instruction of a
> function, the frame-pointer hasn't been setup yet, and the frame
> unwinder uses the stack-pointer for unwinding.  This fails because we
> haven't actually called the function we're analyzing.

Aha!

This is consistent with the stack dump I attached to pr gdb/1250.
gdb picks out a word for the program counter that would be correct
if the caller looked like this:

  func2:
    call abort
    push %ebp
    mov %ebp, %esp

This is a good candidate for a standalone test, gdb.base/gdb1250.exp,
with a KFAIL in it.  I'll write such a test tomorrow.

Michael C


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-07-15 17:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-11 21:57 [patch, rfc, 6.0] Change frame predicates to sniffers Andrew Cagney
2003-07-15 17:41 ` Andrew Cagney
2003-07-12 19:19 Michael Elizabeth Chastain
2003-07-13 16:55 ` Mark Kettenis
2003-07-15 15:56 ` Andrew Cagney
2003-07-14  5:35 Michael Elizabeth Chastain

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