diff mbox

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

Message ID 20180917125314.71795-4-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Sept. 17, 2018, 12:53 p.m. UTC 
  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.

gdb/testsuite/ChangeLog:

2018-09-14  Alan Hayward  <alan.hayward@arm.com>

	* gdb.arch/aarch64-sighandler-regs.c: New test.
	* gdb.arch/aarch64-sighandler-regs.exp: New file.
	* lib/gdb.exp (skip_aarch64_sve_tests): New proc.
---
 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c   | 170 +++++++++++++++++++++
 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp | 150 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                          |  51 +++++++
 3 files changed, 371 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp

Comments

Simon Marchi Sept. 30, 2018, 5 a.m. UTC | #1 
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
Alan Hayward Oct. 1, 2018, 2:26 p.m. UTC | #2 
> 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.
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
new file mode 100644
index 0000000000..5f3674ae7b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
@@ -0,0 +1,170 @@ 
+#include <signal.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define OVERWRITE_GP_REGS \
+		    "ldr x1, [x0]\n\t" \
+		    "ldr x2, [x0]\n\t" \
+		    "ldr x3, [x0]\n\t" \
+		    "ldr x4, [x0]\n\t" \
+		    "ldr x5, [x0]\n\t" \
+		    "ldr x6, [x0]\n\t" \
+		    "ldr x7, [x0]\n\t" \
+		    "ldr x8, [x0]\n\t" \
+		    "ldr x9, [x0]\n\t" \
+		    "ldr x10, [x0]\n\t" \
+		    "ldr x11, [x0]\n\t" \
+		    "ldr x12, [x0]\n\t" \
+		    "ldr x13, [x0]\n\t" \
+		    "ldr x14, [x0]\n\t" \
+		    "ldr x15, [x0]\n\t" \
+		    "ldr x16, [x0]\n\t" \
+		    "ldr x17, [x0]\n\t" \
+		    "ldr x18, [x0]\n\t" \
+		    "ldr x19, [x0]\n\t" \
+		    "ldr x20, [x0]\n\t" \
+		    "ldr x21, [x0]\n\t" \
+		    "ldr x22, [x0]\n\t" \
+		    "ldr x23, [x0]\n\t" \
+		    "ldr x24, [x0]\n\t" \
+		    "ldr x25, [x0]\n\t" \
+		    "ldr x26, [x0]\n\t" \
+		    "ldr x27, [x0]\n\t" \
+		    "ldr x28, [x0]\n\t"
+
+#ifdef SVE
+#define OVERWRITE_FP_REGS \
+		    "ptrue p3.s\n\t" \
+		    "ld1w z0.s, p3/z, [x0]\n\t" \
+		    "ld1w z1.s, p3/z, [x0]\n\t" \
+		    "ld1w z2.s, p3/z, [x0]\n\t" \
+		    "ld1w z3.s, p3/z, [x0]\n\t" \
+		    "ld1w z4.s, p3/z, [x0]\n\t" \
+		    "ld1w z5.s, p3/z, [x0]\n\t" \
+		    "ld1w z6.s, p3/z, [x0]\n\t" \
+		    "ld1w z7.s, p3/z, [x0]\n\t" \
+		    "ld1w z8.s, p3/z, [x0]\n\t" \
+		    "ld1w z9.s, p3/z, [x0]\n\t" \
+		    "ld1w z10.s, p3/z, [x0]\n\t" \
+		    "ld1w z11.s, p3/z, [x0]\n\t" \
+		    "ld1w z12.s, p3/z, [x0]\n\t" \
+		    "ld1w z13.s, p3/z, [x0]\n\t" \
+		    "ld1w z14.s, p3/z, [x0]\n\t" \
+		    "ld1w z15.s, p3/z, [x0]\n\t" \
+		    "ld1w z16.s, p3/z, [x0]\n\t" \
+		    "ld1w z17.s, p3/z, [x0]\n\t" \
+		    "ld1w z18.s, p3/z, [x0]\n\t" \
+		    "ld1w z19.s, p3/z, [x0]\n\t" \
+		    "ld1w z20.s, p3/z, [x0]\n\t" \
+		    "ld1w z21.s, p3/z, [x0]\n\t" \
+		    "ld1w z22.s, p3/z, [x0]\n\t" \
+		    "ld1w z23.s, p3/z, [x0]\n\t" \
+		    "ld1w z24.s, p3/z, [x0]\n\t" \
+		    "ld1w z25.s, p3/z, [x0]\n\t" \
+		    "ld1w z26.s, p3/z, [x0]\n\t" \
+		    "ld1w z27.s, p3/z, [x0]\n\t" \
+		    "ld1w z28.s, p3/z, [x0]\n\t" \
+		    "ld1w z29.s, p3/z, [x0]\n\t" \
+		    "ld1w z30.s, p3/z, [x0]\n\t" \
+		    "ld1w z31.s, p3/z, [x0]\n\t"
+#else
+#define OVERWRITE_FP_REGS \
+		    "ldr q0, [x0]\n\t" \
+		    "ldr q1, [x0]\n\t" \
+		    "ldr q2, [x0]\n\t" \
+		    "ldr q3, [x0]\n\t" \
+		    "ldr q4, [x0]\n\t" \
+		    "ldr q5, [x0]\n\t" \
+		    "ldr q6, [x0]\n\t" \
+		    "ldr q7, [x0]\n\t" \
+		    "ldr q8, [x0]\n\t" \
+		    "ldr q9, [x0]\n\t" \
+		    "ldr q10, [x0]\n\t" \
+		    "ldr q11, [x0]\n\t" \
+		    "ldr q12, [x0]\n\t" \
+		    "ldr q13, [x0]\n\t" \
+		    "ldr q14, [x0]\n\t" \
+		    "ldr q15, [x0]\n\t" \
+		    "ldr q16, [x0]\n\t" \
+		    "ldr q17, [x0]\n\t" \
+		    "ldr q18, [x0]\n\t" \
+		    "ldr q19, [x0]\n\t" \
+		    "ldr q20, [x0]\n\t" \
+		    "ldr q21, [x0]\n\t" \
+		    "ldr q22, [x0]\n\t" \
+		    "ldr q23, [x0]\n\t" \
+		    "ldr q24, [x0]\n\t" \
+		    "ldr q25, [x0]\n\t" \
+		    "ldr q26, [x0]\n\t" \
+		    "ldr q27, [x0]\n\t" \
+		    "ldr q28, [x0]\n\t" \
+		    "ldr q29, [x0]\n\t" \
+		    "ldr q30, [x0]\n\t" \
+		    "ldr q31, [x0]\n\t"
+#endif
+
+#ifdef SVE
+#define OVERWRITE_P_REGS(pattern) \
+		    "ptrue p0.s, " #pattern "\n\t" \
+		    "ptrue p1.s, " #pattern "\n\t" \
+		    "ptrue p2.s, " #pattern "\n\t" \
+		    "ptrue p3.s, " #pattern "\n\t" \
+		    "ptrue p4.s, " #pattern "\n\t" \
+		    "ptrue p5.s, " #pattern "\n\t" \
+		    "ptrue p6.s, " #pattern "\n\t" \
+		    "ptrue p7.s, " #pattern "\n\t" \
+		    "ptrue p8.s, " #pattern "\n\t" \
+		    "ptrue p9.s, " #pattern "\n\t" \
+		    "ptrue p10.s, " #pattern "\n\t" \
+		    "ptrue p11.s, " #pattern "\n\t" \
+		    "ptrue p12.s, " #pattern "\n\t" \
+		    "ptrue p13.s, " #pattern "\n\t" \
+		    "ptrue p14.s, " #pattern "\n\t" \
+		    "ptrue p15.s, " #pattern "\n\t"
+#else
+#define OVERWRITE_P_REGS(pattern)
+#endif
+
+
+void
+handler (int sig)
+{
+  char buf_handler[] = {0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
+			0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f};
+
+  __asm __volatile ("mov x0, %0\n\t" \
+		    OVERWRITE_GP_REGS \
+		    OVERWRITE_FP_REGS \
+		    OVERWRITE_P_REGS (MUL3) \
+		    : : "r" (buf_handler));
+
+  exit (0);
+}
+
+
+
+int
+main ()
+{
+  /* Ensure all the signals aren't blocked.  */
+  sigset_t newset;
+  sigemptyset (&newset);
+  sigprocmask (SIG_SETMASK, &newset, NULL);
+
+  signal (SIGILL, handler);
+
+  char buf_main[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+		     0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
+
+  /* 0x06000000 : Cause an illegal instruction.  Value undefined as per ARM
+     Architecture Reference Manual ARMv8, Section C4.1.  */
+
+  __asm __volatile ("mov x0, %0\n\t" \
+		    OVERWRITE_GP_REGS \
+		    OVERWRITE_FP_REGS \
+		    OVERWRITE_P_REGS (VL1) \
+		    ".inst 0x06000000"
+		    : : "r" (buf_main));
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
new file mode 100644
index 0000000000..3fe6a28929
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
@@ -0,0 +1,150 @@ 
+# Copyright 2008-2018 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# This file is part of the gdb testsuite.
+
+
+if {![is_aarch64_target]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return -1
+}
+
+set compile_flags {debug}
+
+if { [skip_aarch64_sve_tests] } {
+    unsupported "target does not support SVE"
+    set sve_hw 0
+} else {
+    set sve_hw 1
+    lappend compile_flags "additional_flags=-DSVE"
+}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+    return -1
+}
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+set endianness [get_endianness]
+
+if {$endianness == "little"} {
+    set reg_handler_value_128 "0x5f5e5d5c5b5a59585756555453525150"
+    set reg_handler_value_64 "0x5756555453525150"
+    set reg_handler_value_32 "0x53525150"
+    set reg_handler_value_16 "0x5150"
+    set reg_handler_value_8 "0x50"
+    set reg_main_value_128 "0x1f1e1d1c1b1a19181716151413121110"
+    set reg_main_value_64 "0x1716151413121110"
+    set reg_main_value_32 "0x13121110"
+    set reg_main_value_16 "0x1110"
+    set reg_main_value_8 "0x10"
+} else {
+    set reg_handler_value_128 "0x505152535455565758595a5b5c5d5e5f"
+    set reg_handler_value_64 "0x5051525354555657"
+    set reg_handler_value_32 "0x50515253"
+    set reg_handler_value_16 "0x5051"
+    set reg_handler_value_8 "0x50"
+    set reg_main_value_128 "0x101112131415161718191a1b1c1d1e1f"
+    set reg_main_value_64 "0x1011121314151617"
+    set reg_main_value_32 "0x10111213"
+    set reg_main_value_16 "0x1011"
+    set reg_main_value_8 "0x10"
+}
+set zreg_handler_value "\\{0x5756555453525150, .*"
+set zreg_main_value "\\{0x1716151413121110, .*"
+set preg_handler_value "\\{0x11, .*"
+set preg_main_value "\\{0x1, 0x0, .*"
+
+#Ignore x0, and x29 to x31
+set xreg_nums [list 1 2 3 4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 21 22 23 24 \
+		    25 26 27 28 ]
+set vreg_nums [list 0 1 2 3 4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 21 22 23 \
+		    24 25 26 27 28 29 30 31]
+set preg_nums [list 0 1 2 3 4 5 6 7 8 9 11 12 13 14 15]
+
+proc check_regs {regtype regnums value postfix} {
+  foreach regnum $regnums {
+    gdb_test "print /x \$$regtype$regnum$postfix" \
+      ".* = {?$value}?" \
+      "check register \$$regtype$regnum has value $value"
+  }
+}
+
+# Run until end of signal handler
+
+gdb_test "continue" \
+    "Continuing.*Program received signal SIGILL.*" \
+    "continue until signal"
+
+gdb_breakpoint [gdb_get_line_number "exit (0)"]
+gdb_continue_to_breakpoint "exit" ".*exit.*"
+
+set handlerframe [get_current_frame_number]
+set mainframe [expr $handlerframe + 2]
+
+
+# Check register values
+
+check_regs x $xreg_nums $reg_handler_value_64 ""
+check_regs v $vreg_nums $reg_handler_value_128 ".q.u"
+check_regs q $vreg_nums $reg_handler_value_128 ".u"
+check_regs d $vreg_nums $reg_handler_value_64 ".u"
+check_regs s $vreg_nums $reg_handler_value_32 ".u"
+check_regs h $vreg_nums $reg_handler_value_16 ".u"
+check_regs b $vreg_nums $reg_handler_value_8 ".u"
+if { $sve_hw } {
+  check_regs z $vreg_nums $zreg_handler_value ".d.u"
+  check_regs p $preg_nums $preg_handler_value ""
+}
+
+# Switch to the frame for main(), and check register values
+
+gdb_test "frame $mainframe" \
+      "#$mainframe.*in main ().*" \
+      "Set to main frame"
+
+check_regs x $xreg_nums $reg_main_value_64 ""
+check_regs v $vreg_nums $reg_main_value_128 ".q.u"
+check_regs q $vreg_nums $reg_main_value_128 ".u"
+check_regs d $vreg_nums $reg_main_value_64 ".u"
+check_regs s $vreg_nums $reg_main_value_32 ".u"
+check_regs h $vreg_nums $reg_main_value_16 ".u"
+check_regs b $vreg_nums $reg_main_value_8 ".u"
+if { $sve_hw } {
+  check_regs z $vreg_nums $zreg_main_value ".d.u"
+  check_regs p $preg_nums $preg_main_value ""
+}
+
+# Switch back to the signal handler frame, and check register values
+
+gdb_test "frame $handlerframe" \
+      "#$handlerframe.*handler \\\(sig=4\\\).*" \
+      "Set to signal handler frame"
+
+check_regs x $xreg_nums $reg_handler_value_64 ""
+check_regs v $vreg_nums $reg_handler_value_128 ".q.u"
+check_regs q $vreg_nums $reg_handler_value_128 ".u"
+check_regs d $vreg_nums $reg_handler_value_64 ".u"
+check_regs s $vreg_nums $reg_handler_value_32 ".u"
+check_regs h $vreg_nums $reg_handler_value_16 ".u"
+check_regs b $vreg_nums $reg_handler_value_8 ".u"
+if { $sve_hw } {
+  check_regs z $vreg_nums $zreg_handler_value ".d.u"
+  check_regs p $preg_nums $preg_handler_value ""
+}
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 {
+    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"
+    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
+    remote_file build delete $obj
+
+    verbose "$me:  returning $skip_sve_tests" 2
+    return $skip_sve_tests
+}
+
+
 # A helper that compiles a test case to see if __int128 is supported.
 proc gdb_int128_helper {lang} {
     return [gdb_can_simple_compile "i128-for-$lang" {