Re: RFC: Mostly kill FRAME_CHAIN_VALID, add user knob

[Andrew Cagney <ac131313@redhat.com>]

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

> Pretty gross, neh?  Well, file vs. func is merely a question of whether we
> stop at main or not, so I added "set backtrace-below-main" in order to let
> the user choose.  Generic vs. not is a question of dummy frames, and the
> generic versions work with non-generic dummy frames, so there's no reason
> for that distinction earlier.  It won't harm those three m68k targets (if
> they still work) to use a more comprehensive frame_chain_valid.  And the
> five more specific ones up above can be retained, since they are only
> _additional_ checks.  I'm not entirely convinced that the Interix one is
> necessary but I left it alone.
> 
> So, after this patch we have FRAME_CHAIN_VALID as a predicated function that
> only five architectures define; everything else just uses the new
> frame_chain_valid () function, which is a more general version of
> generic_func_frame_chain_valid.
> 
> I'm more confident I got the texinfo right this time :)  I tested the patch
> and the new functionality on i386-linux and arm-elf, to make sure I got the
> details of FRAME_CHAIN_VALID_P () right.
> 
> I'll look to commit this in January, if no one has any comments.  Andrew,
> would you rather this went in frame.c?  Since a purpose of that file seems
> to be moving things from blockframe.c to it...

FYI,

Much of this is superseeded by the frame overhaul - in particular the 
introduction of frame_id_unwind().  The new code doesn't even call frame 
chain valid!

Perhaphs wait for the attached [wip] to be committed and then tweak that 
to match your proposed policy (we can then just deprecate 
FRAME_CHAIN_VALID_P :-).  However, making the change in parallel 
wouldn't hurt.

Looking at my WIP, I'll need to tweak the code segment:

+  prev_frame->pc = frame_pc_unwind (next_frame);
+  if (prev_frame->pc == 0)
+    /* The allocated PREV_FRAME will be reclaimed when the frame
+       obstack is next purged.  */
+    return NULL;
+  prev_frame->type = frame_type_from_pc (prev_frame->pc);

so that it checks for where the PC resides and abort accordingly.

The attached is WIP since I still need to see it working once :-)

Andrew


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

2002-12-18  Andrew Cagney  <ac131313@redhat.com>

	* frame.c (frame_type_from_pc): New function.
	(create_new_frame): Use frame_type_from_pc.
	(deprecated_get_prev_frame): New function.  Move the old style
	previous frame code to here....
	(get_prev_frame): ... from here.  Re-implement using
	deprecated_get_prev_frame, frame_pc_unwind, frame_id_unwind and
	frame_type_from_pc

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.47
diff -u -r1.47 frame.c
--- frame.c	13 Dec 2002 23:18:56 -0000	1.47
+++ frame.c	18 Dec 2002 19:07:42 -0000
@@ -846,6 +846,29 @@
     }
 }
 
+/* Determine the frame's type based on its PC.  */
+
+static enum frame_type
+frame_type_from_pc (CORE_ADDR pc)
+{
+  /* FIXME: cagney/2002-11-24: Can't yet directly call
+     pc_in_dummy_frame() as some architectures don't set
+     PC_IN_CALL_DUMMY() to generic_pc_in_call_dummy() (remember the
+     latter is implemented by simply calling pc_in_dummy_frame).  */
+  if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES
+      && DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0))
+    return DUMMY_FRAME;
+  else
+    {
+      char *name;
+      find_pc_partial_function (pc, &name, NULL, NULL);
+      if (PC_IN_SIGTRAMP (pc, name))
+	return SIGTRAMP_FRAME;
+      else
+	return NORMAL_FRAME;
+    }
+}
+
 /* Create an arbitrary (i.e. address specified by user) or innermost frame.
    Always returns a non-NULL value.  */
 
@@ -864,37 +887,14 @@
 
   fi->frame = addr;
   fi->pc = pc;
-  /* NOTE: cagney/2002-11-18: The code segments, found in
-     create_new_frame and get_prev_frame(), that initializes the
-     frames type is subtly different.  The latter only updates ->type
-     when it encounters a SIGTRAMP_FRAME or DUMMY_FRAME.  This stops
-     get_prev_frame() overriding the frame's type when the INIT code
-     has previously set it.  This is really somewhat bogus.  The
-     initialization, as seen in create_new_frame(), should occur
-     before the INIT function has been called.  */
-  if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES
-      && (DEPRECATED_PC_IN_CALL_DUMMY_P ()
-	  ? DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0)
-	  : pc_in_dummy_frame (pc)))
-    /* NOTE: cagney/2002-11-11: Does this even occure?  */
-    type = DUMMY_FRAME;
-  else
-    {
-      char *name;
-      find_pc_partial_function (pc, &name, NULL, NULL);
-      if (PC_IN_SIGTRAMP (fi->pc, name))
-	type = SIGTRAMP_FRAME;
-      else
-	type = NORMAL_FRAME;
-    }
-  fi->type = type;
+  fi->type = frame_type_from_pc (pc);
 
   if (INIT_EXTRA_FRAME_INFO_P ())
     INIT_EXTRA_FRAME_INFO (0, fi);
 
@@ -936,12 +936,10 @@
     }
 }
 
-/* Return a structure containing various interesting information
-   about the frame that called NEXT_FRAME.  Returns NULL
-   if there is no such frame.  */
+/* Create the previous frame using the old style methods.  */
 
-struct frame_info *
-get_prev_frame (struct frame_info *next_frame)
+static struct frame_info *
+deprecated_get_prev_frame (struct frame_info *next_frame)
 {
   CORE_ADDR address = 0;
   struct frame_info *prev;
@@ -1185,6 +1183,102 @@
     }
 
   return prev;
+}
+
+/* Return a structure containing various interesting information
+   about the frame that called NEXT_FRAME.  Returns NULL
+   if there is no such frame.  */
+
+struct frame_info *
+get_prev_frame (struct frame_info *next_frame)
+{
+  struct frame_info *prev_frame;
+
+  if (DEPRECATED_INIT_FRAME_PC_P ()
+      || DEPRECATED_INIT_FRAME_PC_FIRST_P ())
+    return deprecated_get_prev_frame (next_frame);
+
+  /* There is always a frame.  If this assertion fails, suspect that
+     something should be calling get_selected_frame() or
+     get_current_frame().  */
+  gdb_assert (next_frame != NULL);
+
+  /* Only try to do the unwind once.  */
+  if (next_frame->prev_p)
+    return next_frame->prev;
+  next_frame->prev_p = 1;
+
+  /* Allocate the new frame but do not wire it in.  Some (bad) code
+     tries to look along frame->next to pull some fancy tricks (of
+     course such code is, by definition, recursive).  Try to prevent
+     it.  */
+  prev_frame = frame_obstack_alloc (sizeof (struct frame_info));
+  memset (prev_frame, 0, sizeof (struct frame_info));
+  prev_frame->level = next_frame->level + 1;
+
+  /* Try to unwind the PC.  If that doesn't work, assume we've reached
+     the oldest frame and simply return.  Is there a better sentinal
+     value?  The unwound PC value is then used to initialize the new
+     previous frame's type.
+
+     Note that the pc-unwind is intentionally performed before the
+     frame chain.  This is ok since, for old targets, both
+     frame_pc_unwind (nee, FRAME_SAVED_PC) and FRAME_CHAIN()) assume
+     NEXT_FRAME's data structures have already been initialized (using
+     INIT_EXTRA_FRAME_INFO) and hence the call order doesn't matter.
+
+     By unwinding the PC first, it becomes possible to, in the case of
+     a dummy frame, avoid also unwinding the frame ID.  This is
+     because (well ignoring the PPC) a dummy frame can be located
+     using NEXT_FRAME's frame ID.  */
+
+  prev_frame->pc = frame_pc_unwind (next_frame);
+  if (prev_frame->pc == 0)
+    /* The allocated PREV_FRAME will be reclaimed when the frame
+       obstack is next purged.  */
+    return NULL;
+  prev_frame->type = frame_type_from_pc (prev_frame->pc);
+
+  /* Set the unwind functions based on that identified PC.  */
+  set_unwind_by_pc (prev_frame->pc, &prev_frame->register_unwind,
+		    &prev_frame->pc_unwind, &prev_frame->id_unwind);
+
+  /* Now figure out how to initialize this new frame.  Perhaphs one
+     day, this will too, be selected by set_unwind_by_pc().  */
+  if (prev_frame->type != DUMMY_FRAME)
+    {
+      /* A dummy frame doesn't need to unwind the frame ID because the
+	 frame ID comes from the previous frame.  The other frames do
+	 though.  True?  */
+#if 0
+      /* Oops, the frame doesn't chain.  Treat this as the last frame.  */
+      prev_frame->id = frame_id_unwind (next_frame);
+      if (!frame_id_p (prev_frame->id))
+	return NULL;
+#else      
+      /* FIXME: cagney/2002-12-18: Instead of this hack, should just
+	 save the frame ID directly.  */
+      struct frame_id id = frame_id_unwind (next_frame);
+      if (!frame_id_p (id))
+	return NULL;
+      prev_frame->frame = id.base;
+#endif
+    }
+
+  /* Link it in.  */
+  next_frame->prev = prev_frame;
+  prev_frame->next = next_frame;
+
+  /* NOTE: cagney/2002-12-18: Eventually this call will go away.
+     Instead of initializing extra info, all frames will use the
+     frame_cache (passed to the unwind functions) to store extra frame
+     info.  */
+  if (INIT_EXTRA_FRAME_INFO_P ())
+    /* NOTE: This code doesn't bother trying to sort out frameless
+       functions.  That is left to the target.  */
+    INIT_EXTRA_FRAME_INFO (0, prev_frame);
+
+  return prev_frame;
 }
 
 CORE_ADDR