This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test
On 2018-09-17 8:53 a.m., Alan Hayward wrote:
> Add function to detect if aarch64 SVE is available.
>
> Add Aarch64 test to check register values of a previous frame
> can be shown correctly across a signal.
Just some nits here and there:
- The .c file should have a copyright header.
- Make sure test names start with a lowercase letter.
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index f32abfedd5..524cf623b2 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2824,6 +2824,57 @@ gdb_caching_proc skip_btrace_pt_tests {
> return $skip_btrace_tests
> }
>
> +gdb_caching_proc skip_aarch64_sve_tests {
Can you add a short description of this proc, especially about the side-effects
and the return value?
> + global srcdir subdir gdb_prompt inferior_exited_re
> +
> + set me "skip_aarch64_sve_tests"
> +
> + if { ![is_aarch64_target]} {
> + return 0
> + }
IIUC, you return 1 if SVE tests should be skipped. If the target is not aarch64,
shouldn't we skip the SVE tests?
> +
> + set compile_flags "{additional_flags=-march=armv8-a+sve}"
> +
> + # Compile a test program containing SVE instructions.
> + set src {
> + int main() {
> + asm volatile ("ptrue p0.b");
> + return 0;
> + }
> + }
> + if {![gdb_simple_compile $me $src executable $compile_flags]} {
> + return 1
> + }
> +
> + # Compilation succeeded so now run it via gdb.
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load "$obj"
This sequence can probably be replaced with clean_restart.
> + gdb_run_cmd
> + gdb_expect {
> + -re ".*Illegal instruction.*${gdb_prompt} $" {
> + verbose -log "\n$me sve hardware not detected"
> + set skip_sve_tests 1
> + }
> + -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> + verbose -log "\n$me: sve hardware detected"
> + set skip_sve_tests 0
> + }
> + default {
> + warning "\n$me: default case taken"
> + set skip_sve_tests 1
> + }
> + }
> + gdb_exit
Instead of executing the test case in gdb and having to mess up
the current debug run, would it be possible to just run the executable
and check the exit code? You could use "remote_Exec target" to execute
the program, and the exit code should be != 0 if the program was terminated
by a signal (SIGILL). Of course, that only works with Linux (and perhaps
FreeBSD), so if you need this to work with bare-metal or other AArch64 targets,
it won't do.
If the side-effect of restarting GDB is clearly stated in the function doc, then
it's fine like this.
LGTM with those fixed.
Simon