[PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Klaus Gerlicher]

Some debug prints in breakpoint.c are using
infrun_debug_printf ().  This does not seem to be the
right debug print macro.

Use breakpoint_debug_printf () exclusively in this file.
---
 gdb/breakpoint.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d704ad1e3e2..a4eb337746c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2531,8 +2531,8 @@ should_be_inserted (struct bp_location *bl)
       && !(bl->owner->type == bp_single_step
 	   && thread_is_stepping_over_breakpoint (bl->owner->thread)))
     {
-      infrun_debug_printf ("skipping breakpoint: stepping past insn at: %s",
-			   paddress (bl->gdbarch, bl->address));
+      breakpoint_debug_printf ("skipping breakpoint: stepping past insn at: %s",
+			       paddress (bl->gdbarch, bl->address));
       return false;
     }
 
@@ -2541,9 +2541,9 @@ should_be_inserted (struct bp_location *bl)
   if ((bl->loc_type == bp_loc_hardware_watchpoint)
       && stepping_past_nonsteppable_watchpoint ())
     {
-      infrun_debug_printf ("stepping past non-steppable watchpoint. "
-			   "skipping watchpoint at %s:%d",
-			   paddress (bl->gdbarch, bl->address), bl->length);
+      breakpoint_debug_printf ("stepping past non-steppable watchpoint. "
+			       "skipping watchpoint at %s:%d",
+			       paddress (bl->gdbarch, bl->address), bl->length);
       return false;
     }
 
@@ -5679,9 +5679,9 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
   b = bs->breakpoint_at;
   gdb_assert (b != NULL);
 
-  infrun_debug_printf ("thread = %s, breakpoint %d.%d",
-		       thread->ptid.to_string ().c_str (),
-		       b->number, find_loc_num_by_location (bl));
+  breakpoint_debug_printf ("thread = %s, breakpoint %d.%d",
+			   thread->ptid.to_string ().c_str (),
+			   b->number, find_loc_num_by_location (bl));
 
   /* Even if the target evaluated the condition on its end and notified GDB, we
      need to do so again since GDB does not know if we stopped due to a
@@ -5690,9 +5690,9 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
   if (frame_id_p (b->frame_id)
       && b->frame_id != get_stack_frame_id (get_current_frame ()))
     {
-      infrun_debug_printf ("incorrect frame %s not %s, not stopping",
-			   get_stack_frame_id (get_current_frame ()).to_string ().c_str (),
-			   b->frame_id.to_string ().c_str ());
+      breakpoint_debug_printf ("incorrect frame %s not %s, not stopping",
+	get_stack_frame_id (get_current_frame ()).to_string ().c_str (),
+	b->frame_id.to_string ().c_str ());
       bs->stop = false;
       return;
     }
@@ -5704,7 +5704,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
       || (b->inferior != -1 && b->inferior != thread->inf->num)
       || (b->task != -1 && b->task != ada_get_task_number (thread)))
     {
-      infrun_debug_printf ("incorrect thread or task, not stopping");
+      breakpoint_debug_printf ("incorrect thread or task, not stopping");
       bs->stop = false;
       return;
     }
@@ -5803,14 +5803,14 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
 
   if (cond != nullptr && !condition_result)
     {
-      infrun_debug_printf ("condition_result = false, not stopping");
+      breakpoint_debug_printf ("condition_result = false, not stopping");
       bs->stop = false;
       return;
     }
   else if (b->ignore_count > 0)
     {
-      infrun_debug_printf ("ignore count %d, not stopping",
-			   b->ignore_count);
+      breakpoint_debug_printf ("ignore count %d, not stopping",
+			       b->ignore_count);
       b->ignore_count--;
       bs->stop = false;
       /* Increase the hit count even though we don't stop.  */
@@ -5820,9 +5820,9 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
     }
 
   if (bs->stop)
-    infrun_debug_printf ("stopping at this breakpoint");
+    breakpoint_debug_printf ("stopping at this breakpoint");
   else
-    infrun_debug_printf ("not stopping at this breakpoint");
+    breakpoint_debug_printf ("not stopping at this breakpoint");
 }
 
 /* Returns true if we need to track moribund locations of LOC's type
@@ -6044,7 +6044,7 @@ handle_jit_event (CORE_ADDR address)
 {
   struct gdbarch *gdbarch;
 
-  infrun_debug_printf ("handling bp_jit_event");
+  breakpoint_debug_printf ("handling bp_jit_event");
 
   /* Switch terminal for any messages produced by
      breakpoint_re_set.  */
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Kevin Buettner]

On Tue,  8 Jul 2025 13:21:23 +0000
Klaus Gerlicher <klaus.gerlicher@intel.com> wrote:

> From: Klaus Gerlicher <klaus.gerlicher@intel.com>
> To: gdb-patches@sourceware.org
> Subject: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf
> Date: Tue,  8 Jul 2025 13:21:23 +0000
> 
> Some debug prints in breakpoint.c are using
> infrun_debug_printf ().  This does not seem to be the
> right debug print macro.
> 
> Use breakpoint_debug_printf () exclusively in this file.

Approved-by: Kevin Buettner <kevinb@redhat.com>

Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Tom Tromey]

>>>>> "Klaus" == Klaus Gerlicher <klaus.gerlicher@intel.com> writes:

Klaus> Some debug prints in breakpoint.c are using
Klaus> infrun_debug_printf ().  This does not seem to be the
Klaus> right debug print macro.

Klaus> Use breakpoint_debug_printf () exclusively in this file.

I am not so sure about this.  It seems to me that some of these may
mostly of interest when infrun debugging is enabled.  For example:

Klaus> -      infrun_debug_printf ("incorrect thread or task, not stopping");
Klaus> +      breakpoint_debug_printf ("incorrect thread or task, not stopping");

This kind of thing is a decision impacting infrun decisions.

However I would defer to Pedro in this matter since he works on infrun
more than anyone else seems to.

thanks,
Tom

Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Kevin Buettner]

On Wed, 09 Jul 2025 13:21:14 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Klaus" == Klaus Gerlicher <klaus.gerlicher@intel.com> writes:  
> 
> Klaus> Some debug prints in breakpoint.c are using
> Klaus> infrun_debug_printf ().  This does not seem to be the
> Klaus> right debug print macro.  
> 
> Klaus> Use breakpoint_debug_printf () exclusively in this file.  
> 
> I am not so sure about this.  It seems to me that some of these may
> mostly of interest when infrun debugging is enabled.  For example:
> 
> Klaus> -      infrun_debug_printf ("incorrect thread or task, not
> Klaus> stopping");
> Klaus> +      breakpoint_debug_printf ("incorrect thread or task, not
> Klaus> stopping");  
> 
> This kind of thing is a decision impacting infrun decisions.

Hmm, good point.  I rescind my approval.

Kevin

RE: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Gerlicher, Klaus]

Hi, 

Thanks for the quick response. I see, we need to include Pedro then I guess. It's 
not terribly important, I was just adding some things to breakpoint for our purposes 
and I hit this oddity.

+Pedro

Thanks
Klaus

> -----Original Message-----
> From: Kevin Buettner <kevinb@redhat.com>
> Sent: Thursday, July 10, 2025 4:59 AM
> To: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org; Gerlicher, Klaus
> <klaus.gerlicher@intel.com>
> Subject: Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf
> 
> On Wed, 09 Jul 2025 13:21:14 -0600
> Tom Tromey <tom@tromey.com> wrote:
> 
> > >>>>> "Klaus" == Klaus Gerlicher <klaus.gerlicher@intel.com> writes:
> >
> > Klaus> Some debug prints in breakpoint.c are using
> > Klaus> infrun_debug_printf ().  This does not seem to be the
> > Klaus> right debug print macro.
> >
> > Klaus> Use breakpoint_debug_printf () exclusively in this file.
> >
> > I am not so sure about this.  It seems to me that some of these may
> > mostly of interest when infrun debugging is enabled.  For example:
> >
> > Klaus> -      infrun_debug_printf ("incorrect thread or task, not
> > Klaus> stopping");
> > Klaus> +      breakpoint_debug_printf ("incorrect thread or task, not
> > Klaus> stopping");
> >
> > This kind of thing is a decision impacting infrun decisions.
> 
> Hmm, good point.  I rescind my approval.
> 
> Kevin

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Simon Marchi]

On 7/10/25 4:43 AM, Gerlicher, Klaus wrote:
> Hi, 
> 
> Thanks for the quick response. I see, we need to include Pedro then I guess. It's 
> not terribly important, I was just adding some things to breakpoint for our purposes 
> and I hit this oddity.
> 
> +Pedro
> 
> Thanks
> Klaus

I agree with Tom, it's not just the file that dictates which "debug
print" function to use, it's also what the code does.  You have to ask
yourself, when would I want to see this debug print.  This is how I see
it:

 - "infrun" is about GDB controlling the threads, handling target
   events, resuming threads, stopping threads, etc.
 - "breakpoint" is about the actions of inserting or removing
   breakpoints, computing the locations of breakpoints, etc.

You have to ask yourself, when would you like to see each statement.
The code in bpstat_check_breakpoint_conditions, for instance, is about
evaluating a breakpoint condition after a thread has hit it.  I think
these debug prints are useful to have when debugging infrun issues,
when you ask yourself "why did GDB decide to stop/not stop this thread".

It's possible that some of these infrun prints should be breakpoint
prints, but it needs to be more thought out.

I see that breakpoint_debug_printf is only used at 5 spots, which is
surprising, since the breakpoint machinery is rather complex and core to
GDB.  It is relatively recent (2023), so it perhaps just means we need
to add more of them, when we debug issues.

Simon

RE: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf

[Andrew Burgess]

"Gerlicher, Klaus" <klaus.gerlicher@intel.com> writes:

> Hi, 
>
> Thanks for the quick response. I see, we need to include Pedro then I guess. It's 
> not terribly important, I was just adding some things to breakpoint for our purposes 
> and I hit this oddity.

For what it's worth I agree with Tom and Simon that these should not be
changed to breakpoint debug.  In my head the debug printing has always
been based on functionality rather than file, and these do seem to be
more about inferior control than breakpoint creation/management.

Thanks,
Andrew


>
> +Pedro
>
> Thanks
> Klaus
>
>> -----Original Message-----
>> From: Kevin Buettner <kevinb@redhat.com>
>> Sent: Thursday, July 10, 2025 4:59 AM
>> To: Tom Tromey <tom@tromey.com>
>> Cc: gdb-patches@sourceware.org; Gerlicher, Klaus
>> <klaus.gerlicher@intel.com>
>> Subject: Re: [PATCH 1/1] gdb, breakpoint: use breakpoint_debug_printf
>> 
>> On Wed, 09 Jul 2025 13:21:14 -0600
>> Tom Tromey <tom@tromey.com> wrote:
>> 
>> > >>>>> "Klaus" == Klaus Gerlicher <klaus.gerlicher@intel.com> writes:
>> >
>> > Klaus> Some debug prints in breakpoint.c are using
>> > Klaus> infrun_debug_printf ().  This does not seem to be the
>> > Klaus> right debug print macro.
>> >
>> > Klaus> Use breakpoint_debug_printf () exclusively in this file.
>> >
>> > I am not so sure about this.  It seems to me that some of these may
>> > mostly of interest when infrun debugging is enabled.  For example:
>> >
>> > Klaus> -      infrun_debug_printf ("incorrect thread or task, not
>> > Klaus> stopping");
>> > Klaus> +      breakpoint_debug_printf ("incorrect thread or task, not
>> > Klaus> stopping");
>> >
>> > This kind of thing is a decision impacting infrun decisions.
>> 
>> Hmm, good point.  I rescind my approval.
>> 
>> Kevin
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928