From: Andrew Burgess <aburgess@redhat.com>
To: Pedro Alves <pedro@palves.net>, gdb-patches@sourceware.org
Subject: Re: [PATCH 6/8] gdb/testsuite: introduce is_target_non_stop helper proc
Date: Mon, 27 Feb 2023 14:58:49 +0000 [thread overview]
Message-ID: <87ttz7i28m.fsf@redhat.com> (raw)
In-Reply-To: <d4fdf042-5a63-d1a7-a1a6-b075e4edbef9@palves.net>
Pedro Alves <pedro@palves.net> writes:
> On 2023-02-20 2:13 p.m., Andrew Burgess via Gdb-patches wrote:
>> I noticed that several test included copy & pasted code to run the
>
> "several test" -> "several tests"
>
Thanks, fixed.
>> 'maint show target-non-stop' command, and then switch based on the
>> result.
>>
>> In this commit I factor this code out into a helper proc in
>> lib/gdb.exp, and update all the places I could find that used this
>> pattern to make use of the helper proc.
>>
>> There should be no change in what is tested after this commit.
>> ---
>> gdb/testsuite/gdb.base/access-mem-running.exp | 10 +++-------
>> gdb/testsuite/gdb.base/fork-running-state.exp | 11 +++--------
>> .../access-mem-running-thread-exit.exp | 10 +++-------
>> .../gdb.threads/clone-attach-detach.exp | 11 +++--------
>> .../gdb.threads/detach-step-over.exp | 8 ++------
>> gdb/testsuite/lib/gdb.exp | 19 +++++++++++++++++++
>> 6 files changed, 33 insertions(+), 36 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/access-mem-running.exp b/gdb/testsuite/gdb.base/access-mem-running.exp
>> index 90ddedc470d..d0f7871fc0f 100644
>> --- a/gdb/testsuite/gdb.base/access-mem-running.exp
>> +++ b/gdb/testsuite/gdb.base/access-mem-running.exp
>> @@ -46,13 +46,9 @@ proc test { non_stop } {
>> && ([target_info gdb_protocol] == "remote"
>> || [target_info gdb_protocol] == "extended-remote")} {
>>
>> - gdb_test_multiple "maint show target-non-stop" "" {
>> - -wrap -re "(is|currently) on.*" {
>> - }
>> - -wrap -re "(is|currently) off.*" {
>> - unsupported "can't issue commands while target is running"
>> - return 0
>> - }
>> + if {![is_target_non_stop]} {
>> + unsupported "can't issue commands while target is running"
>> + return 0
>> }
>> }
>>
>> diff --git a/gdb/testsuite/gdb.base/fork-running-state.exp b/gdb/testsuite/gdb.base/fork-running-state.exp
>> index b8045ad490f..9a18193c4dc 100644
>> --- a/gdb/testsuite/gdb.base/fork-running-state.exp
>> +++ b/gdb/testsuite/gdb.base/fork-running-state.exp
>> @@ -46,14 +46,9 @@ proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
>> && ([target_info gdb_protocol] == "remote"
>> || [target_info gdb_protocol] == "extended-remote")} {
>>
>> - set test "maint show target-non-stop"
>> - gdb_test_multiple "maint show target-non-stop" $test {
>> - -re "(is|currently) on.*$gdb_prompt $" {
>> - }
>> - -re "(is|currently) off.*$gdb_prompt $" {
>> - unsupported "can't issue info threads while target is running"
>> - return 0
>> - }
>> + if {![is_target_non_stop]} {
>> + unsupported "can't issue info threads while target is running"
>> + return 0
>> }
>> }
>>
>> diff --git a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
>> index 09b05480c04..2d3ace44ccd 100644
>> --- a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
>> +++ b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
>> @@ -65,13 +65,9 @@ proc test { non_stop } {
>> && ([target_info gdb_protocol] == "remote"
>> || [target_info gdb_protocol] == "extended-remote")} {
>>
>> - gdb_test_multiple "maint show target-non-stop" "" {
>> - -wrap -re "(is|currently) on.*" {
>> - }
>> - -wrap -re "(is|currently) off.*" {
>> - unsupported "can't issue commands while target is running"
>> - return 0
>> - }
>> + if {![is_target_non_stop]} {
>> + unsupported "can't issue commands while target is running"
>> + return 0
>> }
>> }
>>
>> diff --git a/gdb/testsuite/gdb.threads/clone-attach-detach.exp b/gdb/testsuite/gdb.threads/clone-attach-detach.exp
>> index ac9b92d5f57..a71713ea8ba 100644
>> --- a/gdb/testsuite/gdb.threads/clone-attach-detach.exp
>> +++ b/gdb/testsuite/gdb.threads/clone-attach-detach.exp
>> @@ -64,14 +64,9 @@ if {[target_info exists gdb_protocol]
>> && ([target_info gdb_protocol] == "remote"
>> || [target_info gdb_protocol] == "extended-remote")} {
>>
>> - set test "maint show target-non-stop"
>> - gdb_test_multiple "maint show target-non-stop" $test {
>> - -re "(is|currently) on.*$gdb_prompt $" {
>> - }
>> - -re "(is|currently) off.*$gdb_prompt $" {
>> - unsupported "bg attach: can't issue info threads while target is running"
>> - return 0
>> - }
>> + if {![is_target_non_stop]} {
>> + unsupported "bg attach: can't issue info threads while target is running"
>> + return 0
>> }
>> }
>>
>> diff --git a/gdb/testsuite/gdb.threads/detach-step-over.exp b/gdb/testsuite/gdb.threads/detach-step-over.exp
>> index ed9dc1aab88..bf5ef6b06a1 100644
>> --- a/gdb/testsuite/gdb.threads/detach-step-over.exp
>> +++ b/gdb/testsuite/gdb.threads/detach-step-over.exp
>> @@ -314,12 +314,8 @@ proc_with_prefix test_detach_quit {condition_eval target_non_stop \
>> start_gdb_for_test $condition_eval $target_non_stop \
>> $non_stop $displaced
>>
>> - gdb_test_multiple "maint show target-non-stop" "" {
>> - -wrap -re "(is|currently) on.*" {
>> - }
>> - -wrap -re "(is|currently) off.*" {
>> - return
>> - }
>> + if {![is_target_non_stop]} {
>> + return
>> }
>> }
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index e48228ed4f6..ef62174301e 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -9303,6 +9303,25 @@ proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
>> }
>> }
>>
>> +# Return true if the current target is operating in non-stop mode,
>> +# otherwise, return false.
>> +#
>> +# The inferior will need to have started running in order to get the
>> +# correct result.
>> +
>> +proc is_target_non_stop { {testname ""} } {
>> + set is_non_stop false
>> + gdb_test_multiple "maint show target-non-stop" $testname {
>> + -wrap -re "(is|currently) on.*" {
>> + set is_non_stop true
>> + }
>> + -wrap -re "(is|currently) off.*" {
>> + set is_non_stop false
>> + }
>> + }
>> + return $is_non_stop
>> +}
>
> The original code defaulted to "true", while this defaults to "false".
>
> I.e., a plain refactor that doesn't change behavior would look like this:
>
> proc is_target_non_stop { {testname ""} } {
> gdb_test_multiple "maint show target-non-stop" $testname {
> -wrap -re "(is|currently) on.*" {
> }
> -wrap -re "(is|currently) off.*" {
> return false
> }
> }
> return true
> }
>
> The original code was written that way so that we'd only skip
> the remainder of the tests if we're absolutely sure they
> wouldn't work. If the maint command failed for some reason, we
> don't know that, so we continue with the testcase.
>
> I think that for a refactoring patch, that detail shouldn't be
> changed.
Thanks for the feedback. How's the patch below? The only change is in
testsuite/lib/gdb.exp, everything else is the same as before.
Thanks,
Andrew
---
commit 40b895a3386c8472e54bc8bb5c3eae01d7123b9a
Author: Andrew Burgess <aburgess@redhat.com>
Date: Sat Feb 18 20:52:40 2023 +0000
gdb/testsuite: introduce is_target_non_stop helper proc
I noticed that several tests included copy & pasted code to run the
'maint show target-non-stop' command, and then switch based on the
result.
In this commit I factor this code out into a helper proc in
lib/gdb.exp, and update all the places I could find that used this
pattern to make use of the helper proc.
There should be no change in what is tested after this commit.
diff --git a/gdb/testsuite/gdb.base/access-mem-running.exp b/gdb/testsuite/gdb.base/access-mem-running.exp
index 90ddedc470d..d0f7871fc0f 100644
--- a/gdb/testsuite/gdb.base/access-mem-running.exp
+++ b/gdb/testsuite/gdb.base/access-mem-running.exp
@@ -46,13 +46,9 @@ proc test { non_stop } {
&& ([target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote")} {
- gdb_test_multiple "maint show target-non-stop" "" {
- -wrap -re "(is|currently) on.*" {
- }
- -wrap -re "(is|currently) off.*" {
- unsupported "can't issue commands while target is running"
- return 0
- }
+ if {![is_target_non_stop]} {
+ unsupported "can't issue commands while target is running"
+ return 0
}
}
diff --git a/gdb/testsuite/gdb.base/fork-running-state.exp b/gdb/testsuite/gdb.base/fork-running-state.exp
index b8045ad490f..9a18193c4dc 100644
--- a/gdb/testsuite/gdb.base/fork-running-state.exp
+++ b/gdb/testsuite/gdb.base/fork-running-state.exp
@@ -46,14 +46,9 @@ proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
&& ([target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote")} {
- set test "maint show target-non-stop"
- gdb_test_multiple "maint show target-non-stop" $test {
- -re "(is|currently) on.*$gdb_prompt $" {
- }
- -re "(is|currently) off.*$gdb_prompt $" {
- unsupported "can't issue info threads while target is running"
- return 0
- }
+ if {![is_target_non_stop]} {
+ unsupported "can't issue info threads while target is running"
+ return 0
}
}
diff --git a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
index 09b05480c04..2d3ace44ccd 100644
--- a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
+++ b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
@@ -65,13 +65,9 @@ proc test { non_stop } {
&& ([target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote")} {
- gdb_test_multiple "maint show target-non-stop" "" {
- -wrap -re "(is|currently) on.*" {
- }
- -wrap -re "(is|currently) off.*" {
- unsupported "can't issue commands while target is running"
- return 0
- }
+ if {![is_target_non_stop]} {
+ unsupported "can't issue commands while target is running"
+ return 0
}
}
diff --git a/gdb/testsuite/gdb.threads/clone-attach-detach.exp b/gdb/testsuite/gdb.threads/clone-attach-detach.exp
index ac9b92d5f57..a71713ea8ba 100644
--- a/gdb/testsuite/gdb.threads/clone-attach-detach.exp
+++ b/gdb/testsuite/gdb.threads/clone-attach-detach.exp
@@ -64,14 +64,9 @@ if {[target_info exists gdb_protocol]
&& ([target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote")} {
- set test "maint show target-non-stop"
- gdb_test_multiple "maint show target-non-stop" $test {
- -re "(is|currently) on.*$gdb_prompt $" {
- }
- -re "(is|currently) off.*$gdb_prompt $" {
- unsupported "bg attach: can't issue info threads while target is running"
- return 0
- }
+ if {![is_target_non_stop]} {
+ unsupported "bg attach: can't issue info threads while target is running"
+ return 0
}
}
diff --git a/gdb/testsuite/gdb.threads/detach-step-over.exp b/gdb/testsuite/gdb.threads/detach-step-over.exp
index ed9dc1aab88..bf5ef6b06a1 100644
--- a/gdb/testsuite/gdb.threads/detach-step-over.exp
+++ b/gdb/testsuite/gdb.threads/detach-step-over.exp
@@ -314,12 +314,8 @@ proc_with_prefix test_detach_quit {condition_eval target_non_stop \
start_gdb_for_test $condition_eval $target_non_stop \
$non_stop $displaced
- gdb_test_multiple "maint show target-non-stop" "" {
- -wrap -re "(is|currently) on.*" {
- }
- -wrap -re "(is|currently) off.*" {
- return
- }
+ if {![is_target_non_stop]} {
+ return
}
}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 669a5f606d6..19c782bea46 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9298,6 +9298,28 @@ proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
}
}
+# Return false if the current target is not operating in non-stop
+# mode, otherwise, return true.
+#
+# The inferior will need to have started running in order to get the
+# correct result.
+
+proc is_target_non_stop { {testname ""} } {
+ # For historical reasons we assume non-stop mode is on. If the
+ # maintenance command fails for any reason then we're going to
+ # return true.
+ set is_non_stop true
+ gdb_test_multiple "maint show target-non-stop" $testname {
+ -wrap -re "(is|currently) on.*" {
+ set is_non_stop true
+ }
+ -wrap -re "(is|currently) off.*" {
+ set is_non_stop false
+ }
+ }
+ return $is_non_stop
+}
+
# Check if the compiler emits epilogue information associated
# with the closing brace or with the last statement line.
#
next prev parent reply other threads:[~2023-02-27 14:58 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-20 14:13 [PATCH 0/8] Fix missing MI =breakpoint-deleted notifications Andrew Burgess
2023-02-20 14:13 ` [PATCH 1/8] gdb: remove an out of date comment about disp_del_at_next_stop Andrew Burgess
2023-02-20 14:13 ` [PATCH 2/8] gdb: don't duplicate 'thread' field in MI breakpoint output Andrew Burgess
2023-02-20 14:47 ` Eli Zaretskii
2023-02-22 15:18 ` Pedro Alves
2023-02-20 14:13 ` [PATCH 3/8] gdb/testsuite: make more use of mi-support.exp Andrew Burgess
2023-02-20 14:13 ` [PATCH 4/8] gdb/testsuite: extend the use of mi_clean_restart Andrew Burgess
2023-02-20 14:13 ` [PATCH 5/8] gdb/testsuite introduce foreach_mi_ui_mode helper proc Andrew Burgess
2023-02-22 15:18 ` Pedro Alves
2023-02-20 14:13 ` [PATCH 6/8] gdb/testsuite: introduce is_target_non_stop " Andrew Burgess
2023-02-22 15:19 ` Pedro Alves
2023-02-27 14:58 ` Andrew Burgess [this message]
2023-02-27 16:18 ` Pedro Alves
2023-02-20 14:13 ` [PATCH 7/8] gdb/testsuite: fix failure in gdb.mi/mi-pending.exp with extended-remote Andrew Burgess
2023-02-22 15:19 ` Pedro Alves
2023-02-20 14:13 ` [PATCH 8/8] gdb: fix mi breakpoint-deleted notifications for thread-specific b/p Andrew Burgess
2023-02-22 15:20 ` Pedro Alves
2023-02-22 15:23 ` [PATCH 0/8] Fix missing MI =breakpoint-deleted notifications Pedro Alves
2023-02-28 11:09 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ttz7i28m.fsf@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=pedro@palves.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).