diff mbox series

gdb/frame: add FRAME_SCOPED_DEBUG_START_END

Message ID 2ce7d5465de836f6180411a699471dc165e61b74.1780476716.git.aburgess@redhat.com
State New
Headers
Series gdb/frame: add FRAME_SCOPED_DEBUG_START_END |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply

Commit Message

Andrew Burgess June 3, 2026, 8:52 a.m. UTC 
  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(-)


base-commit: 663cb9c0428068165fc98675599442163d302d41

Comments

Tom Tromey June 3, 2026, 12:47 p.m. UTC | #1 
>>>>> "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
Andrew Burgess June 4, 2026, 9:34 a.m. UTC | #2 
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
diff mbox series

Patch

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).