Re: [PATCH][gdb/testsuite] Fix handling of nr_args < 3 in mi_gdb_test

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom de Vries <tdevries@suse.de> [2021-09-09 15:19:13 +0200]:

> Hi,
> 
> The documentation of mi_gdb_test states that the command, pattern and message
> arguments are mandatory:
> ...
>  # mi_gdb_test COMMAND PATTERN MESSAGE [IPATTERN] -- send a command to gdb;
>  #   test the result.
> ...
> 
> However, this is not checked, and when mi_gdb_test is called with less than 3
> arguments, it passes or fails silently.
> 
> Fix this by using the following semantics:
> - if there are less than 2 arguments, use the command as the message.
> - if there is less than 1 argument, use ".*" as the pattern.
> - if there are no or too much arguments, error out.

I think this description is wrong, you mean:

  - if there are 2 arguments, .....
  - if there is just 1 argument, .....
  - if there are no or too many arguments, ....

Otherwise, LGTM.

Thanks,
Andrew


> 
> Fix a PATH issue in gdb.mi/mi-logging.exp, introduced by using the command as
> message.  Fix a few other trivial-looking FAILs.
> 
> There are 11 less trivial-looking FAILs left in gdb.mi in test-cases:
> - mi-nsmoribund.exp
> - mi-breakpoint-changed.exp
> - mi-break.exp.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Fix handling of nr_args < 3 in mi_gdb_test
> 
> ---
>  gdb/testsuite/gdb.mi/mi-break.exp          |  2 +-
>  gdb/testsuite/gdb.mi/mi-logging.exp        |  2 +-
>  gdb/testsuite/gdb.mi/mi-memory-changed.exp |  5 +++--
>  gdb/testsuite/gdb.mi/mi-nsmoribund.exp     |  2 +-
>  gdb/testsuite/lib/mi-support.exp           | 28 ++++++++++++++++++++++++----
>  5 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
> index 7d01a936947..86916c42c16 100644
> --- a/gdb/testsuite/gdb.mi/mi-break.exp
> +++ b/gdb/testsuite/gdb.mi/mi-break.exp
> @@ -341,7 +341,7 @@ proc_with_prefix test_explicit_breakpoints {} {
>  
>      # First check mixed explicit/parsed linespecs.
>      mi_gdb_test "-break-insert --function main $srcfile:$line_callee3_head" \
> -	".*Garbage following explicit linespec"
> +	".*Garbage following explicit location.*"
>  
>      # Insert some breakpoints and list them
>      # Also, disable some so they do not interfere with other tests
> diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
> index ff47b0849c0..830ade91c6d 100644
> --- a/gdb/testsuite/gdb.mi/mi-logging.exp
> +++ b/gdb/testsuite/gdb.mi/mi-logging.exp
> @@ -32,7 +32,7 @@ if {[mi_runto_main] < 0} {
>  
>  set milogfile [standard_output_file "milog.txt"]
>  
> -mi_gdb_test "-gdb-set logging file $milogfile" ".*"
> +mi_gdb_test "-gdb-set logging file $milogfile" ".*" "-gdb-set logging file"
>  
>  mi_gdb_test "-gdb-set logging overwrite on" ".*"
>  
> diff --git a/gdb/testsuite/gdb.mi/mi-memory-changed.exp b/gdb/testsuite/gdb.mi/mi-memory-changed.exp
> index 4843e91dc0a..1116abffb29 100644
> --- a/gdb/testsuite/gdb.mi/mi-memory-changed.exp
> +++ b/gdb/testsuite/gdb.mi/mi-memory-changed.exp
> @@ -75,7 +75,8 @@ gdb_expect {
>      }
>  }
>  
> -mi_gdb_test "set var *(unsigned int *) ${main_addr} = ${main_insn}" \
> -    ".*=memory-changed,thread-group=\"i${decimal}\".addr=\"${main_addr}\",len=\"0x4\",type=\"code\".*\\^done"
> +regsub 0x $main_addr "" main_addr
> +mi_gdb_test "set var *(unsigned int *) 0x${main_addr} = ${main_insn}" \
> +    ".*=memory-changed,thread-group=\"i${decimal}\".addr=\"0x0*${main_addr}\",len=\"0x4\",type=\"code\".*\\^done"
>  mi_gdb_exit
>  return 0
> diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
> index 9dc3f7fdb63..b0b3b7ff1aa 100644
> --- a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
> +++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
> @@ -115,7 +115,7 @@ mi_check_thread_states \
>  # gdb_test_multiple (or an MI equivalent)
>  
>  mi_gdb_test "102-break-delete" "102\\^done.*"
> -mi_gdb_test "print done = 1" { = 1"}
> +mi_gdb_test "print done = 1" "~\"\[$\]$decimal = 1\\\\n\"\r\n\\^done"
>  mi_gdb_test "103-exec-continue --all" "\[^\n\]*\r\n$running_re"
>  
>  gdb_expect {
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 78290031763..f6ee352b67e 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -691,16 +691,18 @@ proc mi_readnow { args } {
>      return $readnow_p
>  }
>  
> -# mi_gdb_test COMMAND PATTERN MESSAGE [IPATTERN] -- send a command to gdb; 
> +# mi_gdb_test COMMAND [PATTERN [MESSAGE [IPATTERN]]] -- send a command to gdb;
>  #   test the result.
>  #
>  # COMMAND is the command to execute, send to GDB with send_gdb.  If
>  #   this is the null string no command is sent.
>  # PATTERN is the pattern to match for a PASS, and must NOT include
>  #   the \r\n sequence immediately before the gdb prompt.
> +#   If not specified, .* is used.
>  # MESSAGE is the message to be printed.  (If this is the empty string,
>  #   then sometimes we don't call pass or fail at all; I don't
>  #   understand this at all.)
> +#   If not specified, COMMAND is used.
>  # IPATTERN is the pattern to match for the inferior's output.  This parameter
>  #   is optional.  If present, it will produce a PASS if the match is
>  #   successful, and a FAIL if unsuccessful.
> @@ -717,9 +719,23 @@ proc mi_gdb_test { args } {
>      global inferior_exited_re async
>      upvar timeout timeout
>  
> -    set command [lindex $args 0]
> -    set pattern [lindex $args 1]
> -    set message [lindex $args 2]
> +    if { [llength $args] >= 1 } then {
> +	set command [lindex $args 0]
> +    } else {
> +	error "Not enough arguments in mi_gdb_test"
> +    }
> +
> +    if { [llength $args] >= 2 } then {
> +	set pattern [lindex $args 1]
> +    } else {
> +	set pattern ".*"
> +    }
> +
> +    if { [llength $args] >= 3 } then {
> +	set message [lindex $args 2]
> +    } else {
> +	set message $command
> +    }
>  
>      if [llength $args]==4 {
>  	set ipattern [lindex $args 3]
> @@ -732,6 +748,10 @@ proc mi_gdb_test { args } {
>  	set question_string "^FOOBAR$"
>      }
>  
> +    if { [llength $args] >= 6 } {
> +	error "Too many arguments in mi_gdb_test"
> +    }
> +
>      if $verbose>2 then {
>  	send_user "Sending \"$command\" to gdb\n"
>  	send_user "Looking to match \"$pattern\"\n"