blarsen / rpms / gdb

Forked from rpms/gdb 2 years ago
Clone
ec6bc35
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
ec6bc35
From: Pedro Alves <pedro@palves.net>
ec6bc35
Date: Sat, 31 Oct 2020 00:27:18 +0000
ec6bc35
Subject: gdb-rhbz1909902-frame_id_p-assert-2.patch
ec6bc35
ec6bc35
;; Backport patch #2 which fixes a frame_id_p assertion failure (RH BZ 1909902).
ec6bc35
ec6bc35
Fix frame cycle detection
ec6bc35
ec6bc35
The recent commit to make scoped_restore_current_thread's cdtors
ec6bc35
exception free regressed gdb.base/eh_return.exp:
ec6bc35
ec6bc35
  Breakpoint 1, 0x00000000004012bb in eh2 (gdb/frame.c:641: internal-error: frame_id get_frame_id(frame_info*): Assertion `stashed' failed.
ec6bc35
  A problem internal to GDB has been detected,
ec6bc35
  further debugging may prove unreliable.
ec6bc35
  Quit this debugging session? (y or n) FAIL: gdb.base/eh_return.exp: hit breakpoint (GDB internal error)
ec6bc35
ec6bc35
That testcase uses __builtin_eh_return and, before the regression, the
ec6bc35
backtrace at eh2 looked like this:
ec6bc35
ec6bc35
 (gdb) bt
ec6bc35
 #0  0x00000000004006eb in eh2 (p=0x4006ec <continuation>) at src/gdb/testsuite/gdb.base/eh_return.c:54
ec6bc35
 Backtrace stopped: previous frame identical to this frame (corrupt stack?)
ec6bc35
ec6bc35
That "previous frame identical to this frame" is caught by the cycle
ec6bc35
detection based on frame id.
ec6bc35
ec6bc35
The assertion failing is this one:
ec6bc35
ec6bc35
 638           /* Since this is the first frame in the chain, this should
ec6bc35
 639              always succeed.  */
ec6bc35
 640           bool stashed = frame_stash_add (fi);
ec6bc35
 641           gdb_assert (stashed);
ec6bc35
ec6bc35
originally added by
ec6bc35
ec6bc35
  commit f245535cf583ae4ca13b10d47b3c7d3334593ece
ec6bc35
  Author:     Pedro Alves <palves@redhat.com>
ec6bc35
  AuthorDate: Mon Sep 5 18:41:38 2016 +0100
ec6bc35
ec6bc35
      Fix PR19927: Avoid unwinder recursion if sniffer uses calls parse_and_eval
ec6bc35
ec6bc35
The assertion is failing because frame #1's frame id was stashed
ec6bc35
before the id of frame #0 is stashed.  The frame id of frame #1 was
ec6bc35
stashed here:
ec6bc35
ec6bc35
 (top-gdb) bt
ec6bc35
 #0  frame_stash_add (frame=0x1e24c90) at src/gdb/frame.c:276
ec6bc35
 #1  0x0000000000669c1b in get_prev_frame_if_no_cycle (this_frame=0x19f8370) at src/gdb/frame.c:2120
ec6bc35
 #2  0x000000000066a339 in get_prev_frame_always_1 (this_frame=0x19f8370) at src/gdb/frame.c:2303
ec6bc35
 #3  0x000000000066a360 in get_prev_frame_always (this_frame=0x19f8370) at src/gdb/frame.c:2319
ec6bc35
 #4  0x000000000066b56c in get_frame_unwind_stop_reason (frame=0x19f8370) at src/gdb/frame.c:3028
ec6bc35
 #5  0x000000000059f929 in dwarf2_frame_cfa (this_frame=0x19f8370) at src/gdb/dwarf2/frame.c:1462
ec6bc35
 #6  0x00000000005ce434 in dwarf_evaluate_loc_desc::get_frame_cfa (this=0x7fffffffc070) at src/gdb/dwarf2/loc.c:666
ec6bc35
 #7  0x00000000005989a9 in dwarf_expr_context::execute_stack_op (this=0x7fffffffc070, op_ptr=0x1b2a053 "\364\003", op_end=0x1b2a053 "\364\003") at src/gdb/dwarf2/expr.c:1161
ec6bc35
 #8  0x0000000000596af6 in dwarf_expr_context::eval (this=0x7fffffffc070, addr=0x1b2a052 "\234\364\003", len=1) at src/gdb/dwarf2/expr.c:303
ec6bc35
 #9  0x0000000000597b4e in dwarf_expr_context::execute_stack_op (this=0x7fffffffc070, op_ptr=0x1b2a063 "", op_end=0x1b2a063 "") at src/gdb/dwarf2/expr.c:865
ec6bc35
 #10 0x0000000000596af6 in dwarf_expr_context::eval (this=0x7fffffffc070, addr=0x1b2a061 "\221X", len=2) at src/gdb/dwarf2/expr.c:303
ec6bc35
 #11 0x00000000005c8b5a in dwarf2_evaluate_loc_desc_full (type=0x1b564d0, frame=0x19f8370, data=0x1b2a061 "\221X", size=2, per_cu=0x1b28760, per_objfile=0x1a84930, subobj_type=0x1b564d0, subobj_byte_offset=0) at src/gdb/dwarf2/loc.c:2260
ec6bc35
 #12 0x00000000005c9243 in dwarf2_evaluate_loc_desc (type=0x1b564d0, frame=0x19f8370, data=0x1b2a061 "\221X", size=2, per_cu=0x1b28760, per_objfile=0x1a84930) at src/gdb/dwarf2/loc.c:2444
ec6bc35
 #13 0x00000000005cb769 in locexpr_read_variable (symbol=0x1b59840, frame=0x19f8370) at src/gdb/dwarf2/loc.c:3687
ec6bc35
 #14 0x0000000000663137 in language_defn::read_var_value (this=0x122ea60 <c_language_defn>, var=0x1b59840, var_block=0x0, frame=0x19f8370) at src/gdb/findvar.c:618
ec6bc35
 #15 0x0000000000663c3b in read_var_value (var=0x1b59840, var_block=0x0, frame=0x19f8370) at src/gdb/findvar.c:822
ec6bc35
 #16 0x00000000008c7d9f in read_frame_arg (fp_opts=..., sym=0x1b59840, frame=0x19f8370, argp=0x7fffffffc470, entryargp=0x7fffffffc490) at src/gdb/stack.c:542
ec6bc35
 #17 0x00000000008c89cd in print_frame_args (fp_opts=..., func=0x1b597c0, frame=0x19f8370, num=-1, stream=0x1aba860) at src/gdb/stack.c:890
ec6bc35
 #18 0x00000000008c9bf8 in print_frame (fp_opts=..., frame=0x19f8370, print_level=0, print_what=SRC_AND_LOC, print_args=1, sal=...) at src/gdb/stack.c:1394
ec6bc35
 #19 0x00000000008c92b9 in print_frame_info (fp_opts=..., frame=0x19f8370, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at src/gdb/stack.c:1119
ec6bc35
 #20 0x00000000008c75f0 in print_stack_frame (frame=0x19f8370, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1) at src/gdb/stack.c:366
ec6bc35
 #21 0x000000000070250b in print_stop_location (ws=0x7fffffffc9e0) at src/gdb/infrun.c:8110
ec6bc35
 #22 0x0000000000702569 in print_stop_event (uiout=0x1a8b9e0, displays=true) at src/gdb/infrun.c:8126
ec6bc35
 #23 0x000000000096d04b in tui_on_normal_stop (bs=0x1bcd1c0, print_frame=1) at src/gdb/tui/tui-interp.c:98
ec6bc35
 ...
ec6bc35
ec6bc35
Before the commit to make scoped_restore_current_thread's cdtors
ec6bc35
exception free, scoped_restore_current_thread's dtor would call
ec6bc35
get_frame_id on the selected frame, and we use
ec6bc35
scoped_restore_current_thread pervasively.  That had the side effect
ec6bc35
of stashing the frame id of frame #0 before reaching the path shown in
ec6bc35
the backtrace.  I.e., the frame id of frame #0 happened to be stashed
ec6bc35
before the frame id of frame #1.  But that was by chance, not by
ec6bc35
design.
ec6bc35
ec6bc35
This commit:
ec6bc35
ec6bc35
  commit 256ae5dbc73d1348850f86ee77a0dc3b04bc7cc0
ec6bc35
  Author:     Kevin Buettner <kevinb@redhat.com>
ec6bc35
  AuthorDate: Mon Oct 31 12:47:42 2016 -0700
ec6bc35
ec6bc35
      Stash frame id of current frame before stashing frame id for previous frame
ec6bc35
ec6bc35
Fixed a similar problem, by making sure get_prev_frame computes the
ec6bc35
frame id of the current frame before unwinding the previous frame, so
ec6bc35
that the cycle detection works properly.  That fix misses the scenario
ec6bc35
we're now running against, because if you notice, the backtrace above
ec6bc35
shows that frame #4 calls get_prev_frame_always, not get_prev_frame.
ec6bc35
I.e., nothing is calling get_frame_id on the current frame.
ec6bc35
ec6bc35
The fix here is to move Kevin's fix down from get_prev_frame to
ec6bc35
get_prev_frame_always.  Or actually, a bit further down to
ec6bc35
get_prev_frame_always_1 -- note that inline_frame_this_id calls
ec6bc35
get_prev_frame_always, so we need to be careful to avoid recursion in
ec6bc35
that scenario.
ec6bc35
ec6bc35
gdb/ChangeLog:
ec6bc35
ec6bc35
	* frame.c (get_prev_frame): Move get_frame_id call from here ...
ec6bc35
	(get_prev_frame_always_1): ... to here.
ec6bc35
	* inline-frame.c (inline_frame_this_id): Mention
ec6bc35
	get_prev_frame_always_1 in comment.
ec6bc35
ec6bc35
Change-Id: Id960c98ab2d072c48a436c3eb160cc4b2a5cfd1d
ec6bc35
ec6bc35
diff --git a/gdb/frame.c b/gdb/frame.c
ec6bc35
--- a/gdb/frame.c
ec6bc35
+++ b/gdb/frame.c
ec6bc35
@@ -2133,6 +2133,23 @@ get_prev_frame_always_1 (struct frame_info *this_frame)
ec6bc35
   if (get_frame_type (this_frame) == INLINE_FRAME)
ec6bc35
     return get_prev_frame_if_no_cycle (this_frame);
ec6bc35
 
ec6bc35
+  /* If this_frame is the current frame, then compute and stash its
ec6bc35
+     frame id prior to fetching and computing the frame id of the
ec6bc35
+     previous frame.  Otherwise, the cycle detection code in
ec6bc35
+     get_prev_frame_if_no_cycle() will not work correctly.  When
ec6bc35
+     get_frame_id() is called later on, an assertion error will be
ec6bc35
+     triggered in the event of a cycle between the current frame and
ec6bc35
+     its previous frame.
ec6bc35
+
ec6bc35
+     Note we do this after the INLINE_FRAME check above.  That is
ec6bc35
+     because the inline frame's frame id computation needs to fetch
ec6bc35
+     the frame id of its previous real stack frame.  I.e., we need to
ec6bc35
+     avoid recursion in that case.  This is OK since we're sure the
ec6bc35
+     inline frame won't create a cycle with the real stack frame.  See
ec6bc35
+     inline_frame_this_id.  */
ec6bc35
+  if (this_frame->level == 0)
ec6bc35
+    get_frame_id (this_frame);
ec6bc35
+
ec6bc35
   /* Check that this frame is unwindable.  If it isn't, don't try to
ec6bc35
      unwind to the prev frame.  */
ec6bc35
   this_frame->stop_reason
ec6bc35
@@ -2410,16 +2427,6 @@ get_prev_frame (struct frame_info *this_frame)
ec6bc35
      something should be calling get_selected_frame() or
ec6bc35
      get_current_frame().  */
ec6bc35
   gdb_assert (this_frame != NULL);
ec6bc35
-  
ec6bc35
-  /* If this_frame is the current frame, then compute and stash
ec6bc35
-     its frame id prior to fetching and computing the frame id of the
ec6bc35
-     previous frame.  Otherwise, the cycle detection code in
ec6bc35
-     get_prev_frame_if_no_cycle() will not work correctly.  When
ec6bc35
-     get_frame_id() is called later on, an assertion error will
ec6bc35
-     be triggered in the event of a cycle between the current
ec6bc35
-     frame and its previous frame.  */
ec6bc35
-  if (this_frame->level == 0)
ec6bc35
-    get_frame_id (this_frame);
ec6bc35
 
ec6bc35
   frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc);
ec6bc35
 
ec6bc35
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
ec6bc35
--- a/gdb/inline-frame.c
ec6bc35
+++ b/gdb/inline-frame.c
ec6bc35
@@ -161,7 +161,8 @@ inline_frame_this_id (struct frame_info *this_frame,
ec6bc35
      real frame's this_id method.  So we must call
ec6bc35
      get_prev_frame_always.  Because we are inlined into some
ec6bc35
      function, there must be previous frames, so this is safe - as
ec6bc35
-     long as we're careful not to create any cycles.  */
ec6bc35
+     long as we're careful not to create any cycles.  See related
ec6bc35
+     comments in get_prev_frame_always_1.  */
ec6bc35
   *this_id = get_frame_id (get_prev_frame_always (this_frame));
ec6bc35
 
ec6bc35
   /* We need a valid frame ID, so we need to be based on a valid