Re: [PATCHv3] gdb/testsuite: modernize gdb.base/maint.exp

[Andrew Burgess <aburgess@redhat.com>]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> gdb.base/maint.exp was using several gdb_expect statements, probably
> because this test case predates the existance of gdb_test_multiple. This
> commit updates the test case to use gdb_test_multiple, making it more
> resilient to internal errors and such.
>
> The only gdb_expect left in the testcase is one that specifically looks
> for an internal error being triggered as a PASS.
> ---
>
> Changes for version 3:
>     * changed patterns when testing psymbols, now using trailing
> parenthesis
>     * used -lbl when running "check psymtabs"
>     * made use of boolean variables and gdb_assert when testing
> psymtabs
>     * Simplified msymbols and psymbosl test to 3 tests, instead of
> nested tests
>
> Changes for v2:
>  - Addressed Andrew's comments
>  - rebased on current master.
>
> ---
>  gdb/testsuite/gdb.base/maint.exp | 142 ++++++++++---------------------
>  1 file changed, 43 insertions(+), 99 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index 2817c6eafb9..aea8a10df0c 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -151,22 +151,20 @@ set have_psyms [expr ! ( $have_gdb_index || $readnow_p )]
>  # unless there is some problem with the symtabs and psymtabs
>  # so that branch will really never be covered in this tests here!!
>  #
> +# When there is a problem, there may be loads of output, which can
> +# overwhelm the expect buffer. Splitting it seems to fix those
> +# issues.
> +
> +set seen_command false
> +gdb_test_multiple "maint check-psymtabs" "" -lbl {

Bruno,

Thanks for continuing to work on this...

Sorry to be a pain, but I don't think -lbl will actually do what you
want here.  To me the implementation of -lbl just looks weird, and I'm
not sure how it is expected to help.

Your patterns are:

    -re "^maint check-psymtabs\r\n" {
	set seen_command true
	exp_continue
    }

    -re "^$gdb_prompt $" {
	gdb_assert { $seen_command } $gdb_test_name
    }

And -lbl adds:

    -re "\r\n\[^\r\n\]*(?=\r\n)" {
        exp_continue
    }

The problem I think exists here is that your first pattern runs from the
star of the buffer and consumes the trailing \r\n.  Your prompt pattern
also expects to start from the start of the buffer.

If the output is:

  maint check-psymtabs\r\n
  (gdb) 

Then these patterns will work fine.  But if your output is:

  maint check-psymtabs\r\n
  Some other line here\r\n
  (gdb)

Then your first pattern will consume the first line, including the
trailing \r\n.  The -lbl pattern will then fail to match the second line
as the -lbl pattern expects a leading \r\n.  And now your pattern will
fail to match the third line as the second line is still in the buffer.

The -lbl pattern always leaves a trailing \r\n in the expect buffer,
which will cause your prompt pattern to fail.

Personally, I'd just drop the use of -lbl and add what I think is the
more correct pattern:

  -re "^\[^\r\n\]+\r\n" {
    exp_continue
  }

which is what you originally proposed.  But if you really want to make
use of -lbl then you can change your patterns to:

    -re "^maint check-psymtabs(?=\r\n)" {
	set seen_command true
	exp_continue
    }

    -re "\r\n$gdb_prompt $" {
	gdb_assert { $seen_command } $gdb_test_name
    }

I don't think that's an improvement, but I think that should work.

> +    -re "^maint check-psymtabs\r\n" {
> +	set seen_command true
> +	exp_continue
> +    }
>  
> -# guo: on linux this command output is huge.  for some reason splitting up
> -# the regexp checks works.
> -#
> -send_gdb "maint check-psymtabs\n"
> -gdb_expect  {
> -    -re "^maint check-psymtabs" {
> -	gdb_expect {
> -	    -re "$gdb_prompt $" {
> -		pass "maint check-psymtabs"
> -	    }
> -	    timeout { fail "(timeout) maint check-psymtabs" }
> -	}
> +    -re "^$gdb_prompt $" {
> +	gdb_assert { $seen_command } $gdb_test_name
>      }
> -    -re ".*$gdb_prompt $"     { fail "maint check-psymtabs" }
> -    timeout         { fail "(timeout) maint check-psymtabs" }
>  }
>  
>  # This command does not produce any output unless there is some problem
> @@ -272,63 +270,33 @@ if { $have_psyms } {
>  		       "maint print psymbols -pc" \
>  		       "maint print psymbols -pc main $psymbols_output"]
>      foreach { test_name command } $test_list {
> -	send_gdb "$command\n"
> -	    gdb_expect  {
> -		-re "^maint print psymbols \[^\n\]*\r\n$gdb_prompt $" {
> -		    send_gdb "shell ls $psymbols_output\n"
> -		    gdb_expect {
> -			-re "$psymbols_output_re\r\n$gdb_prompt $" {
> -			    # We want this grep to be as specific as possible,
> -			    # so it's less likely to match symbol file names in
> -			    # psymbols_output.  Yes, this actually happened;
> -			    # poor expect got tons of output, and timed out
> -			    # trying to match it.   --- Jim Blandy <jimb@cygnus.com>
> -			    send_gdb "shell grep 'main.*function' $psymbols_output\n"
> -			    gdb_expect {
> -				-re ".main., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 1"
> -				}
> -				-re ".*main.  .., function, $hex.*$gdb_prompt $" {
> -				    pass "$test_name 2"
> -				}
> -				-re ".*$gdb_prompt $" { fail "$test_name" }
> -				timeout { fail "$test_name (timeout)" }
> -			    }
> -			    gdb_test "shell rm -f $psymbols_output" ".*" \
> -				"${test_name}: shell rm -f psymbols_output"
> +	gdb_test_multiple "$command" "$test_name" {
> +	    -re  -wrap "^maint print psymbols \[^\n\]*" {
> +		gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
> +		    "$test_name internal" {
> +			-re -wrap ".main., function, $hex.*" {
> +			    pass "$test_name (pattern 1)"
> +			}
> +			-re -wrap ".*main.  .., function, $hex.*" {
> +			    pass "$test_name (pattern 2)"
>  			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
>  		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
>  	    }
> +	}
> +	gdb_test "shell rm -f $psymbols_output" ".*" \
> +	    "${test_name}: shell rm -f psymbols_output"
>      }

Again, apologies for only just spotting this, but I think this test can
be further simplified to just:

    foreach { test_name command } $test_list {
        with_test_prefix "$test_name" {
          gdb_test_no_output "$command" "collect output"
          gdb_test_multiple "shell grep 'main.*function' $psymbols_output" \
            "$analyse output" {
  	        -re -wrap ".main., function, $hex.*" {
  	            pass "$gdb_test_name (pattern 1)"
  	        }
  	        -re -wrap ".*main.  .., function, $hex.*" {
  	            pass "$gdb_test_name (pattern 2)"
                }
          }
  	  gdb_test "shell rm -f $psymbols_output" ".*" \
  	      "shell rm -f psymbols_output"
        }
    }

I think we already made a similar change later on in this script, so
this just brings this test into line with the later ones.

Thanks,
Andrew

>  }
>  
>  
>  set msymbols_output [standard_output_file msymbols_output]
>  set msymbols_output_re [string_to_regexp $msymbols_output]
> -send_gdb "maint print msymbols -objfile ${binfile} $msymbols_output\n"
> -gdb_expect  {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -	send_gdb "shell ls $msymbols_output\n"
> -	gdb_expect {
> -	    -re "$msymbols_output_re\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial $msymbols_output" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, absolute pathname"
> -		gdb_test "shell rm -f $msymbols_output" ".*" \
> -		    "shell rm -f msymbols_output"
> -	    }
> -	    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -	    timeout { fail "maint print msymbols (timeout)" }
> -	}
> -    }
> -    -re ".*$gdb_prompt $" { fail "maint print msymbols" }
> -    timeout { fail "maint print msymbols (timeout)" }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${binfile} $msymbols_output" \
> +    "print msymbols to file, with absolute path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, absolute pathname"
> +gdb_test "shell rm -f $msymbols_output" ".*" "remove absolute path msymbols"
>  
>  # Check that maint print msymbols allows relative pathnames
>  set mydir [pwd]
> @@ -336,18 +304,13 @@ gdb_test "cd [standard_output_file {}]" \
>      "Working directory .*\..*" \
>      "cd to objdir"
>  
> -gdb_test_multiple "maint print msymbols -objfile ${testfile} msymbols_output2" "maint print msymbols" {
> -    -re "^maint print msymbols \[^\n\]*\r\n$gdb_prompt $" {
> -    	gdb_test_multiple "shell ls msymbols_output2" "maint print msymbols" {
> -	    -re "msymbols_output2\r\n$gdb_prompt $" {
> -		gdb_test "shell grep factorial msymbols_output2" \
> -		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> -		    "maint print msymbols, relative pathname"
> -		gdb_test "shell rm -f msymbols_output2" ".*"
> -	    }
> -	}
> -    }
> -}
> +gdb_test_no_output "maint print msymbols -objfile ${testfile} $msymbols_output"\
> +    "print msymbols to file, with relative path"
> +gdb_test "shell grep factorial $msymbols_output" \
> +    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
> +    "maint print msymbols, relative pathname"
> +gdb_test "shell rm -f msymbols_output" ".*" "remove relative path msymbols"
> +
>  gdb_test "cd ${mydir}" \
>      "Working directory [string_to_regexp ${mydir}]\..*" \
>      "cd to mydir"
> @@ -365,31 +328,12 @@ set test_list [list \
>  		   "maint print symbols -pc" \
>  		   "maint print symbols -pc main $symbols_output"]
>  foreach { test_name command } $test_list {
> -    send_gdb "$command\n"
> -    gdb_expect {
> -	-re "^maint print symbols \[^\n\]*\r\n$gdb_prompt $" {
> -	    send_gdb "shell ls $symbols_output\n"
> -	    gdb_expect {
> -		-re "$symbols_output_re\r\n$gdb_prompt $" {
> -		    # See comments for `maint print psymbols'.
> -		    send_gdb "shell grep 'main(.*block' $symbols_output\n"
> -		    gdb_expect {
> -			-re "int main\\(int, char \\*\\*, char \\*\\*\\); block.*$gdb_prompt $" {
> -			    pass "$test_name"
> -			}
> -			-re ".*$gdb_prompt $" { fail "$test_name" }
> -			timeout { fail "$test_name (timeout)" }
> -		    }
> -		    gdb_test "shell rm -f $symbols_output" ".*" \
> -			"$test_name: shell rm -f symbols_output"
> -		}
> -		-re ".*$gdb_prompt $" { fail "$test_name" }
> -		timeout { fail "$test_name (timeout)" }
> -	    }
> -	}
> -	-re ".*$gdb_prompt $" { fail "$test_name" }
> -	timeout { fail "$test_name (timeout)" }
> -    }
> +    gdb_test_no_output "$command" "$test_name generate"
> +    gdb_test "shell grep 'main(.*block' $symbols_output"\
> +	"int main\\(int, char \\*\\*, char \\*\\*\\); block.*"\
> +	"$test_name read"
> +    gdb_test "shell rm -f $symbols_output" ".*" \
> +	"$test_name: shell rm -f symbols_output"
>  }
>  
>  set msg "maint print type"
> -- 
> 2.31.1