This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/4] Distinguish sentinel frame from null frame


On Wed, 26 Oct 2016 00:21:53 -0700
Kevin Buettner <kevin@buettner.to> wrote:

> > >  /* Cache for frame addresses already read by gdb.  Valid only while
> > >     inferior is stopped.  Control variables for the frame cache should
> > >     be local to this module.  */
> > > @@ -1511,6 +1527,9 @@ static struct frame_info *get_prev_frame_always_1 (struct frame_info *this_frame
> > >  struct frame_info *
> > >  get_current_frame (void)
> > >  {
> > > +  struct frame_info *current_frame;
> > > +  int new_sentinel_p = 0;
> > > +  /* Set the current frame before computing the frame id, to avoid
> > > +     recursion inside compute_frame_id, in case the frame's
> > > +     unwinder decides to do a symbol lookup (which depends on the
> > > +     selected frame's block).
> > > +
> > > +     This call must always succeed.  In particular, nothing inside
> > > +     get_prev_frame_always_1 should try to unwind from the
> > > +     sentinel frame, because that could fail/throw, and we always
> > > +     want to leave with the current frame created and linked in --
> > > +     we should never end up with the sentinel frame as outermost
> > > +     frame.  */
> > > +  current_frame = get_prev_frame_always_1 (sentinel_frame);
> > > +  gdb_assert (current_frame != NULL);
> > > +
> > > +  /* The call to get_frame_id, below, is not necessary when the stack is
> > > +     well behaved.  But when it's not well behaved, we want to ensure
> > > +     that the frame id for the current frame is known (stashed) prior to
> > > +     finding frame id values for any outer frames.
> > > +
> > > +     The test gdb.dwarf2/dw2-dup-frame.exp shows an internal error
> > > +     in the "backtrace from stop_frame" test when we don't do this here.  */
> > > +  if (new_sentinel_p)
> > > +    get_frame_id (current_frame);    
[...]
> An alternate "fix" (or perhaps band-aid) for this problem is as
> follows:
> 
> diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
> index 6bdac08..a1d305e 100644
> --- a/gdb/python/py-frame.c
> +++ b/gdb/python/py-frame.c
> @@ -375,6 +375,10 @@ frame_info_to_frame_object (struct frame_info *frame)
>    TRY
>      {
>  
> +      /* Avoid case where the frame id for previous frame is computed
> +        before that for the frame under consideration.  */
> +      (void) get_frame_id (frame);
> +
>        /* Try to get the previous frame, to determine if this is the last frame
>  	 in a corrupt stack.  If so, we need to store the frame_id of the next
>  	 frame and not of this one (which is possibly invalid).  */
> 
> I'm not especially fond of this solution though.  If it's necessary to
> create frame ids in a certain sequence, this ordering should be
> imposed elsewhere.  Also, I've caught just one case here.  There may
> also be similar problems lurking which we haven't caught yet.
> 
> I hope you that you (or someone else) can suggest a better solution...

FYI, I found a solution that I like better.  I plan to submit this
patch - or something like it - as part of a new patch series for
preventing recursion in python based unwinders.

So...  no need to review it now, but (I hope that) there's also no
need to spend time looking for the "better solution" requested in my
earlier post.

diff --git a/gdb/frame.c b/gdb/frame.c
index bc42674..b19e782 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2217,6 +2217,18 @@ get_prev_frame (struct frame_info *this_frame)
      something should be calling get_selected_frame() or
      get_current_frame().  */
   gdb_assert (this_frame != NULL);
+  
+  /* If this_frame is the current frame, then compute and stash
+     its frame id prior to fetching and computing the frame id of the
+     previous frame.  Otherwise, the cycle detection code in
+     get_prev_frame_if_no_cycle() will not work correctly.  When
+     get_frame_id() is called later on, an assertion error will
+     be triggered in the event of a cycle between the current
+     frame and its previous frame.  */
+     
+  if (this_frame->level == 0)
+    get_frame_id (this_frame);
+
   frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc);
 
   /* tausq/2004-12-07: Dummy frames are skipped because it doesn't make much


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]