Re: [PATCH][gdb/testsuite] Fix gdb.base/style.exp with -m32

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

* Tom de Vries <tdevries@suse.de> [2021-01-13 01:09:30 +0100]:

> Hi,
> 
> When running test-case gdb.base/style.exp with target board unix/-m32, we run
> into (stripped styling from output, shortened file name):
> ...
> (gdb) frame
>     argv=0xffffc714) at src/gdb/testsuite/gdb.base/style.c:45
> 45        return some_called_function (); /* break here */
> (gdb) FAIL: gdb.base/style.exp: frame when width=20
> ...
> while with native we have instead:
> ...
> (gdb) frame
>     argv=0x7fffffffd478)
>     at src/gdb/testsuite/gdb.base/style.c:45
> 45        return some_called_function (); /* break here */
> (gdb) PASS: gdb.base/style.exp: frame when width=20
> ...
> 
> The problem is that due to argv having a different length for -m32, we get a
> different layout, and the test-case doesn't accommodate for that.
> 
> Fix this by first printing the frame without styling and checking the layout,
> and then printing the frame with styling and verifying that the layout is the
> same.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Fix gdb.base/style.exp with -m32
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-01-13  Tom de Vries  <tdevries@suse.de>
> 
> 	PR testsuite/24590
> 	* gdb.base/style.exp: Handle shorter argv in frame command output.
> 
> ---
>  gdb/testsuite/gdb.base/style.exp | 66 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 60 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index cc143b43292..848d4d337d4 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -70,21 +70,75 @@ save_vars { env(TERM) } {
>      # Regression test for a bug where line-wrapping would occur at the
>      # wrong spot with styling.  There were different bugs at different
>      # widths, so try two.
> -    foreach width {20 30} {
> -	gdb_test_no_output "set width $width"
> +    foreach_with_prefix width {20 30} {
> +
>  	# There was also a bug where the styling could be wrong in the
>  	# line listing; this is why the words from the source code are
>  	# spelled out in the final result line of the test.
> -	gdb_test "frame" \
> +	set re1 \
> +	    [multi_line \
> +		 "#0 *main.*argc.*" \
> +		 ".*argv.*" \
> +		 ".* at .*style\\.c.*" \
> +		 "\[0-9\]+.*return.* break here .*"]
> +	set re1_styled \
>  	    [multi_line \
>  		 "#0 *$main_expr.*$arg_expr.*" \
>  		 ".*$arg_expr.*" \
>  		 ".* at .*$file_expr.*" \
> -		 "\[0-9\]+.*return.* break here .*"
> -	    ] \
> -	    "frame when width=$width"
> +		 "\[0-9\]+.*return.* break here .*"]
> +	set re2 \
> +	    [multi_line \
> +		 "#0 *main.*argc.*" \
> +		 ".*argv.* at .*style\\.c.*" \
> +		 "\[0-9\]+.*return.* break here .*"]
> +	set re2_styled \
> +	    [multi_line \
> +		 "#0 *$main_expr.*$arg_expr.*" \
> +		 ".*$arg_expr.* at .*$file_expr.*" \
> +		 "\[0-9\]+.*return.* break here .*"]
> +
> +	set re_styled ""
> +	with_test_prefix "style=off" {
> +	    # We temporarily set width to 0 to be able to use
> +	    # gdb_test_no_output for "set style enabled on".
> +	    gdb_test_no_output "set width 0"
> +	    gdb_test_no_output "set style enabled off"
> +	    gdb_test_no_output "set width $width"
> +
> +	    # Depending on the width and the length of argv, there may be
> +	    # a different layout.
> +	    gdb_test_multiple "frame" "" {
> +		-re -wrap $re1 {
> +		    set re_styled $re1_styled
> +		    pass $gdb_test_name
> +		}
> +		-re -wrap $re2 {
> +		    set re_styled $re2_styled
> +		    pass $gdb_test_name
> +		}
> +	    }
> +	}
> +	if { $re_styled == "" } {
> +	    continue
> +	}

All of the above is really just a way to select between two different
output patterns, the choice of which is based on the length of argv.

So, could we not just skip the complexity, and instead do:

    set argv_value [get_hexadecimal_valueof "argv" "??"]
    if { [string length $argv_value] > 12 } {
       # Choose the first pattern...
    } else {
       # Choose the second pattern...
    }
    foreach_with_prefix width {20 30} {
      # Run the tests using the selected pattern...
    }

Seems like this might be a simpler solution?

Thanks,
Andrew


> +
> +	with_test_prefix "style=on" {
> +	    # We temporarily set width to 0 to be able to use
> +	    # gdb_test_no_output for "set style enabled on".
> +	    gdb_test_no_output "set width 0"
> +	    gdb_test_no_output "set style enabled on"
> +	    gdb_test_no_output "set width $width"
> +
> +	    # Check that the frame has the same layout with "style enabled on"
> +	    # as with "style enabled off".
> +	    gdb_test "frame" $re_styled
> +	}
>      }
>  
> +    # Reset width back to 0.
> +    gdb_test_no_output "set width 0"
> +
>      if {$test_macros} {
>  	set macro_line [gdb_get_line_number "\#define SOME_MACRO"]
>  	gdb_test "info macro SOME_MACRO" \