[PATCH] gdb: prevent the use of the clear command to remove the internal breakpoint (PR cli/7161)

[PATCH] gdb: prevent the use of the clear command to remove the internal breakpoint (PR cli/7161)

[Enze Li]

This patch fixes the PR cli/7161 - "clear command removes internal
breakpoints".

In this patch, a new function "internal_breakpoint" is added to
determine whether the breakpoint is internal or not.  If the
breakpoint is internal when using the clear command, no action will
be taken, thus preventing the use of the clear command to remove
the interal breakpoint.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7161
---
 gdb/breakpoint.c | 10 +++++++++-
 gdb/breakpoint.h |  3 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a3cfeea6989..08628d885e7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6477,6 +6477,13 @@ pending_breakpoint_p (struct breakpoint *b)
   return b->loc == NULL;
 }
 
+/* See breakpoint.h.  */
+bool
+internal_breakpoint (struct breakpoint *b)
+{
+  return b->ops == &internal_breakpoint_ops;
+}
+
 /* Print information on breakpoints (including watchpoints and tracepoints).
 
    If non-NULL, BP_NUM_LIST is a list of numbers and number ranges as
@@ -10979,7 +10986,8 @@ clear_command (const char *arg, int from_tty)
 	{
 	  int match = 0;
 	  /* Are we going to delete b?  */
-	  if (b->type != bp_none && !is_watchpoint (b))
+	  if (b->type != bp_none && !is_watchpoint (b)
+	      && !internal_breakpoint (b))
 	    {
 	      for (bp_location *loc : b->locations ())
 		{
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ba28219c236..c6ddfb0b87a 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1752,6 +1752,9 @@ extern int user_breakpoint_p (struct breakpoint *);
 /* Return true if this breakpoint is pending, false if not.  */
 extern int pending_breakpoint_p (struct breakpoint *);
 
+/* Return true if this breakpoint is internal, false if not.  */
+extern bool internal_breakpoint (struct breakpoint *b);
+
 /* Attempt to determine architecture of location identified by SAL.  */
 extern struct gdbarch *get_sal_arch (struct symtab_and_line sal);
 
-- 
2.35.1

Re: [PATCH] gdb: prevent the use of the clear command to remove the internal breakpoint (PR cli/7161)

[Pedro Alves]

Hi!

On 2022-03-14 12:24, Enze Li via Gdb-patches wrote:
> This patch fixes the PR cli/7161 - "clear command removes internal
> breakpoints".
> 
> In this patch, a new function "internal_breakpoint" is added to
> determine whether the breakpoint is internal or not.  If the
> breakpoint is internal when using the clear command, no action will
> be taken, thus preventing the use of the clear command to remove
> the interal breakpoint.
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7161
> ---
>  gdb/breakpoint.c | 10 +++++++++-
>  gdb/breakpoint.h |  3 +++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index a3cfeea6989..08628d885e7 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6477,6 +6477,13 @@ pending_breakpoint_p (struct breakpoint *b)
>    return b->loc == NULL;
>  }
>  
> +/* See breakpoint.h.  */
> +bool
> +internal_breakpoint (struct breakpoint *b)
> +{
> +  return b->ops == &internal_breakpoint_ops;
> +}
> +

The clear command shouldn't delete momentary breakpoints either (those have bp num == 0),
nor internal breakpoints created via Python's gdb.Breakpoint, I think.  Neither of these
use internal_breakpoint_ops. 

I think we should check the existing user_breakpoint_p instead.

This should really have a testcase in the gdb testsuite.

Pedro Alves

Re: [PATCH] gdb: prevent the use of the clear command to remove the internal breakpoint (PR cli/7161)

[Enze Li]

Hi Pedro, Tom

Thanks to both of you for the code review.  I have sent the v2 of the
patch to the list, adding a testcase dedicated to testing PR7161.

Thanks,
Enze

On Mon, 2022-03-14 at 13:53 +0000, Pedro Alves wrote:
> Hi!
> 
> On 2022-03-14 12:24, Enze Li via Gdb-patches wrote:
> > This patch fixes the PR cli/7161 - "clear command removes internal
> > breakpoints".
> > 
> > In this patch, a new function "internal_breakpoint" is added to
> > determine whether the breakpoint is internal or not.  If the
> > breakpoint is internal when using the clear command, no action will
> > be taken, thus preventing the use of the clear command to remove
> > the interal breakpoint.
> > 
> > Tested on x86_64-linux.
> > 
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7161
> > ---
> >  gdb/breakpoint.c | 10 +++++++++-
> >  gdb/breakpoint.h |  3 +++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> > index a3cfeea6989..08628d885e7 100644
> > --- a/gdb/breakpoint.c
> > +++ b/gdb/breakpoint.c
> > @@ -6477,6 +6477,13 @@ pending_breakpoint_p (struct breakpoint *b)
> >    return b->loc == NULL;
> >  }
> >  
> > +/* See breakpoint.h.  */
> > +bool
> > +internal_breakpoint (struct breakpoint *b)
> > +{
> > +  return b->ops == &internal_breakpoint_ops;
> > +}
> > +
> 
> The clear command shouldn't delete momentary breakpoints either
> (those have bp num == 0),
> nor internal breakpoints created via Python's gdb.Breakpoint, I
> think.  Neither of these
> use internal_breakpoint_ops. 
> 
> I think we should check the existing user_breakpoint_p instead.
> 
> This should really have a testcase in the gdb testsuite.
> 
> Pedro Alves

Re: [PATCH] gdb: prevent the use of the clear command to remove the internal breakpoint (PR cli/7161)

[Tom Tromey]

>>>>> Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:

> +/* See breakpoint.h.  */
> +bool
> +internal_breakpoint (struct breakpoint *b)
> +{
> +  return b->ops == &internal_breakpoint_ops;
> +}

I think the correct test is probably if the breakpoint number is <=0.
There are, I think, "internal" breakpoints that don't use
internal_breakpoint_ops.

Tom