[PATCH v2 1/2] gdb/testsuite: rename get_maint_bp_addr and move it to gdb-utils.exp

[PATCH v2 1/2] gdb/testsuite: rename get_maint_bp_addr and move it to gdb-utils.exp

[Enze Li]

The get_maint_bp_addr procedure will be shared by other test suite, so
move it to gdb-utils.exp.

Following Andrew's suggestion, I renamed get_maint_bp_addr to
gdb_get_bp_addr, since it would have handled normal breakpoints in
addition to the internal ones.  Note that there is still room for
improvement in this procedure, which I indicated in comments nearby.
---
 gdb/testsuite/gdb.base/clear_non_user_bp.exp | 25 +----------------
 gdb/testsuite/lib/gdb-utils.exp              | 28 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 24 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..da694351c87 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:
@@ -77,7 +54,7 @@ if ![runto_main] then {
 }
 
 set bp_num [get_first_maint_bp_num]
-set bp_addr [get_maint_bp_addr $bp_num]
+set bp_addr [gdb_get_bp_addr $bp_num]
 
 gdb_test "maint info break $bp_num" \
     "$bp_num.*$bp_addr.*" \
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index ffdfb75557c..21953611ec2 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -72,3 +72,31 @@ proc style {str style} {
     }
     return "\033\\\[${style}m${str}\033\\\[m"
 }
+
+# gdb_get_bp_addr num
+#
+# Purpose:
+#    Get address of a particular breakpoint.
+#
+# Parameter:
+#    The parameter "num" indicates the number of the breakpoint to get.
+#    Note that *currently* this parameter must be an integer value.
+#    E.g., -1 means that we're gonna get the first internal breakpoint;
+#    2 means to get the second user-defined breakpoint.
+#
+# Return:
+#    First address for a particular breakpoint.
+#
+# TODO:
+#    It would be nice if this procedure could accept floating point value.
+#    E.g., 'gdb_get_bp_addr 1.2' means to get the address of the second
+#    location of breakpoint #1.
+#
+proc gdb_get_bp_addr { num } {
+    gdb_test_multiple "maint info break $num" "find address of specified bp $num" {
+	-re -wrap ".*(0x\[0-9a-f\]+).*" {
+	    return $expect_out(1,string)
+	}
+    }
+    return ""
+}
-- 
2.36.1

[PATCH v2 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp

[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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 58b1af3a0da..583408e10af 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -309,6 +309,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_python_valueof bp1.number "*DEFAULT*"]
+    set bp_addr [gdb_get_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

Re: [PATCH v2 1/2] gdb/testsuite: rename get_maint_bp_addr and move it to gdb-utils.exp

[Tom Tromey]

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

Enze> The get_maint_bp_addr procedure will be shared by other test suite, so
Enze> move it to gdb-utils.exp.

Enze> Following Andrew's suggestion, I renamed get_maint_bp_addr to
Enze> gdb_get_bp_addr, since it would have handled normal breakpoints in
Enze> addition to the internal ones.  Note that there is still room for
Enze> improvement in this procedure, which I indicated in comments nearby.

This looks good to me, thanks.

Tom

Re: [PATCH v2 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp

[Tom Tromey]

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

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

Enze> +    gdb_test "python gdb.execute(\'clear *$bp_addr\')" \

It seems weird to do this via gdb.execute rather than directly, but also
harmless I suppose.  So I think this is ok either way.

Tom

Re: [PATCH v2 1/2] gdb/testsuite: rename get_maint_bp_addr and move it to gdb-utils.exp

[Enze Li]

On Fri, 2022-07-15 at 10:25 -0600, Tom Tromey wrote:
> > > > > > "Enze" == Enze Li via Gdb-patches
> > > > > > <gdb-patches@sourceware.org> writes:
> 
> Enze> The get_maint_bp_addr procedure will be shared by other test
> suite, so
> Enze> move it to gdb-utils.exp.
> 
> Enze> Following Andrew's suggestion, I renamed get_maint_bp_addr to
> Enze> gdb_get_bp_addr, since it would have handled normal breakpoints
> in
> Enze> addition to the internal ones.  Note that there is still room
> for
> Enze> improvement in this procedure, which I indicated in comments
> nearby.
> 
> This looks good to me, thanks.
> 
> Tom

Hi Tom,

Thanks for your review.  I'm checking these in now.

Best Regards,
Enze