Re: [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 30 Sep 2018, at 06:00, Simon Marchi <simark@simark.ca> wrote:
> 


> 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
>> +    }


> 
>> +
>> +    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.

These three comments are all due to me copying the proc from the almost identical
procs skip_altivec_tests, skip_vsx_tests, skip_tsx_tests, skip_btrace_tests,
skip_btrace_pt_tests. It might be worth me opening another quick patch to fix
those up too?

Probably best to keep the restart in to ensure it works on bare metal too.


Fixed all the nits in this patch as requested and pushed.


Thanks,
Alan.