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

[Andrew Burgess <aburgess@redhat.com>]

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