[PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Enze Li]

Tom and Simon feedback that there is a test failing in this 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)

Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
fixed the nit in this patch.  The root of the problem is not correctly
matching the presentation of internal breakpoints.

In addition, as Pedro pointed out, the original testcase is not portable
in some methods, so this patch fixes this issue and some other
improvements.

Tested on x86_64 ubuntu 20.04.4 and openSUSE Tumbleweed(VERSION_ID="20220415").
---
 gdb/testsuite/gdb.base/clear_non_user_bp.exp | 33 ++++++++++++++------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
index d7bb8ab7e9a..4dc6f3bcbfd 100644
--- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
+++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
@@ -16,7 +16,7 @@
 # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
 # breakpoints with clear command.
 
-proc get_maint_info_bp { var } {
+proc get_maint_bp_addr { var } {
     global expect_out
     global gdb_prompt
 
@@ -24,9 +24,17 @@ proc get_maint_info_bp { var } {
 	-re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
 	    return $expect_out(1,string)
 	}
-	timeout {
-	    perror "couldn't find address of $var"
-	    return ""
+    }
+    return ""
+}
+
+proc get_maint_bp_num { } {
+    global expect_out
+    global gdb_prompt
+
+    gdb_test_multiple "maint info break" "find first internal bp num" {
+	-re "(-\[0-9\]).*$gdb_prompt $" {
+	    return $expect_out(1,string)
 	}
     }
     return ""
@@ -52,13 +60,20 @@ gdb_test "break main.c:21" \
     ".*Breakpoint.* at .*" \
     "set breakpoint"
 
-set bp_addr [get_maint_info_bp "-1"]
+set bp_num [get_maint_bp_num]
+set bp_addr [get_maint_bp_addr $bp_num]
 
-gdb_test "maint info break -1" \
-    "-1.*shlib events.*keep y.*$bp_addr.*" \
-    "maint info breakpoint -1 error"
+gdb_test "maint info break $bp_num" \
+    "$bp_num.*$bp_addr.*" \
+    "maint info breakpoint $bp_num"
 
 gdb_test "clear *$bp_addr" \
     "No breakpoint at \\*$bp_addr." \
-    "clear internal breakpoint error"
+    "clear internal breakpoint"
+
+# Check again, make sure that GDB really didn't delete the internal breakpoint.
+set bp_num2 [get_maint_bp_num]
+gdb_test "maint info break $bp_num2" \
+    "$bp_num2.*$bp_addr.*" \
+    "maint info breakpoint $bp_num after clear"
 
-- 
2.35.3

Re: [PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Simon Marchi]

On 2022-04-19 09:48, Enze Li via Gdb-patches wrote:
> Tom and Simon feedback that there is a test failing in this 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)
>
> Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
> fixed the nit in this patch.  The root of the problem is not correctly
> matching the presentation of internal breakpoints.
>
> In addition, as Pedro pointed out, the original testcase is not portable
> in some methods, so this patch fixes this issue and some other
> improvements.
>
> Tested on x86_64 ubuntu 20.04.4 and openSUSE Tumbleweed(VERSION_ID="20220415").
> ---
>  gdb/testsuite/gdb.base/clear_non_user_bp.exp | 33 ++++++++++++++------
>  1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> index d7bb8ab7e9a..4dc6f3bcbfd 100644
> --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> @@ -16,7 +16,7 @@
>  # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
>  # breakpoints with clear command.
>
> -proc get_maint_info_bp { var } {
> +proc get_maint_bp_addr { var } {

Can you please document these procs?  Describe the arguments and the
return values.

>      global expect_out
>      global gdb_prompt
>
> @@ -24,9 +24,17 @@ proc get_maint_info_bp { var } {
>  	-re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
>  	    return $expect_out(1,string)
>  	}
> -	timeout {
> -	    perror "couldn't find address of $var"
> -	    return ""
> +    }
> +    return ""
> +}
> +
> +proc get_maint_bp_num { } {
> +    global expect_out

I don't know what the logic is, but you don't need "global expect_out".
Just using $expect_out works.

> +    global gdb_prompt

I suggest referring to global variables using the $::gdb_prompt syntax,
rather than using the global keyword.

> +
> +    gdb_test_multiple "maint info break" "find first internal bp num" {
> +	-re "(-\[0-9\]).*$gdb_prompt $" {

You can use -wrap instead of matching .* and the prompt:

  -re -wrap "(-\[0-9\])" {
  }

(this is untested)

> +	    return $expect_out(1,string)
>  	}
>      }
>      return ""
> @@ -52,13 +60,20 @@ gdb_test "break main.c:21" \
>      ".*Breakpoint.* at .*" \
>      "set breakpoint"
>
> -set bp_addr [get_maint_info_bp "-1"]
> +set bp_num [get_maint_bp_num]
> +set bp_addr [get_maint_bp_addr $bp_num]
>
> -gdb_test "maint info break -1" \
> -    "-1.*shlib events.*keep y.*$bp_addr.*" \
> -    "maint info breakpoint -1 error"
> +gdb_test "maint info break $bp_num" \
> +    "$bp_num.*$bp_addr.*" \
> +    "maint info breakpoint $bp_num"

If the breakpoint number can vary from system to system, let's avoid
putting it in the test name.  It's better to reduce the differences in
test names between runs, when possible.

>  gdb_test "clear *$bp_addr" \
>      "No breakpoint at \\*$bp_addr." \
> -    "clear internal breakpoint error"
> +    "clear internal breakpoint"
> +
> +# Check again, make sure that GDB really didn't delete the internal breakpoint.
> +set bp_num2 [get_maint_bp_num]
> +gdb_test "maint info break $bp_num2" \
> +    "$bp_num2.*$bp_addr.*" \
> +    "maint info breakpoint $bp_num after clear"

Likewise here.

Simon

Re: [PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Pedro Alves]

On 2022-04-19 14:48, Enze Li wrote:
>  gdb_test "clear *$bp_addr" \
>      "No breakpoint at \\*$bp_addr." \
> -    "clear internal breakpoint error"
> +    "clear internal breakpoint"
> +
> +# Check again, make sure that GDB really didn't delete the internal breakpoint.
> +set bp_num2 [get_maint_bp_num]

If the clear managed to erroneously delete the internal breakpoint, then this get_maint_bp_num will find another,
different, internal breakpoint.  You should instead use the same $bp_num as before, in the gdb_test below.

> +gdb_test "maint info break $bp_num2" \
> +    "$bp_num2.*$bp_addr.*" \
> +    "maint info breakpoint $bp_num after clear"

Re: [PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Enze Li]

Hi Simon,

Thanks for the review.  Please See below.

On Tue, 2022-04-19 at 10:47 -0400, Simon Marchi wrote:
> On 2022-04-19 09:48, Enze Li via Gdb-patches wrote:
<snip>
> > diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > index d7bb8ab7e9a..4dc6f3bcbfd 100644
> > --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > @@ -16,7 +16,7 @@
> >  # Regression test for PR gdb/7161.  Test that GDB cannot delete
> > non-user
> >  # breakpoints with clear command.
> > 
> > -proc get_maint_info_bp { var } {
> > +proc get_maint_bp_addr { var } {
> 
> Can you please document these procs?  Describe the arguments and the
> return values.
> 
> >      global expect_out
> >      global gdb_prompt
> > 
> > @@ -24,9 +24,17 @@ proc get_maint_info_bp { var } {
> >         -re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
> >             return $expect_out(1,string)
> >         }
> > -       timeout {
> > -           perror "couldn't find address of $var"
> > -           return ""
> > +    }
> > +    return ""
> > +}
> > +
> > +proc get_maint_bp_num { } {
> > +    global expect_out
> 
> I don't know what the logic is, but you don't need "global
> expect_out".
> Just using $expect_out works.
> 
> > +    global gdb_prompt
> 
> I suggest referring to global variables using the $::gdb_prompt
> syntax,
> rather than using the global keyword.
> 
> > +
> > +    gdb_test_multiple "maint info break" "find first internal bp
> > num" {
> > +       -re "(-\[0-9\]).*$gdb_prompt $" {
> 
> You can use -wrap instead of matching .* and the prompt:
> 
>   -re -wrap "(-\[0-9\])" {
>   }
> 
> (this is untested)

It seems that the "-wrap" doesn't wrap ".*".  It works fine with
"-re -wrap "(-\[0-9\]).*".

> 
> > +           return $expect_out(1,string)
> >         }
> >      }
> >      return ""
> > @@ -52,13 +60,20 @@ gdb_test "break main.c:21" \
> >      ".*Breakpoint.* at .*" \
> >      "set breakpoint"
> > 
> > -set bp_addr [get_maint_info_bp "-1"]
> > +set bp_num [get_maint_bp_num]
> > +set bp_addr [get_maint_bp_addr $bp_num]
> > 
> > -gdb_test "maint info break -1" \
> > -    "-1.*shlib events.*keep y.*$bp_addr.*" \
> > -    "maint info breakpoint -1 error"
> > +gdb_test "maint info break $bp_num" \
> > +    "$bp_num.*$bp_addr.*" \
> > +    "maint info breakpoint $bp_num"
> 
> If the breakpoint number can vary from system to system, let's avoid
> putting it in the test name.  It's better to reduce the differences
> in
> test names between runs, when possible.

I didn't get it.  Does it mean that we need to use fixed internal
breakpoint number instead of dynamically getting one?  Like this?
    gdb_test "maint info break -1" \
        "-1.*$bp_addr.*" \
        "maint info breakpoint -1"

Thanks,
Enze

Re: [PATCH] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Simon Marchi]

On 2022-04-20 08:34, Enze Li via Gdb-patches wrote:
> Hi Simon,
> 
> Thanks for the review.  Please See below.
> 
> On Tue, 2022-04-19 at 10:47 -0400, Simon Marchi wrote:
>> On 2022-04-19 09:48, Enze Li via Gdb-patches wrote:
> <snip>
>>> diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
>>> b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
>>> index d7bb8ab7e9a..4dc6f3bcbfd 100644
>>> --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
>>> +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
>>> @@ -16,7 +16,7 @@
>>>  # Regression test for PR gdb/7161.  Test that GDB cannot delete
>>> non-user
>>>  # breakpoints with clear command.
>>>
>>> -proc get_maint_info_bp { var } {
>>> +proc get_maint_bp_addr { var } {
>>
>> Can you please document these procs?  Describe the arguments and the
>> return values.
>>
>>>      global expect_out
>>>      global gdb_prompt
>>>
>>> @@ -24,9 +24,17 @@ proc get_maint_info_bp { var } {
>>>         -re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
>>>             return $expect_out(1,string)
>>>         }
>>> -       timeout {
>>> -           perror "couldn't find address of $var"
>>> -           return ""
>>> +    }
>>> +    return ""
>>> +}
>>> +
>>> +proc get_maint_bp_num { } {
>>> +    global expect_out
>>
>> I don't know what the logic is, but you don't need "global
>> expect_out".
>> Just using $expect_out works.
>>
>>> +    global gdb_prompt
>>
>> I suggest referring to global variables using the $::gdb_prompt
>> syntax,
>> rather than using the global keyword.
>>
>>> +
>>> +    gdb_test_multiple "maint info break" "find first internal bp
>>> num" {
>>> +       -re "(-\[0-9\]).*$gdb_prompt $" {
>>
>> You can use -wrap instead of matching .* and the prompt:
>>
>>   -re -wrap "(-\[0-9\])" {
>>   }
>>
>> (this is untested)
> 
> It seems that the "-wrap" doesn't wrap ".*".  It works fine with
> "-re -wrap "(-\[0-9\]).*".

You're right, sorry.  That looks good.

> 
>>
>>> +           return $expect_out(1,string)
>>>         }
>>>      }
>>>      return ""
>>> @@ -52,13 +60,20 @@ gdb_test "break main.c:21" \
>>>      ".*Breakpoint.* at .*" \
>>>      "set breakpoint"
>>>
>>> -set bp_addr [get_maint_info_bp "-1"]
>>> +set bp_num [get_maint_bp_num]
>>> +set bp_addr [get_maint_bp_addr $bp_num]
>>>
>>> -gdb_test "maint info break -1" \
>>> -    "-1.*shlib events.*keep y.*$bp_addr.*" \
>>> -    "maint info breakpoint -1 error"
>>> +gdb_test "maint info break $bp_num" \
>>> +    "$bp_num.*$bp_addr.*" \
>>> +    "maint info breakpoint $bp_num"
>>
>> If the breakpoint number can vary from system to system, let's avoid
>> putting it in the test name.  It's better to reduce the differences
>> in
>> test names between runs, when possible.
> 
> I didn't get it.  Does it mean that we need to use fixed internal
> breakpoint number instead of dynamically getting one?  Like this?
>     gdb_test "maint info break -1" \
>         "-1.*$bp_addr.*" \
>         "maint info breakpoint -1"

No, just to use a fixed test message, for example:

gdb_test "maint info break $bp_num" \
    "$bp_num.*$bp_addr.*" \
    "maint info breakpoint \$bp_num"

With this, the test name will literally be "maint info breakpoint
$bp_num".  Otherwise, if you run it on your computer and the breakpoint
number happens to be -1, and I run it on my computer, and the breakpoint
number happens to be -2, it makes it (slightly) more difficult to
compare test runs.  For the same reason, we try to avoid paths in test
names, as they are specific to each system.

Simon

[PATCH v2] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Enze Li]

Tom and Simon feedback that there is a test failing in this 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)

Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
fixed the nit in this patch.  The root of the problem is not correctly
matching the presentation of internal breakpoints.

In addition, as Pedro pointed out, the original testcase is not portable
in some methods, so this patch fixes this issue and some other
improvements.

Tested on x86_64 ubuntu 20.04.4 and openSUSE Tumbleweed(VERSION_ID="20220420").
---
 gdb/testsuite/gdb.base/clear_non_user_bp.exp | 64 ++++++++++++++------
 1 file changed, 47 insertions(+), 17 deletions(-)

diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
index d7bb8ab7e9a..383f52a31ed 100644
--- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
+++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
@@ -16,17 +16,45 @@
 # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
 # breakpoints with clear command.
 
-proc get_maint_info_bp { var } {
-    global expect_out
-    global gdb_prompt
-
+# get_maint_bp_addr VAR
+#
+# Purpose:
+#    Get address of the specified internal breakpoint when using command
+#    "maint info breakpoints VAR".
+# 
+# Parameter:
+#    VAR indicates address of the internal breakpoint to get.  Note that
+#    this parameter must be a negative number.  E.g., -1 means the first
+#    internal breakpoint.
+# 
+# Return:
+#    Internal breakpoint address.
+#
+proc get_maint_bp_addr { var } {
     gdb_test_multiple "maint info break $var" "find address of internal bp $var" {
-	-re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
+	-re -wrap ".*(0x\[0-9a-f\]+).*" {
 	    return $expect_out(1,string)
 	}
-	timeout {
-	    perror "couldn't find address of $var"
-	    return ""
+    }
+    return ""
+}
+
+# get_maint_bp_num
+#
+# Purpose:
+#    Get the first number of all internal breakpoints when using command
+#    "maint info breakpoints".
+# 
+# Parameter:
+#    None.
+#
+# Return:
+#    Internal breakpoint number, which is negative.
+#
+proc get_maint_bp_num { } {
+    gdb_test_multiple "maint info break" "find first internal bp num" {
+	-re -wrap "(-\[0-9\]).*" {
+	    return $expect_out(1,string)
 	}
     }
     return ""
@@ -48,17 +76,19 @@ if ![runto_main] then {
     return 0
 }
 
-gdb_test "break main.c:21" \
-    ".*Breakpoint.* at .*" \
-    "set breakpoint"
-
-set bp_addr [get_maint_info_bp "-1"]
+set bp_num [get_maint_bp_num]
+set bp_addr [get_maint_bp_addr $bp_num]
 
-gdb_test "maint info break -1" \
-    "-1.*shlib events.*keep y.*$bp_addr.*" \
-    "maint info breakpoint -1 error"
+gdb_test "maint info break $bp_num" \
+    "$bp_num.*$bp_addr.*" \
+    "maint info breakpoint \$bp_num"
 
 gdb_test "clear *$bp_addr" \
     "No breakpoint at \\*$bp_addr." \
-    "clear internal breakpoint error"
+    "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"
 
-- 
2.35.3

Re: [PATCH v2] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Simon Marchi]

Hi Enze,

The patch LGTM, with the following nits addressed:

On 2022-04-23 04:23, Enze Li wrote:
> Tom and Simon feedback that there is a test failing in this 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)
> 
> Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
> fixed the nit in this patch.  The root of the problem is not correctly
> matching the presentation of internal breakpoints.
> 
> In addition, as Pedro pointed out, the original testcase is not portable
> in some methods, so this patch fixes this issue and some other
> improvements.
> 
> Tested on x86_64 ubuntu 20.04.4 and openSUSE Tumbleweed(VERSION_ID="20220420").

When applying, git tells me:


Applying: gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp
.git/rebase-apply/patch:22: trailing whitespace.
# 
.git/rebase-apply/patch:27: trailing whitespace.
# 
.git/rebase-apply/patch:49: trailing whitespace.
# 
warning: 3 lines add whitespace errors.

Can you fix those?

Also, testsuit -> testsuite in the subject.

> ---
>  gdb/testsuite/gdb.base/clear_non_user_bp.exp | 64 ++++++++++++++------
>  1 file changed, 47 insertions(+), 17 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> index d7bb8ab7e9a..383f52a31ed 100644
> --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> @@ -16,17 +16,45 @@
>  # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
>  # breakpoints with clear command.
>  
> -proc get_maint_info_bp { var } {
> -    global expect_out
> -    global gdb_prompt
> -
> +# get_maint_bp_addr VAR
> +#
> +# Purpose:
> +#    Get address of the specified internal breakpoint when using command
> +#    "maint info breakpoints VAR".
> +# 
> +# Parameter:
> +#    VAR indicates address of the internal breakpoint to get.  Note that

I think this should say "the number of the internal breakpoint to get"?
Maybe the parameter could be named "num" or something like that then,
I'm not sure what "var" means here.

> +#    this parameter must be a negative number.  E.g., -1 means the first
> +#    internal breakpoint.
> +# 
> +# Return:
> +#    Internal breakpoint address.
> +#

Thanks for the A1 formatting of those comments :).

> +proc get_maint_bp_addr { var } {
>      gdb_test_multiple "maint info break $var" "find address of internal bp $var" {
> -	-re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
> +	-re -wrap ".*(0x\[0-9a-f\]+).*" {
>  	    return $expect_out(1,string)
>  	}
> -	timeout {
> -	    perror "couldn't find address of $var"
> -	    return ""
> +    }
> +    return ""
> +}
> +
> +# get_maint_bp_num
> +#
> +# Purpose:
> +#    Get the first number of all internal breakpoints when using command
> +#    "maint info breakpoints".
> +# 
> +# Parameter:
> +#    None.
> +#
> +# Return:
> +#    Internal breakpoint number, which is negative.
> +#
> +proc get_maint_bp_num { } {

Perhaps this proc could be named "get_first_maint_bp_num", to be more
precise.

Thanks,

Simon

[PATCH v3] gdb/testsuite: fix FAIL in gdb.base/clear_non_user_bp.exp

[Enze Li]

Tom and Simon feedback that there is a test failing in this 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)

Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
fixed the nit in this patch.  The root of the problem is not correctly
matching the presentation of internal breakpoints.

In addition, as Pedro pointed out, the original testcase is not portable
in some methods, so this patch fixes this issue and some other
improvements.

Tested on x86_64 ubuntu 20.04.4 and openSUSE Tumbleweed(VERSION_ID="20220425").
---
 gdb/testsuite/gdb.base/clear_non_user_bp.exp | 66 ++++++++++++++------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
index d7bb8ab7e9a..26d7a31fa47 100644
--- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
+++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
@@ -16,17 +16,45 @@
 # Regression test for PR gdb/7161.  Test that GDB cannot delete non-user
 # breakpoints with clear command.
 
-proc get_maint_info_bp { var } {
-    global expect_out
-    global gdb_prompt
-
-    gdb_test_multiple "maint info break $var" "find address of internal bp $var" {
-	-re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
+# 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)
 	}
-	timeout {
-	    perror "couldn't find address of $var"
-	    return ""
+    }
+    return ""
+}
+
+# get_first_maint_bp_num
+#
+# Purpose:
+#    Get the first number of all internal breakpoints when using command
+#    "maint info breakpoints".
+#
+# Parameter:
+#    None.
+#
+# Return:
+#    Internal breakpoint number, which is negative.
+#
+proc get_first_maint_bp_num { } {
+    gdb_test_multiple "maint info break" "find first internal bp num" {
+	-re -wrap "(-\[0-9\]).*" {
+	    return $expect_out(1,string)
 	}
     }
     return ""
@@ -48,17 +76,19 @@ if ![runto_main] then {
     return 0
 }
 
-gdb_test "break main.c:21" \
-    ".*Breakpoint.* at .*" \
-    "set breakpoint"
+set bp_num [get_first_maint_bp_num]
+set bp_addr [get_maint_bp_addr $bp_num]
 
-set bp_addr [get_maint_info_bp "-1"]
-
-gdb_test "maint info break -1" \
-    "-1.*shlib events.*keep y.*$bp_addr.*" \
-    "maint info breakpoint -1 error"
+gdb_test "maint info break $bp_num" \
+    "$bp_num.*$bp_addr.*" \
+    "maint info breakpoint \$bp_num"
 
 gdb_test "clear *$bp_addr" \
     "No breakpoint at \\*$bp_addr." \
-    "clear internal breakpoint error"
+    "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"
 
-- 
2.36.0

Re: [PATCH v3] gdb/testsuite: fix FAIL in gdb.base/clear_non_user_bp.exp

[Simon Marchi]

On 2022-04-27 09:26, Enze Li wrote:
> Tom and Simon feedback that there is a test failing in this 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)
> 
> Then, I reproduced the same fail with Ubuntu 20.04 as Simon said, and I
> fixed the nit in this patch.  The root of the problem is not correctly
> matching the presentation of internal breakpoints.
> 
> In addition, as Pedro pointed out, the original testcase is not 
> portable
> in some methods, so this patch fixes this issue and some other
> improvements.
> 
> Tested on x86_64 ubuntu 20.04.4 and openSUSE 
> Tumbleweed(VERSION_ID="20220425").

Thanks, this looks good to me, please push.

Simon

Re: [PATCH v2] gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp

[Enze Li]

Hi Simon,

Thanks for the review.  I've fixed all nits that you pointed out, and I
posted the v3 here:
https://sourceware.org/pipermail/gdb-patches/2022-April/188432.html

Thanks,
Enze

On Tue, 2022-04-26 at 10:50 -0400, Simon Marchi wrote:
> Hi Enze,
> 
> The patch LGTM, with the following nits addressed:
> 
> On 2022-04-23 04:23, Enze Li wrote:
> > Tom and Simon feedback that there is a test failing in this 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)
> > 
> > Then, I reproduced the same fail with Ubuntu 20.04 as Simon said,
> > and I
> > fixed the nit in this patch.  The root of the problem is not
> > correctly
> > matching the presentation of internal breakpoints.
> > 
> > In addition, as Pedro pointed out, the original testcase is not
> > portable
> > in some methods, so this patch fixes this issue and some other
> > improvements.
> > 
> > Tested on x86_64 ubuntu 20.04.4 and openSUSE
> > Tumbleweed(VERSION_ID="20220420").
> 
> When applying, git tells me:
> 
> 
> Applying: gdb/testsuit: fix FAIL in gdb.base/clear_non_user_bp.exp
> .git/rebase-apply/patch:22: trailing whitespace.
> # 
> .git/rebase-apply/patch:27: trailing whitespace.
> # 
> .git/rebase-apply/patch:49: trailing whitespace.
> # 
> warning: 3 lines add whitespace errors.
> 
> Can you fix those?
> 
> Also, testsuit -> testsuite in the subject.
> 
> > ---
> >  gdb/testsuite/gdb.base/clear_non_user_bp.exp | 64 ++++++++++++++--
> > ----
> >  1 file changed, 47 insertions(+), 17 deletions(-)
> > 
> > diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > index d7bb8ab7e9a..383f52a31ed 100644
> > --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp
> > @@ -16,17 +16,45 @@
> >  # Regression test for PR gdb/7161.  Test that GDB cannot delete
> > non-user
> >  # breakpoints with clear command.
> >  
> > -proc get_maint_info_bp { var } {
> > -    global expect_out
> > -    global gdb_prompt
> > -
> > +# get_maint_bp_addr VAR
> > +#
> > +# Purpose:
> > +#    Get address of the specified internal breakpoint when using
> > command
> > +#    "maint info breakpoints VAR".
> > +# 
> > +# Parameter:
> > +#    VAR indicates address of the internal breakpoint to get. 
> > Note that
> 
> I think this should say "the number of the internal breakpoint to
> get"?
> Maybe the parameter could be named "num" or something like that then,
> I'm not sure what "var" means here.
> 
> > +#    this parameter must be a negative number.  E.g., -1 means the
> > first
> > +#    internal breakpoint.
> > +# 
> > +# Return:
> > +#    Internal breakpoint address.
> > +#
> 
> Thanks for the A1 formatting of those comments :).
> 
> > +proc get_maint_bp_addr { var } {
> >      gdb_test_multiple "maint info break $var" "find address of
> > internal bp $var" {
> > -       -re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" {
> > +       -re -wrap ".*(0x\[0-9a-f\]+).*" {
> >             return $expect_out(1,string)
> >         }
> > -       timeout {
> > -           perror "couldn't find address of $var"
> > -           return ""
> > +    }
> > +    return ""
> > +}
> > +
> > +# get_maint_bp_num
> > +#
> > +# Purpose:
> > +#    Get the first number of all internal breakpoints when using
> > command
> > +#    "maint info breakpoints".
> > +# 
> > +# Parameter:
> > +#    None.
> > +#
> > +# Return:
> > +#    Internal breakpoint number, which is negative.
> > +#
> > +proc get_maint_bp_num { } {
> 
> Perhaps this proc could be named "get_first_maint_bp_num", to be more
> precise.
> 
> Thanks,
> 
> Simon