Re: [PATCH v2 4/4] gdb/aarch64: record/replay support for LSE128

[Guinevere Larsen <guinevere@redhat.com>]

On 5/12/26 8:17 PM, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
>
> FEAT_LSE128 introduces support for 128-bit atomic instructions. This
> patch teaches GDB to decode these instructions for recording and
> reversing.
>
> Regression tested on aarch64-none-linux-gnu on QEMU with LSE128 support.
> I observe unrelated regressions that appear to be environmental rather
> than caused by this patch.
> ---

Hi Ezra!

Thanks for addressing all the things I raised on all the patches! I'm 
happy to approve this patch!

Approved-By: Guinevere Larsen <guinevere@redhat.com>

-- 
Cheers,
Guinevere Larsen
It/she


>   gdb/aarch64-tdep.c                           |  13 ++
>   gdb/testsuite/gdb.reverse/aarch64-lse128.c   | 100 ++++++++++++++
>   gdb/testsuite/gdb.reverse/aarch64-lse128.exp | 137 +++++++++++++++++++
>   gdb/testsuite/lib/gdb.exp                    |  56 ++++++++
>   4 files changed, 306 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.reverse/aarch64-lse128.c
>   create mode 100644 gdb/testsuite/gdb.reverse/aarch64-lse128.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 5c1f8af5..0cce4072 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -6008,6 +6008,19 @@ aarch64_record_load_store (aarch64_insn_decode_record *aarch64_insn_r)
>     else if ((insn_bits24_27 & 1) == 1 && insn_bits28_29 == 1
>   	   && insn_bits10_11 == 1 && !insn_bit21)
>       return aarch64_record_memcopy_memset (aarch64_insn_r);
> +  /* Large System Extension 128 (LSE128) instructions.  */
> +  else if (vector_flag == 0 && insn_bits10_11 == 0 && insn_bit21
> +	   && size_bits == 0 && !bit (aarch64_insn_r->aarch64_insn, 29)
> +	   && bits (aarch64_insn_r->aarch64_insn, 24, 25) == 0x1)
> +    {
> +      regcache_raw_read_unsigned (aarch64_insn_r->regcache, reg_rn, &address);
> +      record_buf_mem[0] = 128 >> 3;
> +      record_buf_mem[1] = address;
> +      aarch64_insn_r->mem_rec_count = 1;
> +      record_buf[0] = reg_rt;
> +      record_buf[1] = bits (aarch64_insn_r->aarch64_insn, 16, 20);
> +      aarch64_insn_r->reg_rec_count = 2;
> +    }
>     /* Advanced SIMD load/store instructions.  */
>     else
>       return aarch64_record_asimd_load_store (aarch64_insn_r);
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-lse128.c b/gdb/testsuite/gdb.reverse/aarch64-lse128.c
> new file mode 100644
> index 00000000..0f1540e3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-lse128.c
> @@ -0,0 +1,100 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2024-2026 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 <stdalign.h>
> +#include <stdint.h>
> +#include <string.h>
> +
> +#define INITIAL_STRING "This is just some string."
> +#define BUF_SIZE sizeof (INITIAL_STRING)
> +
> +#define PREPARE_REGS(X, Y)						    \
> +  strcpy (src, INITIAL_STRING);					       \
> +  __asm__ volatile ("mov x21, %0\n" ::"r"((uint64_t *)src) : "x21");	  \
> +  __asm__ volatile ("mov x19, %0\n" ::"r"(X) : "x19");			\
> +  __asm__ volatile ("mov x20, %0\n" ::"r"(Y) : "x20");
> +
> +int
> +main (void)
> +{
> +  alignas (16) char src[BUF_SIZE];
> +
> +  uint64_t a = 0x0123456789abcdef;
> +  uint64_t b = 0xfedbca9876543210;
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldclrp.  */
> +  __asm__ volatile ("ldclrp x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldclrp.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldclrpa.  */
> +  __asm__ volatile ("ldclrpa x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldclrpa.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldclrpal.  */
> +  __asm__ volatile ("ldclrpal x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldclrpal.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldclrpl.  */
> +  __asm__ volatile ("ldclrpl x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldclrpl.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldsetp.  */
> +  __asm__ volatile ("ldsetp x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldsetp.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldsetpa.  */
> +  __asm__ volatile ("ldsetpa x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldsetpa.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldsetpal.  */
> +  __asm__ volatile ("ldsetpal x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldsetpal.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before ldsetpl.  */
> +  __asm__ volatile ("ldsetpl x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After ldsetpl.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before swpp.  */
> +  __asm__ volatile ("swpp x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After swpp.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before swppa.  */
> +  __asm__ volatile ("swppa x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After swppa.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before swppal.  */
> +  __asm__ volatile ("swppal x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After swppal.  */
> +
> +  PREPARE_REGS (a, b);
> +  /* Before swppl.  */
> +  __asm__ volatile ("swppl x19, x20, [x21]\n" : : : "x19", "x20", "memory");
> +  /* After swppl.  */
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.reverse/aarch64-lse128.exp b/gdb/testsuite/gdb.reverse/aarch64-lse128.exp
> new file mode 100644
> index 00000000..edb254bc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/aarch64-lse128.exp
> @@ -0,0 +1,137 @@
> +# Copyright 2024-2026 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/>.
> +
> +# Test instruction record for AArch64 FEAT_LSE128 instructions.
> +# Based on gdb.reverse/aarch64-mops.exp
> +#
> +# The basic flow of the record tests are:
> +#    1) Stop before executing the instructions of interest.  Record
> +#       the initial value of the registers that the instruction will
> +#       change, i.e. the destination register.
> +#    2) Execute the instructions.  Record the new value of the
> +#       registers that changed.
> +#    3) Reverse the direction of the execution and execute back to
> +#       just before the instructions of interest.  Record the final
> +#       value of the registers of interest.
> +#    4) Check that the initial and new values of the registers are
> +#       different, i.e. the instruction changed the registers as expected.
> +#    5) Check that the initial and final values of the registers are
> +#       the same, i.e. GDB record restored the registers to their
> +#       original values.
> +
> +standard_testfile
> +
> +require allow_aarch64_lse128_tests
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	[list debug additional_flags=-march=armv8-a+lse128]]} {
> +    return
> +}
> +
> +if {![runto_main]} {
> +    return
> +}
> +
> +gdb_test_no_output "record full"
> +
> +proc test_single_asm {insn} {
> +    with_test_prefix $insn {
> +	set before_seq [gdb_get_line_number "Before ${insn}"]
> +	set after_seq [gdb_get_line_number "After ${insn}"]
> +	set regs { x19 x20 x21 }
> +
> +	gdb_test "break $before_seq" \
> +	    "Breakpoint ${::decimal} at ${::hex}: file .*/aarch64-lse128.c, line ${::decimal}\\." \
> +	    "$insn: break before instruction sequence"
> +	gdb_continue_to_breakpoint "$insn: about to execute instruction sequence" \
> +	    [multi_line ".*/aarch64-lse128.c:${::decimal}" \
> +		"${::decimal}\[ \t\]+__asm__ volatile \\(\"${insn} .*\".*"]
> +
> +	# 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-lse128.c, line ${::decimal}\\." \
> +	    "$insn: break at prologue instruction"
> +
> +	# Record the initial memory and register values.
> +	foreach r $regs {
> +	    set ${r}_initial [capture_command_output "info register $r" ""]
> +	}
> +
> +	gdb_test "break $after_seq" \
> +	    "Breakpoint ${::decimal} at ${::hex}: file .*/aarch64-lse128.c, line ${::decimal}\\." \
> +	    "$insn: break after instruction sequence"
> +	gdb_continue_to_breakpoint "$insn: executed instruction sequence" \
> +	    [multi_line ".*/aarch64-lse128.c:${::decimal}" ".*"]
> +
> +	# Record the new memory and register values.
> +	foreach r $regs {
> +	    set ${r}_new [capture_command_output "info register $r" ""]
> +	}
> +
> +	# Execute in reverse to before the instruction sequence.
> +	gdb_test_no_output "set exec-direction reverse"
> +
> +	gdb_continue_to_breakpoint "reversed execution of instruction sequence" \
> +	    [multi_line ".*/aarch64-lse128.c:${::decimal}" \
> +		"${::decimal}\[ \t\]+__asm__ volatile \\(\"${insn} .*\".*"]
> +
> +	# Record the final memory and register values.
> +	foreach r $regs {
> +	    set ${r}_final [capture_command_output "info register $r" ""]
> +	}
> +
> +	foreach v $regs {
> +	    gdb_assert ![string compare [set ${v}_initial] [set ${v}_final]] \
> +		"$insn: check $v initial value versus $v final value"
> +	}
> +	foreach v {x19 x20} {
> +		gdb_assert [string compare [set ${v}_initial] [set ${v}_new]] \
> +		"$insn: check $v initial value versus $v new value"
> +	}
> +	gdb_assert ![string compare [set x21_initial] [set x21_new]] \
> +	"$insn: check x21 initial value versus x21 new 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-lse128.c:${::decimal}" \
> +		".*"]
> +    }
> +}
> +
> +set cases {
> +	{ ldclrp }
> +	{ ldclrpa }
> +	{ ldclrpal }
> +	{ ldclrpl }
> +	{ ldsetp }
> +	{ ldsetpa }
> +	{ ldsetpal }
> +	{ ldsetpl }
> +	{ swpp }
> +	{ swppa }
> +	{ swppal }
> +	{ swppl }
> +}
> +
> +foreach c $cases {
> +    lassign $c insn
> +    test_single_asm $insn
> +}
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 9d5c0e5a..d62b4927 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5402,6 +5402,62 @@ gdb_caching_proc allow_aarch64_cssc_tests {} {
>       return $allow_cssc_tests
>   }
>   
> +# Run a test on the target to see if it supports the AArch64 LSE128 feature.
> +# Return 1 if so, 0 if it does not.  Note this causes a restart of GDB.
> +
> +gdb_caching_proc allow_aarch64_lse128_tests {} {
> +    global srcdir subdir gdb_prompt inferior_exited_re
> +
> +    set me "allow_aarch64_lse128_tests"
> +
> +    if { ![is_aarch64_target]} {
> +	return 0
> +    }
> +
> +    set compile_flags "{additional_flags=-march=armv8-a+lse128}"
> +
> +    # Compile a test program reading LSE128.
> +    set src {
> +    #include <stdint.h>
> +
> +    int main() {
> +	    __attribute__((aligned(16))) uint64_t mem[2] = { 0x0, 0x1 };
> +	    uint64_t *ptr = mem;
> +	    __asm__ volatile ("ldclrp x0, x1, [%0]\n" :: "r"(ptr) : "x0", "x1", "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 lse128 support not detected"
> +	    set allow_lse128_tests 0
> +	}
> +	-re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> +	    verbose -log "\n$me: lse128 support detected"
> +	    set allow_lse128_tests 1
> +	}
> +	default {
> +	  warning "\n$me: default case taken"
> +	    set allow_lse128_tests 0
> +	}
> +    }
> +    gdb_exit
> +    remote_file build delete $obj
> +
> +    verbose "$me:  returning $allow_lse128_tests" 2
> +    return $allow_lse128_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.