From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 3THBJzxAxV8wLwAAWB0awg (envelope-from ) for ; Mon, 30 Nov 2020 13:55:56 -0500 Received: by simark.ca (Postfix, from userid 112) id 9745E1F0AB; Mon, 30 Nov 2020 13:55:56 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B03F71E590 for ; Mon, 30 Nov 2020 13:55:55 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F0AB33898036; Mon, 30 Nov 2020 18:55:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F0AB33898036 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606762555; bh=MspT3ZLQ9fh5eVUwPfM9uNDuf9o0zCyCTSm+EZ0fG9g=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=BGmsz0tIhyXXCpaH4qjfqQ0V3VFVfgfjTikjDhCrJBGHR7lloEB/tTepSr2qGPEND FMSjS0g5UstYwxVCL64M6SMtO2McDAzHk12xR8kmuobjvpvvFhmj+N6sV1387h6Nru +WwKUTfStX3Ire+nT4mZg/RGZ9FQYZSrZKf3nd3E= Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) by sourceware.org (Postfix) with ESMTPS id 390C9388A43D for ; Mon, 30 Nov 2020 18:55:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 390C9388A43D Received: by mail-qk1-x732.google.com with SMTP id q5so11852072qkc.12 for ; Mon, 30 Nov 2020 10:55:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=MspT3ZLQ9fh5eVUwPfM9uNDuf9o0zCyCTSm+EZ0fG9g=; b=eqKZ/b3HBfLi4qu9+q3l8INGAWYeuuwXLkA93eDieXwLfWMwVx8U3bwfeLT/9tI5IL IMj7nWSCms0Z8ogbpbU5YfLN2C7YJfZ/AkjezYfCjOAjvWiqeRGSt37Xv/EJpafThgXb n2P/9h6M1ZPLaFjlUqPA+aQTAwU6QSSgxJBEtBeMi68MQqXdjglHPcs4QTkFTIC8FGvP pLNlZY5InVovVF2EZsqlk3Ky2QaCr5BUGR9tJZWiuxjjpTS+vmXx4D2AfFgbeeefaJ0m tVAiMUBrqqDlGHuKT54KTZlnYCxksO+EcTsCsKnJ8hEAIom4UH7KZcVxJVnoVl5HfOwp nEkQ== X-Gm-Message-State: AOAM533aI4pkioGjCCRA4KLeRr1zOMydbPkUL9gfoiNiimKkCReMlDap GbPDUBwJXcI5PgXXrZfAeKrx/nMLNRYcXg== X-Google-Smtp-Source: ABdhPJxHoKQCE2lNw06NBEBBPwQArrsk3L1BZE+cpXAFZkFVRAExF2cP7SXWX0zHWyeB0P66/nr4lQ== X-Received: by 2002:ae9:df47:: with SMTP id t68mr24097227qkf.438.1606762551195; Mon, 30 Nov 2020 10:55:51 -0800 (PST) Received: from localhost.localdomain ([2804:7f0:8284:370e:e01b:d22:75ad:70ae]) by smtp.gmail.com with ESMTPSA id h13sm18316173qtc.4.2020.11.30.10.55.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Nov 2020 10:55:50 -0800 (PST) To: gdb-patches@sourceware.org Subject: [PATCH][AArch64] SVE/FPSIMD fixup for big endian Date: Mon, 30 Nov 2020 15:55:45 -0300 Message-Id: <20201130185545.940242-1-luis.machado@linaro.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Luis Machado via Gdb-patches Reply-To: Luis Machado Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" 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: YYYY-MM-DD Luis Machado * aarch64-linux-tdep.c (aarch64_linux_restore_vreg) New function. (aarch64_linux_sigframe_init): Call aarch64_linux_restore_vreg. * 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/aarch64-linux-tdep.c | 115 ++++++++++++++++++++++++----- gdb/nat/aarch64-sve-linux-ptrace.c | 57 +++++++++++++- gdb/trad-frame.c | 46 +++++++++++- gdb/trad-frame.h | 19 +++++ 4 files changed, 213 insertions(+), 24 deletions(-) diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index c9898bdafd..108f96be71 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -180,6 +180,94 @@ 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, 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. */ + + 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 +420,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/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c index 2ce90ccfd7..9ef5e91801 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,27 @@ aarch64_sve_get_sveregs (int tid) return buf; } +/* If we are running in BE mode, convert the contents + of VALUE (a 16 byte buffer) to LE. */ + +static void +aarch64_maybe_swab128 (gdb_byte *value) +{ + gdb_assert (value != nullptr); + +#if (__BYTE_ORDER == __BIG_ENDIAN) + gdb_byte copy[16]; + + /* Save the original value. */ + memcpy (copy, value, 16); + + for (int i = 0; i < 15; i++) + value[i] = copy[15 - i]; +#else + /* Nothing to be done. */ +#endif +} + /* See nat/aarch64-sve-linux-ptrace.h. */ void @@ -184,11 +206,22 @@ 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 *zero_reg = (gdb_byte *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); struct user_fpsimd_state *fpsimd = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET); @@ -199,7 +232,9 @@ aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf, for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++) { - memcpy (zero_reg, &fpsimd->vregs[i], sizeof (__int128_t)); + memcpy (zero_reg, &fpsimd->vregs[i], 16); + /* Handle big endian/little endian SIMD/SVE conversion. */ + aarch64_maybe_swab128 (zero_reg); reg_buf->raw_supply (AARCH64_SVE_Z0_REGNUM + i, zero_reg); } @@ -240,7 +275,7 @@ 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 *zero_reg = (gdb_byte *) alloca (SVE_PT_SVE_ZREG_SIZE (vq)); struct user_fpsimd_state *fpsimd = (struct user_fpsimd_state *)(base + SVE_PT_FPSIMD_OFFSET); @@ -274,6 +309,18 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, 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 @@ -285,7 +332,9 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, == 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)); + /* Handle big endian/little endian SIMD/SVE conversion. */ + aarch64_maybe_swab128 (zero_reg); + memcpy (&fpsimd->vregs[i], zero_reg, 16); } } diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c index a6a84790a9..8a1aa818ad 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 7b5785616e..38db439579 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.25.1