Re: [PATCH 1/4] gdb/aarch64: record/replay support for LRCPC3

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello again,

It turns out I do have a couple more comments on this.

<Ezra.Sitorus@arm.com> writes:

> +proc test_single_asm {name diff_reg diff_var diff_mem same_reg same_var same_mem} {
> +    global decimal hex
> +
> +    set before_seq [gdb_get_line_number "Before ${name}"]
> +    set after_seq [gdb_get_line_number "After ${name}"]
> +
> +    set insn [lindex [split $name "-"] 0]
> +
> +    gdb_test "break $before_seq" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-lrcpc3.c, line $decimal\\." \
> +	"$name: break before instruction sequence"
> +    gdb_continue_to_breakpoint "$name: about to execute instruction sequence" \
> +	[multi_line ".*/aarch64-lrcpc3.c:$decimal" \
> +	    "$decimal\[ \t\]+__asm__ volatile \\(\"${insn} \[^\r\n\]+\""]
> +
> +    # Depending on the compiler, the line number information may put GDB a few
> +    # instructions before the beginning of the asm statement.
> +    arrive_at_instruction $insn
> +    # Add a breakpoint that we're sure is at the prologue instruction.
> +    gdb_test "break *\$pc" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-lrcpc3.c, line $decimal\\." \
> +	"$name: break at prologue instruction"
> +
> +    # Record the initial memory and register values.
> +    set regs [concat $diff_reg $same_reg]
> +    set vars [concat $diff_var $same_var]
> +    set mem [concat $diff_mem $same_mem]
> +    foreach r $regs {
> +	set ${r}_initial [capture_command_output "info register $r" ""]
> +    }
> +    foreach v $vars {
> +	set ${v}_initial [get_valueof "/x" "$v" "unable to read $v" \
> +	    "$name: get $v initial value"]
> +    }
> +    foreach m $mem {
> +	set ${m}_initial [capture_command_output "x/x $m" ""]
> +    }
> +
> +    gdb_test "break $after_seq" \
> +	"Breakpoint $decimal at $hex: file .*/aarch64-lrcpc3.c, line $decimal\\." \
> +	"$name: break after instruction sequence"
> +    gdb_continue_to_breakpoint "$name: executed instruction sequence" \
> +	[multi_line ".*/aarch64-lrcpc3.c:$decimal" ".*"]
> +
> +    # Record the new memory and register values.
> +    foreach r $regs {
> +	set ${r}_new [capture_command_output "info register $r" ""]
> +    }
> +    foreach v $vars {
> +	set ${v}_new [get_valueof "/x" "$v" "unable to read $v" \
> +	    "$name: get $v new value"]
> +    }
> +    foreach m $mem {
> +	set ${m}_new [capture_command_output "x/x $m" ""]
> +    }
> +
> +    # Execute in reverse to before the instruction sequence.
> +    gdb_test_no_output "set exec-direction reverse"

This message will be the same for all calls of test_single_arm, causing
DUPLICATE errors.

> +    gdb_continue_to_breakpoint "reversed execution of instruction sequence" \
> +	[multi_line ".*/aarch64-lrcpc3.c:$decimal" \
> +	    "$decimal\[ \t\]+__asm__ volatile \\(\"${insn} \[^\r\n\]+\""]

Same here.

> +    # Record the final memory and register values.
> +    foreach r $regs {
> +	set ${r}_final [capture_command_output "info register $r" ""]
> +    }
> +    foreach v $vars {
> +	set ${v}_final [get_valueof "/x" "$v" "unable to read $v" \
> +	    "$name: get $v final value"]
> +    }
> +    foreach m $mem {
> +	set ${m}_final [capture_command_output "x/x $m" ""]
> +    }
> +
> +    foreach v [concat $same_reg $same_var $same_mem] {
> +	gdb_assert ![string compare [set ${v}_initial] [set ${v}_new]] \
> +	    "$name: check $v initial value versus $v new value"
> +	gdb_assert ![string compare [set ${v}_initial] [set ${v}_final]] \
> +	    "$name: check $v initial value versus $v final value"
> +    }
> +
> +    foreach v [concat $diff_reg $diff_var $diff_mem] {
> +	gdb_assert [string compare [set ${v}_initial] [set ${v}_new]] \
> +	    "$name: check $v initial value versus $v new value"
> +	gdb_assert ![string compare [set ${v}_initial] [set ${v}_final]] \
> +	    "$name: check $v initial value versus $v final value"
> +    }
> +
> +    # Restore forward execution and go to end of recording.
> +    gdb_test_no_output "set exec-direction forward"
> +    gdb_test "record goto end" \
> +	[multi_line \
> +	    "Go forward to insn number $decimal" \
> +	    "#0  main \\(\\) at .*/aarch64-lrcpc3.c:$decimal" \
> +	    ".*"]

The two tests above will also cause DUPLICATE errors.

Instead of manually adding $name to test names to differentiate between
calls of test_single_asm, I suggest wrapping the whole body of this
function in with_test_prefix $name { ... }. This will make sure all of
them are covered.

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 28709004570..c6afc556548 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5287,6 +5287,69 @@ gdb_caching_proc allow_aarch64_fpmr_tests {} {
>      return $allow_fpmr_tests
>  }
>  
> +# Run a test on the target to see if it supports AArch64 LRCPC3 (Load-Acquire
> +# RCpc instructions) extensions.  Return 1 if so, 0 if it does not.  Note this
> +# causes a restart of GDB.
> +
> +gdb_caching_proc allow_aarch64_lrcpc3_tests {} {
> +    global srcdir subdir gdb_prompt inferior_exited_re
> +
> +    set me "allow_aarch64_lrcpc3_tests"
> +
> +    if { ![is_aarch64_target] } {
> +	return 0
> +    }
> +
> +    # Take the opportunity to check whether the toolchain knows about LRCPC3.
> +    set compile_flags "{additional_flags=-march=armv8-a+rcpc3}"
> +
> +    # Compile a program that tests the LRCPC3 feature.
> +    set src {
> +    #include <stdint.h>
> +
> +    int main() {
> +	  uint64_t a, b = 0;
> +	  uint64_t *ptr;
> +	  *ptr = 0xdeadbeef;

I don't understand what the line above is intended to do. It just
crashes the test program and causes the gdb_expect below to take the
default case, before the ldiapp instruction has a chance to be executed:

(gdb) run 
Starting program: /home/bauermann/.cache/builds/gdb-native/gdb/testsuite/temp/1360/allow_aarch64_lrcpc3_tests.x 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/aarch64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x0000aaaaaaaa077c in main () at /home/bauermann/.cache/builds/gdb-native/gdb/testsuite/temp/1360/allow_aarch64_lrcpc3_tests.c:7
warning: 7	/home/bauermann/.cache/builds/gdb-native/gdb/testsuite/temp/1360/allow_aarch64_lrcpc3_tests.c: No such file or directory
(gdb) WARNING: 
allow_aarch64_lrcpc3_tests: default case taken
gdb_caching_proc allow_aarch64_lrcpc3_tests caused gdb_exit to be called
  gdb_caching_proc allow_aarch64_lrcpc3_tests marked as called
  gdb_caching_proc get_compiler_info_1 marked as called
  gdb_caching_proc is_aarch32_target marked as called
  gdb_caching_proc universal_compile_options_c marked as called
UNSUPPORTED: gdb.reverse/aarch64-lrcpc3.exp: require failed: allow_aarch64_lrcpc3_tests

If I make ptr point to a new variable c, then I get the expected SIGILL
when running in a machine which doesn't support LRCPC3.

> +	  __asm__ volatile ("ldiapp %0, %1, [%2]\n"
> +	    : "=r"(a), "=r"(b)
> +	    : "r"(ptr)
> +	    : "memory");
> +
> +	  return 0;
> +	}
> +    }
> +
> +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
> +	return 0
> +    }
> +
> +    # Compilation succeeded so now run it via gdb.
> +    clean_restart
> +    gdb_load $obj
> +    gdb_run_cmd
> +    gdb_expect {
> +	-re ".*Illegal instruction.*${gdb_prompt} $" {
> +	    verbose -log "\n$me lrcpc3 support not detected"
> +	    set allow_lrcpc3_tests 0
> +	}
> +	-re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> +	    verbose -log "\n$me: lrcpc3 support detected"
> +	    set allow_lrcpc3_tests 1
> +	}
> +	default {
> +	  warning "\n$me: default case taken"
> +	    set allow_lrcpc3_tests 0
> +	}
> +    }
> +    gdb_exit
> +    remote_file build delete $obj
> +
> +    verbose "$me:  returning $allow_lrcpc3_tests" 2
> +    return $allow_lrcpc3_tests
> +}

-- 
Thiago