From 6afcd2d4161165fd976c55a9792dbcccd2d5d106 Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Wed, 18 Mar 2020 13:06:05 -0300 Subject: [PATCH] [AArch64] SVE/FPSIMD fixup for big endian The FPSIMD dump in signal frames and ptrace FPSIMD dump in the SVE context structure follows the target endianness, whereas the SVE dumps are endianness-independent (LE). Therefore, when the system is in BE mode, we need to reverse the bytes for the FPSIMD data. Given the V registers are larger than 64-bit, I've added a way for value bytes to be set, as opposed to passing a 64-bit fixed quantity. This fits nicely with the unwinding *_got_bytes function and makes the trad-frame more flexible and capable of saving larger registers. The memory for the bytes is allocated via the frame obstack, so it gets freed after we're done inspecting the frame. gdb/ChangeLog: 2020-12-10 Luis Machado * aarch64-linux-tdep.c (aarch64_linux_restore_vreg) New function. (aarch64_linux_sigframe_init): Call aarch64_linux_restore_vreg. * aarch64-tdep.h (V_REGISTER_SIZE): Move to ... * arch/aarch64.h: ... here. * nat/aarch64-sve-linux-ptrace.c: Include endian.h. (aarch64_maybe_swab128): New function. (aarch64_sve_regs_copy_to_reg_buf) (aarch64_sve_regs_copy_from_reg_buf): Adjust FPSIMD entries. * trad-frame.c (trad_frame_reset_saved_regs): Initialize the data field. (TF_REG_VALUE_BYTES): New enum value. (trad_frame_value_bytes_p): New function. (trad_frame_set_value_bytes): New function. (trad_frame_set_reg_value_bytes): New function. (trad_frame_get_prev_register): Handle register values saved as bytes. * trad-frame.h (trad_frame_set_reg_value_bytes): New prototype. (struct trad_frame_saved_reg) : New field. (trad_frame_set_value_bytes): New prototype. (trad_frame_value_bytes_p): New prototype. --- gdb/ChangeLog | 22 ++++++ gdb/aarch64-linux-tdep.c | 114 ++++++++++++++++++++++++----- gdb/aarch64-tdep.h | 1 - gdb/arch/aarch64.h | 2 + gdb/nat/aarch64-sve-linux-ptrace.c | 77 +++++++++++++++---- gdb/trad-frame.c | 46 +++++++++++- gdb/trad-frame.h | 19 +++++ 7 files changed, 246 insertions(+), 35 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 93529546e3f..ace56aa251d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,25 @@ +2020-12-10 Luis Machado + + * aarch64-linux-tdep.c (aarch64_linux_restore_vreg) New function. + (aarch64_linux_sigframe_init): Call aarch64_linux_restore_vreg. + * aarch64-tdep.h (V_REGISTER_SIZE): Move to ... + * arch/aarch64.h: ... here. + * nat/aarch64-sve-linux-ptrace.c: Include endian.h. + (aarch64_maybe_swab128): New function. + (aarch64_sve_regs_copy_to_reg_buf) + (aarch64_sve_regs_copy_from_reg_buf): Adjust FPSIMD entries. + * trad-frame.c (trad_frame_reset_saved_regs): Initialize + the data field. + (TF_REG_VALUE_BYTES): New enum value. + (trad_frame_value_bytes_p): New function. + (trad_frame_set_value_bytes): New function. + (trad_frame_set_reg_value_bytes): New function. + (trad_frame_get_prev_register): Handle register values saved as bytes. + * trad-frame.h (trad_frame_set_reg_value_bytes): New prototype. + (struct trad_frame_saved_reg) : New field. + (trad_frame_set_value_bytes): New prototype. + (trad_frame_value_bytes_p): New prototype. + 2020-12-07 Mihails Strasuns * jit.c (mem_bfd*, bfd_open_from_target_memory): Removed. diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index ce697ff246e..1e8d8e9d6b3 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -180,6 +180,93 @@ read_aarch64_ctx (CORE_ADDR ctx_addr, enum bfd_endian byte_order, return magic; } +/* Given CACHE, use the trad_frame* functions to restore the FPSIMD + registers from a signal frame. + + VREG_NUM is the number of the V register being restored, OFFSET is the + address containing the register value, BYTE_ORDER is the endianness and + HAS_SVE tells us if we have a valid SVE context or not. */ + +static void +aarch64_linux_restore_vreg (struct trad_frame_cache *cache, int num_regs, + int vreg_num, CORE_ADDR offset, + enum bfd_endian byte_order, bool has_sve) +{ + /* WARNING: SIMD state is laid out in memory in target-endian format. + + So we have a couple cases to consider: + + 1 - If the target is big endian, then SIMD state is big endian, + requiring a byteswap. + + 2 - If the target is little endian, then SIMD state is little endian, so + no byteswap is needed. */ + + if (byte_order == BFD_ENDIAN_BIG) + { + gdb_byte buf[V_REGISTER_SIZE]; + + if (target_read_memory (offset, buf, V_REGISTER_SIZE) != 0) + { + size_t size = V_REGISTER_SIZE/2; + + /* Read the two halves of the V register in reverse byte order. */ + CORE_ADDR u64 = extract_unsigned_integer (buf, size, + byte_order); + CORE_ADDR l64 = extract_unsigned_integer (buf + size, size, + byte_order); + + /* Copy the reversed bytes to the buffer. */ + store_unsigned_integer (buf, size, BFD_ENDIAN_LITTLE, l64); + store_unsigned_integer (buf + size , size, BFD_ENDIAN_LITTLE, u64); + + /* Now we can store the correct bytes for the V register. */ + trad_frame_set_reg_value_bytes (cache, AARCH64_V0_REGNUM + vreg_num, + buf, V_REGISTER_SIZE); + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_Q0_REGNUM + + vreg_num, buf, Q_REGISTER_SIZE); + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_D0_REGNUM + + vreg_num, buf, D_REGISTER_SIZE); + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_S0_REGNUM + + vreg_num, buf, S_REGISTER_SIZE); + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_H0_REGNUM + + vreg_num, buf, H_REGISTER_SIZE); + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_B0_REGNUM + + vreg_num, buf, B_REGISTER_SIZE); + + if (has_sve) + trad_frame_set_reg_value_bytes (cache, + num_regs + AARCH64_SVE_V0_REGNUM + + vreg_num, buf, V_REGISTER_SIZE); + } + return; + } + + /* Little endian, just point at the address containing the register + value. */ + trad_frame_set_reg_addr (cache, AARCH64_V0_REGNUM + vreg_num, offset); + trad_frame_set_reg_addr (cache, num_regs + AARCH64_Q0_REGNUM + vreg_num, + offset); + trad_frame_set_reg_addr (cache, num_regs + AARCH64_D0_REGNUM + vreg_num, + offset); + trad_frame_set_reg_addr (cache, num_regs + AARCH64_S0_REGNUM + vreg_num, + offset); + trad_frame_set_reg_addr (cache, num_regs + AARCH64_H0_REGNUM + vreg_num, + offset); + trad_frame_set_reg_addr (cache, num_regs + AARCH64_B0_REGNUM + vreg_num, + offset); + + if (has_sve) + trad_frame_set_reg_addr (cache, num_regs + AARCH64_SVE_V0_REGNUM + + vreg_num, offset); + +} + /* Implement the "init" method of struct tramp_frame. */ static void @@ -332,27 +419,16 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self, /* If there was no SVE section then set up the V registers. */ if (sve_regs == 0) - for (int i = 0; i < 32; i++) - { - CORE_ADDR offset = (fpsimd + AARCH64_FPSIMD_V0_OFFSET + { + for (int i = 0; i < 32; i++) + { + CORE_ADDR offset = (fpsimd + AARCH64_FPSIMD_V0_OFFSET + (i * AARCH64_FPSIMD_VREG_SIZE)); - trad_frame_set_reg_addr (this_cache, AARCH64_V0_REGNUM + i, offset); - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_Q0_REGNUM + i, offset); - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_D0_REGNUM + i, offset); - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_S0_REGNUM + i, offset); - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_H0_REGNUM + i, offset); - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_B0_REGNUM + i, offset); - if (tdep->has_sve ()) - trad_frame_set_reg_addr (this_cache, - num_regs + AARCH64_SVE_V0_REGNUM + i, - offset); - } + aarch64_linux_restore_vreg (this_cache, num_regs, i, offset, + byte_order, tdep->has_sve ()); + } + } } trad_frame_set_id (this_cache, frame_id_build (sp, func)); diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index 76ff812abc6..7d035c1aecd 100644 --- a/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h @@ -48,7 +48,6 @@ struct regset; #define H_REGISTER_SIZE 2 #define S_REGISTER_SIZE 4 #define D_REGISTER_SIZE 8 -#define V_REGISTER_SIZE 16 #define Q_REGISTER_SIZE 16 /* Total number of general (X) registers. */ diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h index 857bb22b037..b75352461a4 100644 --- a/gdb/arch/aarch64.h +++ b/gdb/arch/aarch64.h @@ -58,6 +58,8 @@ enum aarch64_regnum AARCH64_LAST_V_ARG_REGNUM = AARCH64_V0_REGNUM + 7 }; +#define V_REGISTER_SIZE 16 + /* Pseudo register base numbers. */ #define AARCH64_Q0_REGNUM 0 #define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + AARCH64_D_REGISTER_COUNT) diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c index 2ce90ccfd7e..5f08995f87b 100644 --- a/gdb/nat/aarch64-sve-linux-ptrace.c +++ b/gdb/nat/aarch64-sve-linux-ptrace.c @@ -26,6 +26,7 @@ #include "arch/aarch64.h" #include "gdbsupport/common-regcache.h" #include "gdbsupport/byte-vector.h" +#include /* See nat/aarch64-sve-linux-ptrace.h. */ @@ -142,6 +143,24 @@ aarch64_sve_get_sveregs (int tid) return buf; } +/* If we are running in BE mode, byteswap the contents + of SRC to DST for SIZE bytes. Other, just copy the contents + from SRC to DST. */ + +static void +aarch64_maybe_swab128 (gdb_byte *dst, const gdb_byte *src, size_t size) +{ + gdb_assert (src != nullptr && dst != nullptr); + gdb_assert (size > 1); + +#if (__BYTE_ORDER == __BIG_ENDIAN) + for (int i = 0; i < size - 1; i++) + dst[i] = src[size - i]; +#else + memcpy (dst, src, size); +#endif +} + /* See nat/aarch64-sve-linux-ptrace.h. */ void @@ -184,34 +203,50 @@ aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf, } else { + /* WARNING: SIMD state is laid out in memory in target-endian format, + while SVE state is laid out in an endianness-independent format (LE). + + So we have a couple cases to consider: + + 1 - If the target is big endian, then SIMD state is big endian, + requiring a byteswap. + + 2 - If the target is little endian, then SIMD state is little endian, + which matches the SVE format, so no byteswap is needed. */ + /* There is no SVE state yet - the register dump contains a fpsimd structure instead. These registers still exist in the hardware, but the kernel has not yet initialised them, and so they will be null. */ - char *zero_reg = (char *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); + gdb_byte *reg = (gdb_byte *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); struct user_fpsimd_state *fpsimd = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET); + /* Make sure we have a zeroed register buffer. We will need the zero + padding below. */ + memset (reg, 0, SVE_PT_SVE_ZREG_SIZE (vq)); + /* Copy across the V registers from fpsimd structure to the Z registers, ensuring the non overlapping state is set to null. */ - memset (zero_reg, 0, SVE_PT_SVE_ZREG_SIZE (vq)); - for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++) { - memcpy (zero_reg, &fpsimd->vregs[i], sizeof (__int128_t)); - reg_buf->raw_supply (AARCH64_SVE_Z0_REGNUM + i, zero_reg); + /* Handle big endian/little endian SIMD/SVE conversion. */ + aarch64_maybe_swab128 (reg, (const gdb_byte *) &fpsimd->vregs[i], + V_REGISTER_SIZE); + reg_buf->raw_supply (AARCH64_SVE_Z0_REGNUM + i, reg); } reg_buf->raw_supply (AARCH64_FPSR_REGNUM, &fpsimd->fpsr); reg_buf->raw_supply (AARCH64_FPCR_REGNUM, &fpsimd->fpcr); /* Clear the SVE only registers. */ + memset (reg, 0, SVE_PT_SVE_ZREG_SIZE (vq)); for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++) - reg_buf->raw_supply (AARCH64_SVE_P0_REGNUM + i, zero_reg); + reg_buf->raw_supply (AARCH64_SVE_P0_REGNUM + i, reg); - reg_buf->raw_supply (AARCH64_SVE_FFR_REGNUM, zero_reg); + reg_buf->raw_supply (AARCH64_SVE_FFR_REGNUM, reg); } } @@ -240,11 +275,11 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, kernel, which is why we try to avoid it. */ bool has_sve_state = false; - char *zero_reg = (char *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); + gdb_byte *reg = (gdb_byte *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); struct user_fpsimd_state *fpsimd = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET); - memset (zero_reg, 0, SVE_PT_SVE_ZREG_SIZE (vq)); + memset (reg, 0, SVE_PT_SVE_ZREG_SIZE (vq)); /* Check in the reg_buf if any of the Z registers are set after the first 128 bits, or if any of the other SVE registers are set. */ @@ -252,7 +287,7 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++) { has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_Z0_REGNUM + i, - zero_reg, sizeof (__int128_t)); + reg, sizeof (__int128_t)); if (has_sve_state) break; } @@ -261,19 +296,31 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++) { has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_P0_REGNUM + i, - zero_reg, 0); + reg, 0); if (has_sve_state) break; } if (!has_sve_state) has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_FFR_REGNUM, - zero_reg, 0); + reg, 0); /* If no SVE state exists, then use the existing fpsimd structure to write out state and return. */ if (!has_sve_state) { + /* WARNING: SIMD state is laid out in memory in target-endian format, + while SVE state is laid out in an endianness-independent format + (LE). + + So we have a couple cases to consider: + + 1 - If the target is big endian, then SIMD state is big endian, + requiring a byteswap. + + 2 - If the target is little endian, then SIMD state is little + endian, which matches the SVE format, so no byteswap is needed. */ + /* The collects of the Z registers will overflow the size of a vreg. There is enough space in the structure to allow for this, but we cannot overflow into the next register as we might not be @@ -284,8 +331,10 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, if (REG_VALID == reg_buf->get_register_status (AARCH64_SVE_Z0_REGNUM + i)) { - reg_buf->raw_collect (AARCH64_SVE_Z0_REGNUM + i, zero_reg); - memcpy (&fpsimd->vregs[i], zero_reg, sizeof (__int128_t)); + reg_buf->raw_collect (AARCH64_SVE_Z0_REGNUM + i, reg); + /* Handle big endian/little endian SIMD/SVE conversion. */ + aarch64_maybe_swab128 ((gdb_byte *) &fpsimd->vregs[i], reg, + V_REGISTER_SIZE); } } diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c index a6a84790a99..8a1aa818ade 100644 --- a/gdb/trad-frame.c +++ b/gdb/trad-frame.c @@ -56,6 +56,7 @@ trad_frame_reset_saved_regs (struct gdbarch *gdbarch, { regs[regnum].realreg = regnum; regs[regnum].addr = -1; + regs[regnum].data = nullptr; } } @@ -83,7 +84,7 @@ trad_frame_alloc_saved_regs (struct frame_info *this_frame) return trad_frame_alloc_saved_regs (gdbarch); } -enum { TF_REG_VALUE = -1, TF_REG_UNKNOWN = -2 }; +enum { TF_REG_VALUE = -1, TF_REG_UNKNOWN = -2, TF_REG_VALUE_BYTES = -3 }; int trad_frame_value_p (struct trad_frame_saved_reg this_saved_regs[], int regnum) @@ -106,6 +107,16 @@ trad_frame_realreg_p (struct trad_frame_saved_reg this_saved_regs[], && this_saved_regs[regnum].addr == -1); } +/* See trad-frame.h. */ + +bool +trad_frame_value_bytes_p (struct trad_frame_saved_reg this_saved_regs[], + int regnum) +{ + return (this_saved_regs[regnum].realreg == TF_REG_VALUE_BYTES + && this_saved_regs[regnum].data != nullptr); +} + void trad_frame_set_value (struct trad_frame_saved_reg this_saved_regs[], int regnum, LONGEST val) @@ -224,6 +235,35 @@ trad_frame_set_unknown (struct trad_frame_saved_reg this_saved_regs[], this_saved_regs[regnum].addr = -1; } +/* See trad-frame.h. */ + +void +trad_frame_set_value_bytes (struct trad_frame_saved_reg this_saved_regs[], + int regnum, const gdb_byte *bytes, + size_t size) +{ + this_saved_regs[regnum].realreg = TF_REG_VALUE_BYTES; + + /* Allocate the space and copy the data bytes. */ + this_saved_regs[regnum].data = FRAME_OBSTACK_CALLOC (size, gdb_byte); + memcpy (this_saved_regs[regnum].data, bytes, size); +} + +/* See trad-frame.h. */ + +void +trad_frame_set_reg_value_bytes (struct trad_frame_cache *this_trad_cache, + int regnum, const gdb_byte *bytes, + size_t size) +{ + /* External interface for users of trad_frame_cache + (who cannot access the prev_regs object directly). */ + trad_frame_set_value_bytes (this_trad_cache->prev_regs, regnum, bytes, + size); +} + + + struct value * trad_frame_get_prev_register (struct frame_info *this_frame, struct trad_frame_saved_reg this_saved_regs[], @@ -240,6 +280,10 @@ trad_frame_get_prev_register (struct frame_info *this_frame, /* The register's value is available. */ return frame_unwind_got_constant (this_frame, regnum, this_saved_regs[regnum].addr); + else if (trad_frame_value_bytes_p (this_saved_regs, regnum)) + /* The register's value is available as a sequence of bytes. */ + return frame_unwind_got_bytes (this_frame, regnum, + this_saved_regs[regnum].data); else return frame_unwind_got_optimized (this_frame, regnum); } diff --git a/gdb/trad-frame.h b/gdb/trad-frame.h index 7b5785616e0..38db4395792 100644 --- a/gdb/trad-frame.h +++ b/gdb/trad-frame.h @@ -52,6 +52,12 @@ void trad_frame_set_reg_regmap (struct trad_frame_cache *this_trad_cache, void trad_frame_set_reg_value (struct trad_frame_cache *this_cache, int regnum, LONGEST val); +/* Given the cache in THIS_TRAD_CACHE, set the value of REGNUM to the bytes + contained in BYTES with size SIZE. */ +void trad_frame_set_reg_value_bytes (struct trad_frame_cache *this_trad_cache, + int regnum, const gdb_byte *bytes, + size_t size); + struct value *trad_frame_get_register (struct trad_frame_cache *this_trad_cache, struct frame_info *this_frame, int regnum); @@ -86,6 +92,8 @@ struct trad_frame_saved_reg { LONGEST addr; /* A CORE_ADDR fits in a longest. */ int realreg; + /* Register data (for values that don't fit in ADDR). */ + gdb_byte *data; }; /* Encode REGNUM value in the trad-frame. */ @@ -104,6 +112,12 @@ void trad_frame_set_addr (struct trad_frame_saved_reg this_trad_cache[], void trad_frame_set_unknown (struct trad_frame_saved_reg this_saved_regs[], int regnum); +/* Encode REGNUM value in the trad-frame as a sequence of bytes. This is + useful when the value is larger than what primitive types can hold. */ +void trad_frame_set_value_bytes (struct trad_frame_saved_reg this_saved_regs[], + int regnum, const gdb_byte *bytes, + size_t size); + /* Convenience functions, return non-zero if the register has been encoded as specified. */ int trad_frame_value_p (struct trad_frame_saved_reg this_saved_regs[], @@ -113,6 +127,11 @@ int trad_frame_addr_p (struct trad_frame_saved_reg this_saved_regs[], int trad_frame_realreg_p (struct trad_frame_saved_reg this_saved_regs[], int regnum); +/* Return TRUE if REGNUM is stored as a sequence of bytes, and FALSE + otherwise. */ +bool trad_frame_value_bytes_p (struct trad_frame_saved_reg this_saved_regs[], + int regnum); + /* Reset the save regs cache, setting register values to -1. */ void trad_frame_reset_saved_regs (struct gdbarch *gdbarch, struct trad_frame_saved_reg *regs); -- 2.30.2