This is the mail archive of the gdb-cvs@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]

[binutils-gdb] Stash frame id of current frame before stashing frame id for previous frame


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=256ae5dbc73d1348850f86ee77a0dc3b04bc7cc0

commit 256ae5dbc73d1348850f86ee77a0dc3b04bc7cc0
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Mon Oct 31 12:47:42 2016 -0700

    Stash frame id of current frame before stashing frame id for previous frame
    
    This patch ensures that the frame id for the current frame is stashed
    before that of the previous frame (to the current frame).
    
    First, it should be noted that the frame id for the current frame is
    not stashed by get_current_frame().  The current frame's frame id is
    lazily computed and stashed via calls to get_frame_id().  However,
    it's possible for get_prev_frame() to be called without first stashing
    the current frame.
    
    The frame stash is used not only to speed up frame lookups, but
    also to detect cycles.  When attempting to compute the frame id
    for a "previous" frame (in get_prev_frame_if_no_cycle), a cycle
    is detected if the computed frame id is already in the stash.
    
    If it should happen that a previous frame id is stashed which should
    represent a cycle for the current frame, then an assertion failure
    will trigger should get_frame_id() be later called to determine
    the frame id for the current frame.
    
    As of late 2016, with the "Tweak meaning of VALUE_FRAME_ID" patch in
    place, this actually occurs when running the
    gdb.dwarf2/dw2-dup-frame.exp test.  While attempting to generate a
    backtrace, the python frame filter code is invoked, leading to
    frame_info_to_frame_object() (in python/py-frame.c) being called.
    That function will potentially call get_prev_frame() before
    get_frame_id() is called.  The call to get_prev_frame() can eventually
    end up in get_prev_frame_if_no_cycle() which, in turn, calls
    compute_frame_id(), after which the frame id is stashed for the
    previous frame.
    
    If the frame id for the current frame is stashed, the cycle detection
    code (which relies on the frame stash) in get_prev_frame_if_no_cycle()
    will be triggered for a cycle starting with the current frame.  If the
    current frame's id is not stashed, the cycle detecting code can't
    operate as designed.  Instead, when get_frame_id() is called on the
    current frame at some later point, the current frame's id will found
    to be already in the stash, triggering an assertion failure.
    
    Below is an in depth examination of the failure which lead to this change.
    I've shortened pathnames for brevity and readability.
    
    Here's the portion of the log file showing the failure/internal error:
    
    (gdb) break stop_frame
    Breakpoint 1 at 0x40059a: file dw2-dup-frame.c, line 22.
    (gdb) run
    Starting program: testsuite/outputs/gdb.dwarf2/dw2-dup-frame/dw2-dup-frame
    
    Breakpoint 1, stop_frame () at dw2-dup-frame.c:22
    22	}
    (gdb) bt
    gdb/frame.c:544: internal-error: frame_id get_frame_id(frame_info*): Assertion `stashed' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)
    FAIL: gdb.dwarf2/dw2-dup-frame.exp: backtrace from stop_frame (GDB internal error)
    
    Here's a partial backtrace from the internal error, showing the frames
    which I think are relevant, plus several extra to provide context:
    
        #0  internal_error (
    	file=0x932b98 "gdb/frame.c", line=544,
    	fmt=0x932b20 "%s: Assertion `%s' failed.")
    	at gdb/common/errors.c:54
        #1  0x000000000072207e in get_frame_id (fi=0xe5a760)
    	at gdb/frame.c:544
        #2  0x00000000004eb50d in frame_info_to_frame_object (frame=0xe5a760)
    	at gdb/python/py-frame.c:390
        #3  0x00000000004ef5be in bootstrap_python_frame_filters (frame=0xe5a760,
    	frame_low=0, frame_high=-1)
    	at gdb/python/py-framefilter.c:1453
        #4  0x00000000004ef7a9 in gdbpy_apply_frame_filter (
    	extlang=0x8857e0 <extension_language_python>, frame=0xe5a760, flags=7,
    	args_type=CLI_SCALAR_VALUES, out=0xf6def0, frame_low=0, frame_high=-1)
    	at gdb/python/py-framefilter.c:1548
        #5  0x00000000005f2c5a in apply_ext_lang_frame_filter (frame=0xe5a760,
    	flags=7, args_type=CLI_SCALAR_VALUES, out=0xf6def0, frame_low=0,
    	frame_high=-1)
    	at gdb/extension.c:572
        #6  0x00000000005ea896 in backtrace_command_1 (count_exp=0x0, show_locals=0,
    	no_filters=0, from_tty=1)
    	at gdb/stack.c:1834
    
    Examination of the code in frame_info_to_frame_object(), which is in
    python/py-frame.c, is key to understanding this problem:
    
          if (get_prev_frame (frame) == NULL
    	  && get_frame_unwind_stop_reason (frame) != UNWIND_NO_REASON
    	  && get_next_frame (frame) != NULL)
    	{
    	  frame_obj->frame_id = get_frame_id (get_next_frame (frame));
    	  frame_obj->frame_id_is_next = 1;
    	}
          else
    	{
    	  frame_obj->frame_id = get_frame_id (frame);
    	  frame_obj->frame_id_is_next = 0;
    	}
    
    I will first note that the frame id for frame has not been computed yet.  (This
    was verified by placing a breakpoint on compute_frame_id().)
    
    The call to get_prev_frame() causes the the frame id to (eventually) be
    computed for the previous frame.  Here's a backtrace showing how we
    get there:
    
        #0  compute_frame_id (fi=0x10e2810)
    	at gdb/frame.c:496
        #1  0x0000000000724a67 in get_prev_frame_if_no_cycle (this_frame=0xe5a760)
    	at gdb/frame.c:1871
        #2  0x0000000000725136 in get_prev_frame_always_1 (this_frame=0xe5a760)
    	at gdb/frame.c:2045
        #3  0x000000000072516b in get_prev_frame_always (this_frame=0xe5a760)
    	at gdb/frame.c:2061
        #4  0x000000000072570f in get_prev_frame (this_frame=0xe5a760)
    	at gdb/frame.c:2303
        #5  0x00000000004eb471 in frame_info_to_frame_object (frame=0xe5a760)
    	at gdb/python/py-frame.c:381
    
    For this particular case, we end up in the else clause of the code above
    which calls get_frame_id (frame).  It's at this point that the frame id
    for frame is computed.  Again, here's a backtrace:
    
        #0  compute_frame_id (fi=0xe5a760)
    	at gdb/frame.c:496
        #1  0x000000000072203d in get_frame_id (fi=0xe5a760)
    	at gdb/frame.c:539
        #2  0x00000000004eb50d in frame_info_to_frame_object (frame=0xe5a760)
    	at gdb/python/py-frame.c:390
    
    The test in question, dw2-dup-frame.exp, deliberately creates a broken
    (cyclic) stack.  So, in this instance, the frame id for the prev
    `frame' will be the same as that for `frame'.  But that particular
    frame id ended up in the stash during the previous frame operation.
    When, just a few lines later, we compute the frame id for `frame', the
    id in question is already in the stash, thus triggering the assertion
    failure.
    
    I considered two other solutions to solving this problem:
    
    We could prevent get_prev_frame() from being called before
    get_frame_id() in frame_info_to_frame_object().  (See above for the
    snippet of code where this happens.) A call to get_frame_id (frame)
    could be placed ahead of that code snippet above.  I have tested this
    approach and, while it does work, I can't be certain that
    get_prev_frame() isn't called ahead of stashing the current frame
    somewhere else in GDB, but in a less obvious way.
    
    Another approach is to stash the current frame's id by calling
    get_frame_id() in get_current_frame().  This approach is conceptually
    simpler, but when importing a python unwinder, has the unwelcome side
    effect of causing the unwinder to be called during import.
    
    A cleaner looking fix would be to place this code after code
    corresponding to the "Don't compute the frame id of the current frame
    yet..." comment in get_prev_frame_if_no_cycle().  Sadly, this does not
    work though; by the time we get to this point, the frame state for the
    prev frame has been modified just enough to cause an internal error to
    occur when attempting to compute the (current) frame id for inline
    frames.  (The unexpected failure count increases by roughly 130
    failures.)  Therefore, I decided to place it as early as possible
    in get_prev_frame().
    
    gdb/ChangeLog:
    
    	* frame.c (get_prev_frame): Stash frame id for current frame
    	prior to computing frame id for previous frame.

Diff:
---
 gdb/ChangeLog |  5 +++++
 gdb/frame.c   | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5b53dad..77180ef 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-16  Kevin Buettner  <kevinb@redhat.com>
 
+	* frame.c (get_prev_frame): Stash frame id for current frame
+	prior to computing frame id for previous frame.
+
+2016-11-16  Kevin Buettner  <kevinb@redhat.com>
+
 	* python/py-unwind.c (pending_framepy_read_register): Use
 	value_of_register() instead of get_frame_register_value().
 
diff --git a/gdb/frame.c b/gdb/frame.c
index 02635e6..5414cb3 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2220,6 +2220,17 @@ 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]