Re: [PATCH] Remove zero PC check from blockframe.c:inside_main_func()

[Mark Kettenis <kettenis@chello.nl>]

   Date: Sat, 13 Dec 2003 14:02:10 -0500
   From: Andrew Cagney <cagney@gnu.org>

   > It really makes no sense to check for a zero PC here.  This function
   > is only colled from frame.c:get_prev_frame(), and there we already
   > deal with PC being zero.
   > 
   > The whole concept of using a zero PC as a marker for the end of the
   > frame chain is somewhat flawed.  It prevents us from providing a
   > meaningful backtrace when the program has called a null function
   > pointer; see backtrace/1476.  At the very least we will have to treat
   > a zero PC in the innermost differently.  Classifying the a zero PC as
   > being inside the "main" function doesn't help.  Therefore this patch
   > removes the first obstackle in fixing that PR.
   > 
   > Objections.  Otherwise I'll commit this within a few days.

   FYI, this was made active with:

	    * blockframe.c: Include "gdbcmd.h" and "command.h".
	    (backtrace_below_main): New variable.
	    (file_frame_chain_valid, func_frame_chain_valid)
	    (nonnull_frame_chain_valid, generic_file_frame_chain_valid)
	    (generic_func_frame_chain_valid): Remove functions.
	    (frame_chain_valid, do_flush_frames_sfunc): New functions.
	    (_initialize_blockframe): New function.
	    * Makefile.in (blockframe.o): Update dependencies.
	    * frame.c (frame_saved_regs_id_unwind, get_prev_frame): Remove 
   FIXME
	    comment.  Call frame_chain_valid ().
	    * frame.h: Remove old prototypes.  Add prototype for
	    frame_chain_valid and update comments to match.
	    * gdbarch.sh: Change FRAME_CHAIN_VALID into a predicated function.
	    Remove old comment.
	    * gdbarch.h: Regenerated.
	    * gdbarch.c: Regenerated.

   rather than the new frame code.

Well yes, but ...

   I looked at the new frame code and apart from the wild-card logic, there 
   weren't any obvious PC==0 tests.

... there is a slightly non-obvious PC == 0 test in get_prev_frame():

   ... 
   if (frame_pc_unwind (this_frame) == 0)
     {
       ...
       return NULL;
     }
   ...

We should probably trust the sentinel frame here, and allow it to
unwind.  That is consistent with what we do earlier on.  What about
the attached patch?  Together with the blockframe patch (you're not
actually objecting to that one are you?), this fixes backtrace/1476.

Mark


Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.153
diff -u -p -r1.153 frame.c
--- frame.c 10 Dec 2003 17:40:42 -0000 1.153
+++ frame.c 13 Dec 2003 21:47:43 -0000
@@ -1732,6 +1732,7 @@ struct frame_info *
 get_prev_frame (struct frame_info *this_frame)
 {
   struct frame_info *prev_frame;
+  CORE_ADDR pc;
 
   if (frame_debug)
     {
@@ -1961,7 +1962,8 @@ get_prev_frame (struct frame_info *this_
      because (well ignoring the PPC) a dummy frame can be located
      using THIS_FRAME's frame ID.  */
 
-  if (frame_pc_unwind (this_frame) == 0)
+  pc = frame_pc_unwind (this_frame);
+  if (this_frame->level >= 0 && pc == 0)
     {
       /* The allocated PREV_FRAME will be reclaimed when the frame
 	 obstack is next purged.  */