Re: [RFC PATCH 5/5] gdb/aarch64: Tests for fpmr

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

<Ezra.Sitorus@arm.com> writes:

> From: Ezra Sitorus <ezra.sitorus@arm.com>
>
> Add tests for FPMR support in gdb/gdbserver. These tests check
> availability of FPMR, reading/writing to FPMR and core file generation.
>
> For these tests, I have used shrinkwrap for FPMR emulation. I am not
> sure on how to do these tests properly as there are some things I'm not
> sure about:
>
> * Which board file do I use to test this? I've resorted to adapting
>   remote-stdio-gdbserver.exp to use tcp when starting gdbserver
>   remotely.
> * Do I need to run the whole regression test? This is quite slow, and
>   comes up with a lot of failures.
> * Since gdbserver doesn't support core files, I've had to adapt
>   core_find so that the core file is generated on target (where
>   gdbserver runs), but then it's copied over to host (where gdb runs) to
>   do the actual test. You can find this on aarch64-fpmr-core.exp
>   (remote_core_find). Please let me know if there's a better way of
>   doing this, or how I can improve the way I've done it.
> * Another issue with the core file test is that sometimes it says all
>   the tests pass, but there's 1 unexpected core file or that core files
>   can't be generated and I need to do ulimit -c unlimited, despite
>   having done that already.

If there's an unexpected core file, then either GDB or gdbserver
crashed.

Apart from the things Luis mentioned, another one that interferes with
core file tests is a system service that collects core files. On Ubuntu,
I disable the Apport systemd service.

> ---
>  gdb/testsuite/gdb.arch/aarch64-fpmr-core.c    | 109 +++++++++++++
>  gdb/testsuite/gdb.arch/aarch64-fpmr-core.exp  | 143 ++++++++++++++++++
>  .../gdb.arch/aarch64-fpmr-sighandler.c        | 134 ++++++++++++++++
>  .../gdb.arch/aarch64-fpmr-sighandler.exp      |  75 +++++++++
>  gdb/testsuite/gdb.arch/aarch64-fpmr.c         | 117 ++++++++++++++
>  gdb/testsuite/gdb.arch/aarch64-fpmr.exp       | 116 ++++++++++++++
>  gdb/testsuite/lib/gdb.exp                     |  49 ++++++
>  7 files changed, 743 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr-core.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr-core.exp
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr-sighandler.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr-sighandler.exp
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-fpmr.exp
>
> diff --git a/gdb/testsuite/gdb.arch/aarch64-fpmr-core.c b/gdb/testsuite/gdb.arch/aarch64-fpmr-core.c
> new file mode 100644
> index 00000000000..95c6f302ad7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-fpmr-core.c
> @@ -0,0 +1,109 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2008-2025 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/>.  */
> +
> +#include <stdint.h>
> +
> +uint64_t crash_address = 0;
> +
> +enum FPM_FORMAT
> +{
> +  E5M2,
> +  E4M3,
> +};
> +
> +enum FPM_OVERFLOW
> +{
> +  INFNAN,
> +  SATURATE,
> +};
> +
> +void
> +set_fpmr (uint64_t value)
> +{
> +  register uint64_t x0_val asm ("x0") = value;
> +  __asm__ volatile (".inst	0xd51b4440" : : );

It would be good to have a comment here mentioning to which instruction
the value above decodes to.

> diff --git a/gdb/testsuite/gdb.arch/aarch64-fpmr-core.exp b/gdb/testsuite/gdb.arch/aarch64-fpmr-core.exp
> new file mode 100644
> index 00000000000..52acfd7291b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-fpmr-core.exp
> @@ -0,0 +1,143 @@
> +# Copyright (C) 2018-2025 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.
> +
> +# Test generating and reading a core file with FPMR.
> +
> +proc check_fpmr_core_file { core_filename } {

Ideally, procedures should have documentation comments.

> +    # Load the core file.
> +    if [gdb_test "core $core_filename" \
> +    [multi_line \
> +	    "Core was generated by .*" \
> +	    "Program terminated with signal SIGSEGV, Segmentation fault\\." \
> +	    "#0  ${::hex} in main \\(.*\\) at .*" \
> +	    ".* \\*\\(\\(uint64_t \\*\\) crash_address\\) = 0xDEAD.*"] \
> +	    "load core file"] {
> +	untested "failed to generate core file"
> +	return -1
> +	}
> +
> +    # Check the value of FPMR in the core file.
> +    gdb_test "print/x \$fpmr" " = 0x3fff7fc049" \
> +	     "fpmr contents from core file"
> +}
> +
> +proc remote_core_find {binfile {deletefiles {}} {arg ""}} {
> +    global objdir subdir REMOTE_TMPDIR
> +
> +    set target_binfile [gdbserver_download_current_prog]
> +
> +    set destcore "$target_binfile.core"
> +    remote_exec target delete $destcore
> +
> +    set found 0
> +    set coredir "${REMOTE_TMPDIR}/coredir.[getpid]"
> +    remote_exec target "mkdir $coredir"
> +    remote_exec target "(cd ${coredir}; ulimit -c unlimited; ${target_binfile} ${arg}; true) >/dev/null 2>&1"
> +    set binfile_basename [file tail $target_binfile]
> +    foreach i [list \
> +		   ${coredir}/core \
> +		   ${coredir}/core.coremaker.c \
> +		   ${coredir}/${binfile_basename}.core \
> +		   ${coredir}/${binfile_basename}.exe.core] {
> +	if [remote_file target exists $i] {
> +	    remote_exec target "mv $i $destcore"
> +	    set found 1
> +	}
> +    }
> +
> +    if { $found == 0 } {
> +	set names [glob -nocomplain -directory $coredir core.*]
> +	if {[llength $names] == 1} {
> +	    set corefile [file join $coredir [lindex $names 0]]
> +	    remote_exec target "mv $corefile $destcore"
> +	    set found 1
> +	}
> +    }
> +
> +    foreach deletefile $deletefiles {
> +	remote_file target delete [file join $coredir $deletefile]
> +    }
> +    remote_exec target "rmdir $coredir"
> +
> +    if { $found == 0  } {
> +	warning "can't generate a core file - core tests suppressed - check ulimit -c"
> +	return ""
> +    }
> +    set native_corefile "${binfile}.core"
> +    remote_download host [remote_upload target $destcore] $native_corefile
> +
> +    return $native_corefile
> +}
> +
> +require is_aarch64_target
> +require allow_aarch64_fpmr_tests
> +
> +standard_testfile
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +set binfile [standard_output_file ${testfile}]
> +
> +if ![runto_main] {
> +    untested "could not run to main"
> +    return -1
> +}

Nowadays we don't return a value from the top level of the test, so just
"return" is enough.

> +
> +set crash_breakpoint "crash point"
> +gdb_breakpoint [gdb_get_line_number $crash_breakpoint]
> +gdb_continue_to_breakpoint $crash_breakpoint
> +
> +gdb_test "print/x \$fpmr" " = 0x3fff7fc049" \
> +	     "fpmr contents from core file"
> +
> +gdb_test "continue" \
> +[multi_line \
> +	"Program received signal SIGSEGV, Segmentation fault\\." \
> +	"${::hex} in main \\(\\).* at .*" \
> +	".* \\*\\(\\(uint64_t \\*\\) crash_address\\) = 0xDEAD.*"] \
> +	"run to crash"
> +
> +# Generate the gcore core file.
> +set gcore_filename [standard_output_file "${testfile}.gcore"]
> +set gcore_generated [gdb_gcore_cmd "$gcore_filename" "generate gcore file"]
> +
> +# Generate a native core file.
> +set core_filename [remote_core_find ${binfile}]
> +set core_generated [expr {$core_filename != ""}]
> +
> +# At this point we have a couple core files, the gcore one generated by GDB
> +# and the native one generated by the Linux Kernel.  Make sure GDB can read
> +# both correctly.
> +
> +if {$gcore_generated} {
> +	clean_restart ${binfile}

I think that as of a few days ago this doesn't work anymore.
Now we use:

  clean_restart
  gdb_load $binfile

> +	with_test_prefix "gcore corefile" {
> +		check_fpmr_core_file $gcore_filename
> +	}
> +} else {
> +	fail "gcore corefile not generated"
> +}
> +
> +if {$core_generated} {
> +	clean_restart ${binfile}
> +	with_test_prefix "native corefile" {
> +		check_fpmr_core_file $core_filename
> +	}
> +} else {
> +	untested "native corefile not generated"
> +}

<snip>

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index ab4506a4455..a6961fb8719 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5266,6 +5266,55 @@ proc aarch64_supports_sme_svl { length } {
>      return 1
>  }
>  
> +# Run a test on the target to see if it supports AArch64 FPMR hardware.
> +# Return 1 if so, 0 if it does not.  Note this causes a restart of GDB.
> +
> +gdb_caching_proc allow_aarch64_fpmr_tests {} {
> +    global srcdir subdir gdb_prompt inferior_exited_re
> +
> +    set me "allow_aarch64_fpmr_tests"
> +
> +    if { ![is_aarch64_target]} {
> +	return 0
> +    }
> +
> +    set compile_flags "{additional_flags=-march=armv8-a}"
> +
> +    # Compile a test program that writes to FPMR.
> +    set src {
> +	int main() {
> +	    asm volatile (".inst	0xd51b4440");

It would be good to have a comment mentioning which instruction the
value above decodes to.

Another option is to make the test program check AT_HWCAP instead of
trying to run an instruction. This is what proc allow_aarch64_gcs_tests
does. The advantage is consistency, since if the target machine is
running a kernel that doesn't report FPMR in AT_HWCAP, the test will
report that it is supported, but GDB will not recognize the feature.

Or would this program cause SIGILL if it's running on a processor
supporting this feature but with a kernel that doesn't? I don't know
actually.

> +	    return 0;
> +	}
> +    }
> +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
> +        return 0
> +    }
> +
> +    # Compilation succeeded so now run it via gdb.
> +    clean_restart $obj
> +    gdb_run_cmd

There was a recent patch series standardising this sequence to:

  clean_restart
  gdb_load $obj
  gdb_run_cmd

to avoid passing absolute filenames to clean_restart.

> +    gdb_expect {
> +        -re ".*Illegal instruction.*${gdb_prompt} $" {
> +            verbose -log "\n$me fpmr not detected"
> +            set allow_fpmr_tests 0
> +        }
> +        -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> +            verbose -log "\n$me: fpmr hardware detected"
> +            set allow_fpmr_tests 1
> +        }
> +        default {
> +          warning "\n$me: default case taken"
> +            set allow_fpmr_tests 0
> +        }
> +    }
> +    gdb_exit
> +    remote_file build delete $obj
> +
> +    verbose "$me:  returning $allow_fpmr_tests" 2
> +    return $allow_fpmr_tests
> +}
> +
>  # Run a test on the target to see if it supports AArch64 MOPS (Memory
>  # Operations) extensions.  Return 1 if so, 0 if it does not.  Note this
>  # causes a restart of GDB.

-- 
Thiago