public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp
@ 2022-06-19 12:32 Enze Li
  2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li
  2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess
  0 siblings, 2 replies; 7+ messages in thread
From: Enze Li @ 2022-06-19 12:32 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, enze.li

The get_maint_bp_addr procedure will be shared by other test suite, so
move it to gdb-utils.exp.
---
 gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 --------------------
 gdb/testsuite/lib/gdb-utils.exp              | 23 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
index 26d7a31fa47..399a3a0f0dc 100644
--- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
+++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
@@ -16,29 +16,6 @@
 # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
 # breakpoints with clear command.
 
-# get_maint_bp_addr num
-#
-# Purpose:
-#    Get address of the specified internal breakpoint when using command
-#    "maint info breakpoints $num".
-#
-# Parameter:
-#    The parameter "num" indicates the number of the internal breakpoint
-#    to get.  Note that this parameter must be a negative number.
-#    E.g., -1 means that we're gonna get the first internal breakpoint.
-#
-# Return:
-#    Internal breakpoint address.
-#
-proc get_maint_bp_addr { num } {
-    gdb_test_multiple "maint info break $num" "find address of internal bp $num" {
-	-re -wrap ".*(0x\[0-9a-f\]+).*" {
-	    return $expect_out(1,string)
-	}
-    }
-    return ""
-}
-
 # get_first_maint_bp_num
 #
 # Purpose:
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index ffdfb75557c..bf2863722ce 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -72,3 +72,26 @@ proc style {str style} {
     }
     return "\033\\\[${style}m${str}\033\\\[m"
 }
+
+# get_maint_bp_addr num
+#
+# Purpose:
+#    Get address of the specified internal breakpoint when using command
+#    "maint info breakpoints $num".
+#
+# Parameter:
+#    The parameter "num" indicates the number of the internal breakpoint
+#    to get.  Note that this parameter must be a negative number.
+#    E.g., -1 means that we're gonna get the first internal breakpoint.
+#
+# Return:
+#    Internal breakpoint address.
+#
+proc get_maint_bp_addr { num } {
+    gdb_test_multiple "maint info break $num" "find address of internal bp $num" {
+	-re -wrap ".*(0x\[0-9a-f\]+).*" {
+	    return $expect_out(1,string)
+	}
+    }
+    return ""
+}
-- 
2.36.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp
  2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li
@ 2022-06-19 12:33 ` Enze Li
  2022-06-21 16:21   ` Andrew Burgess
  2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess
  1 sibling, 1 reply; 7+ messages in thread
From: Enze Li @ 2022-06-19 12:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, enze.li

This patch adds a test case to try to clear an internal python
breakpoint using the clear command.

This was suggested by Pedro during a code review of the following
commit.

  commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6
  Date:   Sun Apr 17 15:09:46 2022 +0800

    gdb: fix using clear command to delete non-user breakpoints(PR cli/7161)

Tested on x86_64 openSUSE Tumbleweed.
---
 gdb/testsuite/gdb.python/py-breakpoint.exp | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 58b1af3a0da..984c20f9dc2 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } {
 	"Test watchpoint write"
 }
 
+# get_maint_bp_num bp_name
+#
+# Purpose:
+#    Get the number of a specified internal breakpoint.
+#
+# Parameter:
+#    The parameter "bp_name" indicates the name of the internal breakpoint
+#    to get.
+#
+# Return:
+#    Internal breakpoint number.  Empty string if fails.
+#
+proc get_maint_bp_num { bp_name } {
+    gdb_test_multiple "maint info break" "find $bp_name internal bp num" {
+	-re -wrap "0x.*(-\[0-9\]).*$bp_name.*" {
+	    return $expect_out(1,string)
+	}
+    }
+    return ""
+}
+
 proc_with_prefix test_bkpt_internal { } {
     global srcfile testfile hex decimal
 
@@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } {
     if ![runto_main] then {
 	return 0
     }
+    delete_breakpoints
+    gdb_py_test_silent_cmd  "python bp1 = gdb.Breakpoint (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, internal=True )" \
+	"Set internal breakpoint" 0
+
+    set bp_num [get_maint_bp_num "main"]
+    set bp_addr [get_maint_bp_addr $bp_num]
+
+    gdb_test "maint info break $bp_num" \
+	"$bp_num.*$bp_addr.*" \
+	"maint info breakpoint \$bp_num"
+
+    gdb_test "python gdb.execute(\'clear *$bp_addr\')" \
+	".*No breakpoint at \\*$bp_addr.*" \
+	"clear internal breakpoint"
+
+    # Check again, make sure that GDB really didn't delete the internal breakpoint.
+    gdb_test "maint info break $bp_num" \
+	"$bp_num.*$bp_addr.*" \
+	"maint info breakpoint \$bp_num after clear"
+
     delete_breakpoints
     gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, internal=True )" \
 	"Set watchpoint" 0
-- 
2.36.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp
  2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li
  2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li
@ 2022-06-21 16:12 ` Andrew Burgess
  2022-06-23 13:12   ` Enze Li
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2022-06-21 16:12 UTC (permalink / raw)
  To: Enze Li, gdb-patches; +Cc: pedro, enze.li

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

> The get_maint_bp_addr procedure will be shared by other test suite, so
> move it to gdb-utils.exp.
> ---
>  gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 --------------------
>  gdb/testsuite/lib/gdb-utils.exp              | 23 ++++++++++++++++++++
>  2 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> index 26d7a31fa47..399a3a0f0dc 100644
> --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> @@ -16,29 +16,6 @@
>  # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
>  # breakpoints with clear command.
>  
> -# get_maint_bp_addr num
> -#
> -# Purpose:
> -#    Get address of the specified internal breakpoint when using command
> -#    "maint info breakpoints $num".
> -#
> -# Parameter:
> -#    The parameter "num" indicates the number of the internal breakpoint
> -#    to get.  Note that this parameter must be a negative number.
> -#    E.g., -1 means that we're gonna get the first internal breakpoint.
> -#
> -# Return:
> -#    Internal breakpoint address.
> -#
> -proc get_maint_bp_addr { num } {
> -    gdb_test_multiple "maint info break $num" "find address of internal bp $num" {
> -	-re -wrap ".*(0x\[0-9a-f\]+).*" {
> -	    return $expect_out(1,string)
> -	}
> -    }
> -    return ""
> -}
> -
>  # get_first_maint_bp_num
>  #
>  # Purpose:
> diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
> index ffdfb75557c..bf2863722ce 100644
> --- a/gdb/testsuite/lib/gdb-utils.exp
> +++ b/gdb/testsuite/lib/gdb-utils.exp
> @@ -72,3 +72,26 @@ proc style {str style} {
>      }
>      return "\033\\\[${style}m${str}\033\\\[m"
>  }
> +
> +# get_maint_bp_addr num
> +#
> +# Purpose:
> +#    Get address of the specified internal breakpoint when using command
> +#    "maint info breakpoints $num".
> +#
> +# Parameter:
> +#    The parameter "num" indicates the number of the internal breakpoint
> +#    to get.  Note that this parameter must be a negative number.
> +#    E.g., -1 means that we're gonna get the first internal breakpoint.
> +#
> +# Return:
> +#    Internal breakpoint address.
> +#
> +proc get_maint_bp_addr { num } {
> +    gdb_test_multiple "maint info break $num" "find address of internal bp $num" {
> +	-re -wrap ".*(0x\[0-9a-f\]+).*" {
> +	    return $expect_out(1,string)
> +	}
> +    }
> +    return ""
> +}

Before the get_maint_bp_addr name gets spread around too much, I think
we should rename this.  I'd propose maybe gdb_get_bp_addr.  The comment
should be reworked to remove the mention of negative values, as I
believe this function will work just as well with positive values.

I think the comment should also be expanded to indicate that (currently)
this function will only take integer values for NUM, and will return the
first address for a particular breakpoint, e.g. we can't say
'get_maint_bp_addr 1.2' to get the address of the second location of
breakpoint #1.

I don't think that we need to make the actual implementation of
get_maint_bp_addr better right now, but if we give the function a good
name, and explain the limitations in the comments, then, if we ever need
to, we can improve the function later.

Thanks,
Andrew

> -- 
> 2.36.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp
  2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li
@ 2022-06-21 16:21   ` Andrew Burgess
  2022-06-23 13:29     ` Enze Li
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2022-06-21 16:21 UTC (permalink / raw)
  To: Enze Li, gdb-patches; +Cc: pedro, enze.li

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

> This patch adds a test case to try to clear an internal python
> breakpoint using the clear command.
>
> This was suggested by Pedro during a code review of the following
> commit.
>
>   commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6
>   Date:   Sun Apr 17 15:09:46 2022 +0800
>
>     gdb: fix using clear command to delete non-user breakpoints(PR cli/7161)
>
> Tested on x86_64 openSUSE Tumbleweed.
> ---
>  gdb/testsuite/gdb.python/py-breakpoint.exp | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 58b1af3a0da..984c20f9dc2 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } {
>  	"Test watchpoint write"
>  }
>  
> +# get_maint_bp_num bp_name
> +#
> +# Purpose:
> +#    Get the number of a specified internal breakpoint.
> +#
> +# Parameter:
> +#    The parameter "bp_name" indicates the name of the internal breakpoint
> +#    to get.
> +#
> +# Return:
> +#    Internal breakpoint number.  Empty string if fails.
> +#
> +proc get_maint_bp_num { bp_name } {
> +    gdb_test_multiple "maint info break" "find $bp_name internal bp num" {
> +	-re -wrap "0x.*(-\[0-9\]).*$bp_name.*" {
> +	    return $expect_out(1,string)
> +	}
> +    }
> +    return ""
> +}
> +
>  proc_with_prefix test_bkpt_internal { } {
>      global srcfile testfile hex decimal
>  
> @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } {
>      if ![runto_main] then {
>  	return 0
>      }
> +    delete_breakpoints
> +    gdb_py_test_silent_cmd  "python bp1 = gdb.Breakpoint (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, internal=True )" \
> +	"Set internal breakpoint" 0
> +
> +    set bp_num [get_maint_bp_num "main"]

Instead of adding get_maint_bp_num, would this work?

  set bp_num [get_python_valueof bp1.number "*DEFAULT*"]

> +    set bp_addr [get_maint_bp_addr $bp_num]
> +
> +    gdb_test "maint info break $bp_num" \
> +	"$bp_num.*$bp_addr.*" \
> +	"maint info breakpoint \$bp_num"
> +
> +    gdb_test "python gdb.execute(\'clear *$bp_addr\')" \
> +	".*No breakpoint at \\*$bp_addr.*" \
> +	"clear internal breakpoint"

You might also want to try calling clear without the gdb.execute, just
in case the Python layer makes a difference.

Thanks,
Andrew

> +
> +    # Check again, make sure that GDB really didn't delete the internal breakpoint.
> +    gdb_test "maint info break $bp_num" \
> +	"$bp_num.*$bp_addr.*" \
> +	"maint info breakpoint \$bp_num after clear"
> +
>      delete_breakpoints
>      gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, internal=True )" \
>  	"Set watchpoint" 0
> -- 
> 2.36.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp
  2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess
@ 2022-06-23 13:12   ` Enze Li
  0 siblings, 0 replies; 7+ messages in thread
From: Enze Li @ 2022-06-23 13:12 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches; +Cc: pedro, enze.li

Hi Andrew,

Thanks for your review.

Naming is a hard thing for me.  gdb_get_bp_addr seems to have more
features than before, and starting with 'gdb' looks good. 🙂

V2 of this patch is on the way.

Best Regards,
Enze

On Tue, 2022-06-21 at 17:12 +0100, Andrew Burgess wrote:
> Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> > The get_maint_bp_addr procedure will be shared by other test suite,
> > so
> > move it to gdb-utils.exp.
> > ---
> >  gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 ----------------
> > ----
> >  gdb/testsuite/lib/gdb-utils.exp              | 23
> > ++++++++++++++++++++
> >  2 files changed, 23 insertions(+), 23 deletions(-)
> > 
> > diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > index 26d7a31fa47..399a3a0f0dc 100644
> > --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > @@ -16,29 +16,6 @@
> >  # Regression test for PR gdb/7161.  Test that GDB cannot delete
> > non-user
> >  # breakpoints with clear command.
> >  
> > -# get_maint_bp_addr num
> > -#
> > -# Purpose:
> > -#    Get address of the specified internal breakpoint when using
> > command
> > -#    "maint info breakpoints $num".
> > -#
> > -# Parameter:
> > -#    The parameter "num" indicates the number of the internal
> > breakpoint
> > -#    to get.  Note that this parameter must be a negative number.
> > -#    E.g., -1 means that we're gonna get the first internal
> > breakpoint.
> > -#
> > -# Return:
> > -#    Internal breakpoint address.
> > -#
> > -proc get_maint_bp_addr { num } {
> > -    gdb_test_multiple "maint info break $num" "find address of
> > internal bp $num" {
> > -       -re -wrap ".*(0x\[0-9a-f\]+).*" {
> > -           return $expect_out(1,string)
> > -       }
> > -    }
> > -    return ""
> > -}
> > -
> >  # get_first_maint_bp_num
> >  #
> >  # Purpose:
> > diff --git a/gdb/testsuite/lib/gdb-utils.exp
> > b/gdb/testsuite/lib/gdb-utils.exp
> > index ffdfb75557c..bf2863722ce 100644
> > --- a/gdb/testsuite/lib/gdb-utils.exp
> > +++ b/gdb/testsuite/lib/gdb-utils.exp
> > @@ -72,3 +72,26 @@ proc style {str style} {
> >      }
> >      return "\033\\\[${style}m${str}\033\\\[m"
> >  }
> > +
> > +# get_maint_bp_addr num
> > +#
> > +# Purpose:
> > +#    Get address of the specified internal breakpoint when using
> > command
> > +#    "maint info breakpoints $num".
> > +#
> > +# Parameter:
> > +#    The parameter "num" indicates the number of the internal
> > breakpoint
> > +#    to get.  Note that this parameter must be a negative number.
> > +#    E.g., -1 means that we're gonna get the first internal
> > breakpoint.
> > +#
> > +# Return:
> > +#    Internal breakpoint address.
> > +#
> > +proc get_maint_bp_addr { num } {
> > +    gdb_test_multiple "maint info break $num" "find address of
> > internal bp $num" {
> > +       -re -wrap ".*(0x\[0-9a-f\]+).*" {
> > +           return $expect_out(1,string)
> > +       }
> > +    }
> > +    return ""
> > +}
> 
> Before the get_maint_bp_addr name gets spread around too much, I
> think
> we should rename this.  I'd propose maybe gdb_get_bp_addr.  The
> comment
> should be reworked to remove the mention of negative values, as I
> believe this function will work just as well with positive values.
> 
> I think the comment should also be expanded to indicate that
> (currently)
> this function will only take integer values for NUM, and will return
> the
> first address for a particular breakpoint, e.g. we can't say
> 'get_maint_bp_addr 1.2' to get the address of the second location of
> breakpoint #1.
> 
> I don't think that we need to make the actual implementation of
> get_maint_bp_addr better right now, but if we give the function a
> good
> name, and explain the limitations in the comments, then, if we ever
> need
> to, we can improve the function later.
> 
> Thanks,
> Andrew
> 
> > -- 
> > 2.36.1
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp
  2022-06-21 16:21   ` Andrew Burgess
@ 2022-06-23 13:29     ` Enze Li
  2022-06-24 12:00       ` Andrew Burgess
  0 siblings, 1 reply; 7+ messages in thread
From: Enze Li @ 2022-06-23 13:29 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches; +Cc: pedro, enze.li

Hi Andrew,

Thanks for your review.  Please see below.

On Tue, 2022-06-21 at 17:21 +0100, Andrew Burgess wrote:
> Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> > This patch adds a test case to try to clear an internal python
> > breakpoint using the clear command.
> > 
> > This was suggested by Pedro during a code review of the following
> > commit.
> > 
> >   commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6
> >   Date:   Sun Apr 17 15:09:46 2022 +0800
> > 
> >     gdb: fix using clear command to delete non-user breakpoints(PR
> > cli/7161)
> > 
> > Tested on x86_64 openSUSE Tumbleweed.
> > ---
> >  gdb/testsuite/gdb.python/py-breakpoint.exp | 41
> > ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp
> > b/gdb/testsuite/gdb.python/py-breakpoint.exp
> > index 58b1af3a0da..984c20f9dc2 100644
> > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> > @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } {
> >         "Test watchpoint write"
> >  }
> >  
> > +# get_maint_bp_num bp_name
> > +#
> > +# Purpose:
> > +#    Get the number of a specified internal breakpoint.
> > +#
> > +# Parameter:
> > +#    The parameter "bp_name" indicates the name of the internal
> > breakpoint
> > +#    to get.
> > +#
> > +# Return:
> > +#    Internal breakpoint number.  Empty string if fails.
> > +#
> > +proc get_maint_bp_num { bp_name } {
> > +    gdb_test_multiple "maint info break" "find $bp_name internal
> > bp num" {
> > +       -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" {
> > +           return $expect_out(1,string)
> > +       }
> > +    }
> > +    return ""
> > +}
> > +
> >  proc_with_prefix test_bkpt_internal { } {
> >      global srcfile testfile hex decimal
> >  
> > @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } {
> >      if ![runto_main] then {
> >         return 0
> >      }
> > +    delete_breakpoints
> > +    gdb_py_test_silent_cmd  "python bp1 = gdb.Breakpoint
> > (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE,
> > internal=True )" \
> > +       "Set internal breakpoint" 0
> > +
> > +    set bp_num [get_maint_bp_num "main"]
> 
> Instead of adding get_maint_bp_num, would this work?
> 
>   set bp_num [get_python_valueof bp1.number "*DEFAULT*"]

It works!  There's no need to add a new procedure, and I'll drop it.

> 
> > +    set bp_addr [get_maint_bp_addr $bp_num]
> > +
> > +    gdb_test "maint info break $bp_num" \
> > +       "$bp_num.*$bp_addr.*" \
> > +       "maint info breakpoint \$bp_num"
> > +
> > +    gdb_test "python gdb.execute(\'clear *$bp_addr\')" \
> > +       ".*No breakpoint at \\*$bp_addr.*" \
> > +       "clear internal breakpoint"
> 
> You might also want to try calling clear without the gdb.execute,
> just
> in case the Python layer makes a difference.
> 
> Thanks,
> Andrew

In fact, there already exists a test case in the trunk which calling
clear command without python extention.  (See clear_non_user_bp.exp,
line 86).  However, if it is still needed here, pls let me know.

Best Regards,
Enze

> 
> > +
> > +    # Check again, make sure that GDB really didn't delete the
> > internal breakpoint.
> > +    gdb_test "maint info break $bp_num" \
> > +       "$bp_num.*$bp_addr.*" \
> > +       "maint info breakpoint \$bp_num after clear"
> > +
> >      delete_breakpoints
> >      gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint
> > (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE,
> > internal=True )" \
> >         "Set watchpoint" 0
> > -- 
> > 2.36.1
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp
  2022-06-23 13:29     ` Enze Li
@ 2022-06-24 12:00       ` Andrew Burgess
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2022-06-24 12:00 UTC (permalink / raw)
  To: Enze Li, gdb-patches; +Cc: pedro, enze.li

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

> Hi Andrew,
>
> Thanks for your review.  Please see below.
>
> On Tue, 2022-06-21 at 17:21 +0100, Andrew Burgess wrote:
>> Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
>> 
>> > This patch adds a test case to try to clear an internal python
>> > breakpoint using the clear command.
>> > 
>> > This was suggested by Pedro during a code review of the following
>> > commit.
>> > 
>> >   commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6
>> >   Date:   Sun Apr 17 15:09:46 2022 +0800
>> > 
>> >     gdb: fix using clear command to delete non-user breakpoints(PR
>> > cli/7161)
>> > 
>> > Tested on x86_64 openSUSE Tumbleweed.
>> > ---
>> >  gdb/testsuite/gdb.python/py-breakpoint.exp | 41
>> > ++++++++++++++++++++++
>> >  1 file changed, 41 insertions(+)
>> > 
>> > diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp
>> > b/gdb/testsuite/gdb.python/py-breakpoint.exp
>> > index 58b1af3a0da..984c20f9dc2 100644
>> > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
>> > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
>> > @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } {
>> >         "Test watchpoint write"
>> >  }
>> >  
>> > +# get_maint_bp_num bp_name
>> > +#
>> > +# Purpose:
>> > +#    Get the number of a specified internal breakpoint.
>> > +#
>> > +# Parameter:
>> > +#    The parameter "bp_name" indicates the name of the internal
>> > breakpoint
>> > +#    to get.
>> > +#
>> > +# Return:
>> > +#    Internal breakpoint number.  Empty string if fails.
>> > +#
>> > +proc get_maint_bp_num { bp_name } {
>> > +    gdb_test_multiple "maint info break" "find $bp_name internal
>> > bp num" {
>> > +       -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" {
>> > +           return $expect_out(1,string)
>> > +       }
>> > +    }
>> > +    return ""
>> > +}
>> > +
>> >  proc_with_prefix test_bkpt_internal { } {
>> >      global srcfile testfile hex decimal
>> >  
>> > @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } {
>> >      if ![runto_main] then {
>> >         return 0
>> >      }
>> > +    delete_breakpoints
>> > +    gdb_py_test_silent_cmd  "python bp1 = gdb.Breakpoint
>> > (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE,
>> > internal=True )" \
>> > +       "Set internal breakpoint" 0
>> > +
>> > +    set bp_num [get_maint_bp_num "main"]
>> 
>> Instead of adding get_maint_bp_num, would this work?
>> 
>>   set bp_num [get_python_valueof bp1.number "*DEFAULT*"]
>
> It works!  There's no need to add a new procedure, and I'll drop it.

Excellent!

>
>> 
>> > +    set bp_addr [get_maint_bp_addr $bp_num]
>> > +
>> > +    gdb_test "maint info break $bp_num" \
>> > +       "$bp_num.*$bp_addr.*" \
>> > +       "maint info breakpoint \$bp_num"
>> > +
>> > +    gdb_test "python gdb.execute(\'clear *$bp_addr\')" \
>> > +       ".*No breakpoint at \\*$bp_addr.*" \
>> > +       "clear internal breakpoint"
>> 
>> You might also want to try calling clear without the gdb.execute,
>> just
>> in case the Python layer makes a difference.
>> 
>> Thanks,
>> Andrew
>
> In fact, there already exists a test case in the trunk which calling
> clear command without python extention.  (See clear_non_user_bp.exp,
> line 86).  However, if it is still needed here, pls let me know.

I'll leave it up to you then.  I think if you did add it, the test would
be slightly different than anything else.  But probably not in any
significant way, so I don't really mind.

Thanks,
Andrew



>
> Best Regards,
> Enze
>
>> 
>> > +
>> > +    # Check again, make sure that GDB really didn't delete the
>> > internal breakpoint.
>> > +    gdb_test "maint info break $bp_num" \
>> > +       "$bp_num.*$bp_addr.*" \
>> > +       "maint info breakpoint \$bp_num after clear"
>> > +
>> >      delete_breakpoints
>> >      gdb_py_test_silent_cmd  "python wp1 = gdb.Breakpoint
>> > (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE,
>> > internal=True )" \
>> >         "Set watchpoint" 0
>> > -- 
>> > 2.36.1
>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-24 12:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li
2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li
2022-06-21 16:21   ` Andrew Burgess
2022-06-23 13:29     ` Enze Li
2022-06-24 12:00       ` Andrew Burgess
2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess
2022-06-23 13:12   ` Enze Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).