[PATCH] gdb/frame: add FRAME_SCOPED_DEBUG_START_END

[PATCH] gdb/frame: add FRAME_SCOPED_DEBUG_START_END

[Andrew Burgess]

Add the FRAME_SCOPED_DEBUG_START_END macro, and make use of it in a
couple of places.

I find the frame debug output a little noisy, and it could be easier
to match up the entry/exit lines, by making use of
FRAME_SCOPED_DEBUG_START_END to print the frame level.

While I was adding the FRAME_SCOPED_DEBUG_START_END calls, there were
a couple of places where I also moved a variable declaration from the
top the a function into the function body.  And in another couple of
places there was some debug output that could be removed, where the
existing debug just printed the frame level (this is now handled by
the START/END macro).

There should be no user visible changes after this commit unless 'set
debug frame on' is used of course.
---
 gdb/frame.c | 24 ++++++------------------
 gdb/frame.h |  5 +++++
 2 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 61d37316c6a..179e251eb1f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -619,7 +619,7 @@ skip_tailcall_frames (const frame_info_ptr &initial_frame)
 static void
 compute_frame_id (const frame_info_ptr &fi)
 {
-  FRAME_SCOPED_DEBUG_ENTER_EXIT;
+  FRAME_SCOPED_DEBUG_START_END ("fi=%d", fi->level);
 
   gdb_assert (fi->this_id.p == frame_id_status::NOT_COMPUTED);
 
@@ -630,8 +630,6 @@ compute_frame_id (const frame_info_ptr &fi)
       /* Mark this frame's id as "being computed.  */
       fi->this_id.p = frame_id_status::COMPUTING;
 
-      frame_debug_printf ("fi=%d", fi->level);
-
       /* Find the unwinder.  */
       if (fi->unwind == NULL)
 	frame_unwind_find_by_frame (fi, &fi->prologue_cache);
@@ -2349,18 +2347,10 @@ get_prev_frame_maybe_check_cycle (const frame_info_ptr &this_frame)
 static frame_info_ptr
 get_prev_frame_always_1 (const frame_info_ptr &this_frame)
 {
-  FRAME_SCOPED_DEBUG_ENTER_EXIT;
+  FRAME_SCOPED_DEBUG_START_END ("fi=%d", this_frame->level);
 
   gdb_assert (this_frame != NULL);
 
-  if (frame_debug)
-    {
-      if (this_frame != NULL)
-	frame_debug_printf ("this_frame=%d", this_frame->level);
-      else
-	frame_debug_printf ("this_frame=nullptr");
-    }
-
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
 
   /* Only try to do the unwind once.  */
@@ -2593,7 +2583,7 @@ get_prev_frame_always (const frame_info_ptr &this_frame)
 static frame_info_ptr
 get_prev_frame_raw (const frame_info_ptr &this_frame)
 {
-  frame_info *prev_frame;
+  FRAME_SCOPED_DEBUG_START_END ("fi=%d", this_frame->level);
 
   /* Allocate the new frame but do not wire it in to the frame chain.
      Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
@@ -2605,7 +2595,7 @@ get_prev_frame_raw (const frame_info_ptr &this_frame)
      quickly reclaimed when the frame cache is flushed, and the `we've
      been here before' check above will stop repeated memory
      allocation calls.  */
-  prev_frame = frame_obstack_zalloc<frame_info> ();
+  frame_info *prev_frame = frame_obstack_zalloc<frame_info> ();
   prev_frame->level = this_frame->level + 1;
 
   /* For now, assume we don't have frame chains crossing address
@@ -2729,16 +2719,14 @@ inside_entry_func (const frame_info_ptr &this_frame)
 frame_info_ptr
 get_prev_frame (const frame_info_ptr &this_frame)
 {
-  FRAME_SCOPED_DEBUG_ENTER_EXIT;
-
-  std::optional<CORE_ADDR> frame_pc;
+  FRAME_SCOPED_DEBUG_START_END ("fi=%d", this_frame->level);
 
   /* There is always a frame.  If this assertion fails, suspect that
      something should be calling get_selected_frame() or
      get_current_frame().  */
   gdb_assert (this_frame != NULL);
 
-  frame_pc = get_frame_pc_if_available (this_frame);
+  std::optional<CORE_ADDR> frame_pc = get_frame_pc_if_available (this_frame);
 
   /* tausq/2004-12-07: Dummy frames are skipped because it doesn't make much
      sense to stop unwinding at a dummy frame.  One place where a dummy
diff --git a/gdb/frame.h b/gdb/frame.h
index 00485e6d1e2..f6553fb7b6d 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -125,6 +125,11 @@ extern bool frame_debug;
 #define FRAME_SCOPED_DEBUG_ENTER_EXIT \
   scoped_debug_enter_exit (frame_debug, "frame")
 
+/* Print "frame" start/end debug statements.  */
+
+#define FRAME_SCOPED_DEBUG_START_END(fmt, ...) \
+  scoped_debug_start_end (frame_debug, "frame", fmt, ##__VA_ARGS__)
+
 /* Construct a frame ID.  The first parameter is the frame's constant
    stack address (typically the outer-bound), and the second the
    frame's constant code address (typically the entry point).

base-commit: 663cb9c0428068165fc98675599442163d302d41
-- 
2.25.4

Re: [PATCH] gdb/frame: add FRAME_SCOPED_DEBUG_START_END

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> There should be no user visible changes after this commit unless 'set
Andrew> debug frame on' is used of course.

Seems reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] gdb/frame: add FRAME_SCOPED_DEBUG_START_END

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> There should be no user visible changes after this commit unless 'set
> Andrew> debug frame on' is used of course.
>
> Seems reasonable to me.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew