Re: [Patch] GDB: aarch64: Add ability to step over a BR/BLR instruction

[Pedro Alves <pedro@palves.net>]

On 7/23/20 5:48 PM, Matthew Malcomson wrote:
> +
> +# Test for displaced stepping over the BLR instruction.
> +gdb_test "run" \
> +  "Starting program.*Breakpoint $decimal.*" \
> +  "Run until BLR test start"
> +

Please don't use "run" directly.  Use one of runto, runto_main,
gdb_run_cmd instead.  See amd64-disp-step.exp for example.

If you use "run" directly, then the testcase won't run against
gdbserver.  Please make sure this passes cleanly:

 $ make check \
    RUNTESTFLAGS="--target_board=native-gdbserver" \
    TESTS="gdb.arch/aarch64-disp-stepping.exp"

> +set expected_lr [get_hexadecimal_valueof "\$pc + 4" 0]
> +gdb_test "print/x \$x0" \
> +  ".. = 0x0" \
> +  "Ensure x0 is 0 before BLR test."

The ".." at the start of the pattern are not necessary,
gdb_test will match anything that appears before your
pattern anyway.

Use lowercase, and no period at end.  Throughout.

"Ensure" is redundant, IMHO.  Every test is ensuring something.

So, I'd write:

 gdb_test "print/x \$x0" \
   " = 0x0" \
   "x0 is 0 before BLR test"

> +gdb_test "print/x \$lr == $expected_lr" \
> +  ".. = 0x1" \
> +  "Ensure LR is set to just after BLR."

Is set to ... ?

Please cat the testsuite/gdb.sum file after running
your testcase in isolation and skim it to make sure
it all makes sense.

You can also use 

 with_test_prefix "BLR" {
  ...
 }

 with_test_prefix "BR" {
  ...
 }

to group tests.

Thanks,
Pedro Alves