From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ehHvMRiOxl+vTQAAWB0awg (envelope-from ) for ; Tue, 01 Dec 2020 13:40:24 -0500 Received: by simark.ca (Postfix, from userid 112) id C12E11F0AB; Tue, 1 Dec 2020 13:40:24 -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 13DFE1E58E for ; Tue, 1 Dec 2020 13:40:24 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5DDF63944829; Tue, 1 Dec 2020 18:40:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5DDF63944829 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606848023; bh=4eO5mZVqiygGiKXCdmClaETtoezp22v2gOk+r1Pw/3s=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ii3LR42luJpRqnPkDV4IEiDz/6m+8F7JSDOD3vwcxUuL/WMNWNk4sNfiTCN0sttqu xXxFFlNma6gSz+TFepq89vbrL2I+HZ9LLfhjW7XT12fLPXRk3Mi7rGcoq/15Sjig1P 4EFM6FHhKjR9vjCC3siF+kbaHhCTGIkn8QQTtH1w= Received: from mail-qk1-x741.google.com (mail-qk1-x741.google.com [IPv6:2607:f8b0:4864:20::741]) by sourceware.org (Postfix) with ESMTPS id E8A80385702D for ; Tue, 1 Dec 2020 18:40:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E8A80385702D Received: by mail-qk1-x741.google.com with SMTP id 1so2318845qka.0 for ; Tue, 01 Dec 2020 10:40:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=4eO5mZVqiygGiKXCdmClaETtoezp22v2gOk+r1Pw/3s=; b=IxcByV5tq+g4K1L1Yky6XNRV69g9mTgQ8gYbzr2jc2KAwsiG4Z7ORMITWwxAEDEDcv wHFQw7rNPqkj14DL3WLRK1Umj4onx3i1N4yFjG6zDz2PSbOcQSwDIzvHPXdH0MoUloDg T+nLS+eabFgGgBh0FsIF4XfOfFEqhq9F3QHvz0eoHUhNVhtWqMvFIFsY4j/YkPO90oB2 zNP0/Aq3n4cqvvJckYBj/Y/EknoNZQlXx+GShLG11qwvC7wSxb3Ty2+4MUrYNh1ugaSS PwP7Ze7SBQvilSQVaVR/1/G+9svh4/sUN68rUti+lKoCOhfeFcwazJQBhfKCi3dZ7HQp 4hSw== X-Gm-Message-State: AOAM5309myFRScwrSHRBKTvXMBpS0vOq8Q4mVKTw9sZu47iF6Cb3mwl2 4dzHrBJYVGGwXxKXqM0pCFJiPYCCfb1MuA== X-Google-Smtp-Source: ABdhPJwEXb2gXIXz2TgEx9qSJfJz8clx6a3QKf/0yW/9p01LDsiRoKidPDhWBwD3Jl+RkmyzsrEUHQ== X-Received: by 2002:a37:6191:: with SMTP id v139mr4201666qkb.92.1606848018129; Tue, 01 Dec 2020 10:40:18 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:370e:60a9:ecc0:3a2a:98f9? ([2804:7f0:8284:370e:60a9:ecc0:3a2a:98f9]) by smtp.gmail.com with ESMTPSA id b3sm457242qkf.74.2020.12.01.10.40.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Dec 2020 10:40:16 -0800 (PST) Subject: Re: [PATCH][AArch64] SVE/FPSIMD fixup for big endian To: Alan Hayward References: <20201130185545.940242-1-luis.machado@linaro.org> <791ee593-8d5b-2eec-17c8-fa34a3809d12@linaro.org> Message-ID: Date: Tue, 1 Dec 2020 15:40:12 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US 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 Cc: nd , "gdb-patches\\@sourceware.org" Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 12/1/20 2:38 PM, Alan Hayward wrote: > > >> On 1 Dec 2020, at 12:19, Luis Machado wrote: >> >> Hi, >> >> Thanks for the review. >> >> On 12/1/20 8:28 AM, Alan Hayward wrote: >>>> On 30 Nov 2020, at 18:55, Luis Machado wrote: >>>> >>>> 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. */ >>>> + >>> With this function, we are only handling FPSIMD values, so no need to mention SVE. >>> As it is now, it makes the has_sve parts confusing because they are being treated >>> the same as the rest of the fpsimd. >> >> Though we are handling FPSIMD, we still need to set at least one SVE pseudo-register (AARCH64_SVE_V0_REGNUM), hence why I decided to keep the warning. It is not really a SVE register, but a pseudo-register of the FPSIMD V register. >> >> Do you still want to remove the SVE references in the warning? > > For this function, I would say remove the SVE comment, as everything in the function > is being treated in the same way. But I’m not overly hung up on it. > It now reads... /* 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. */ >> >> What is the difference between AARCH64_SVE_V0_REGNUM and AARCH64_V0_REGNUM? > > Z,V,Q,D,S,H,B registers all overlap with each other. It’s the same register, > just a different size. > In the tdep, when we only have neon, V is the real register, and Q,D,S,H,B are pseudos. > Then when we have sve, Z is the real register, and V,Q,D,S,H,B are pseudos. > > AARCH64_SVE_Z0_REGNUM == AARCH64_V0_REGNUM. > > And then AARCH64_SVE_V0_REGNUM represents the regnum of the V0 pseudo register > when SVE is enabled. > > ARCH64_SVE_V0_REGNUM == AARCH64_B0_REGNUM + 32 > > Maybe eventually this should be updated to be similar to pauth. Have a single define > which requires a v0_reg_base value in the tdep. > > >> >>> The same comment is fine when used elsewhere in the patch. >>>> + 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); >>> I think we have a long standing bug here. zero_reg was meant to stay as the value 0. But then >>> it got reused as a general temp buffer. >>> It’s not shown in the diff, but we do: >>> * memset zero_reg to 0 >>> * use zero_reg as a temp buffer for copying fpsimd values. >>> * use zero_reg as value 0 for fpsr and fpcr. >>> The memset needs moving after using it for fpsimd. (maybe also rename zero_reg to buf?) >> >> I'll address this. >> >>> Can we also reduce the number of memcpys - just byte swap vregs[i] directly into the zero_reg buffer? >> >> The byte swap function only swaps things for __BYTE_ORDER == __BIG_ENDIAN, we would still need to memcpy the bytes to zero_reg. The number of memcpy's would be the same, no? > > For LE, we want to copy into zero_reg to ensure it is zero padded after 128bits, then call raw_supply. > So that’s all fine. > > For BE, we could just byteswap directly from fpsimd->vregs[i] into zero_reg (?) No need for any memcpys. > > Are you suggesting rewriting aarch64_maybe_swab128 to take dst and src parameters, so we can byteswap from vregs[i] into zero_reg? Right now aarch64_maybe_swab128 only swaps things in place. >> >>>> 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); >>> Am I right to assume this means data will be automatically unallocated when >>> the trad_frame_saved_reg goes out of scope? >> >> Not when it goes out of scope, but when all the frame-related data is freed by GDB just before restarting inferior movement. > > Ok. > >> >>>> + 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 >>>> >