Re: [PATCH 5/5] Make gdb_test's question non-optional if specified

[Bruno Larsen <blarsen@redhat.com>]

On 3/30/22 16:29, Pedro Alves wrote:
> gdb_test supports handling scenarios where GDB asks a question before
> finishing handling some command.  The full prototype of gdb_test is:
> 
>    # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
> 
> However, QUESTION is a question that GDB _may_ ask, not one that GDB
> _must_ ask:
> 
>   # QUESTION is a question GDB may ask in response to COMMAND, like
>   #   "are you sure?"
>   # RESPONSE is the response to send if QUESTION appears.
> 
> If GDB doesn't raise the question, the test still passes.
> 
> I think that this is a misfeature.  If GDB regresses and stops asking
> a question, the testsuite won't notice.  So I think that if a QUESTION
> is specified, gdb_test should ensure it comes out of GDB.

I just ran into a neat use of this, or possibly a mis-use, depending on how much you like it. gdb.base/skip.exp uses:

gdb_test "step" "desired spot" "go to desired spot" "undesirable spot" "step"

as a simple way to handle a gcc 9.2.0 bug (misfeature) where gdb could stop in an undesirable spot without actually causing a failure in the test. This feels like a neat way to deal with compiler problems, as it would introduce a simple way to deal with clang's lack of epilogue, for instance.

I'm not suggesting that this patch be scrapped, but maybe this could be implemented on purpose, something like gdb_test_optional. The code itself LGTM, but I am not able to approve patches.

-- 
Cheers!
Bruno Larsen
> 
> Running the testsuite exposed a number of tests that pass
> QUESTION/RESPONSE to GDB, but no question comes out.  The previous
> commits fixed them all, so this commit changes gdb_test's behavior.
> 
> A related issue is that gdb_test doesn't enforce that if you specify
> QUESTION, that you also specify RESPONSE.  I.e., you should pass 1, 2,
> 3, or 5 arguments to gdb_test, but never 4, or more than 5.  Making
> gdb_test detect bogus arguments actually regressed some testcases,
> also all fixed in previous commits.
> 
> Change-Id: I47c39c9034e6a6841129312037a5ca4c5811f0db
> ---
>   gdb/testsuite/lib/gdb.exp | 25 +++++++++++++++++++++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0b242b64992..e9cbbe77fa4 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1324,9 +1324,10 @@ proc gdb_test_multiline { name args } {
>   #   omitted, then the pass/fail messages use the command string as the
>   #   message.  (If this is the empty string, then sometimes we don't
>   #   call pass or fail at all; I don't understand this at all.)
> -# QUESTION is a question GDB may ask in response to COMMAND, like
> -#   "are you sure?"
> -# RESPONSE is the response to send if QUESTION appears.
> +# QUESTION is a question GDB should ask in response to COMMAND, like
> +#   "are you sure?"  If this is specified, the test fails if GDB
> +#   doesn't print the question.
> +# RESPONSE is the response to send when QUESTION appears.
>   #
>   # Returns:
>   #    1 if the test failed,
> @@ -1337,6 +1338,11 @@ proc gdb_test { args } {
>       global gdb_prompt
>       upvar timeout timeout
>   
> +    # Can't have a question without a response.
> +    if { [llength $args] == 4 || [llength $args] > 5 } {
> +	error "Unexpected arguments: $args"
> +    }
> +
>       if [llength $args]>2 then {
>   	set message [lindex $args 2]
>       } else {
> @@ -1345,11 +1351,21 @@ proc gdb_test { args } {
>       set command [lindex $args 0]
>       set pattern [lindex $args 1]
>   
> +    set must_see_question 0
> +    if { [llength $args] == 5 } {
> +	set must_see_question 1
> +	set saw_question 0
> +    }
> +
>       set user_code {}
>       lappend user_code {
>   	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$gdb_prompt $" {
>   	    if ![string match "" $message] then {
> -		pass "$message"
> +		if {$must_see_question} {
> +		    gdb_assert $saw_question "$message"
> +		} else {
> +		    pass "$message"
> +		}
>               }
>           }
>       }
> @@ -1359,6 +1375,7 @@ proc gdb_test { args } {
>   	set response_string [lindex $args 4]
>   	lappend user_code {
>   	    -re "(${question_string})$" {
> +		set saw_question 1
>   		send_gdb "$response_string\n"
>   		exp_continue
>   	    }