From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id 819D33858D32 for ; Thu, 7 Jul 2022 09:11:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 819D33858D32 Received: from pps.filterd (m0288072.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 2675B5so029403; Thu, 7 Jul 2022 11:11:12 +0200 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3h4ubfbmb6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 07 Jul 2022 11:11:11 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 3059610002A; Thu, 7 Jul 2022 11:11:11 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node2.st.com [10.75.129.70]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 237A7212FC1; Thu, 7 Jul 2022 11:11:11 +0200 (CEST) Received: from gnbcxd0114.gnb.st.com (10.75.127.50) by SHFDAG1NODE2.st.com (10.75.129.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.20; Thu, 7 Jul 2022 11:11:08 +0200 Date: Thu, 7 Jul 2022 11:11:07 +0200 From: Yvan Roux To: , Luis Machado CC: Torbjorn SVENSSON Subject: Re: [PATCH] gdb/arm: Sync sp with other *sp registers Message-ID: <20220707091107.GA5239@gnbcxd0114.gnb.st.com> References: <20220630071329.GA28593@gnbcxd0114.gnb.st.com> <3a4bec66-7031-74ad-f7f4-d70af5dfe725@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3a4bec66-7031-74ad-f7f4-d70af5dfe725@arm.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-Originating-IP: [10.75.127.50] X-ClientProxiedBy: SFHDAG2NODE3.st.com (10.75.127.6) To SHFDAG1NODE2.st.com (10.75.129.70) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-07-07_06,2022-06-28_01,2022-06-22_01 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: , X-List-Received-Date: Thu, 07 Jul 2022 09:11:17 -0000 On Wed, Jul 06, 2022 at 01:34:59PM +0100, Luis Machado wrote: > Hi, > > On 6/30/22 08:13, Yvan Roux wrote: > > Hi, > > > > For Arm Cortex-M33 with security extensions, there are 4 different > > stack pointers (msp_s, msp_ns, psp_s, psp_ns), without security > > extensions and for other Cortex-M targets, there are 2 different > > stack pointers (msp and psp). > > > > This patch allows arch dependent unwinding override register values > > for registers not covered by the DWARF data. That means that sp > > will always be in sync with one of the real stack pointers on > > Arm targets that contains more than one stack pointer. > > > > Signed-off-by: Torbjörn SVENSSON > > Signed-off-by: Yvan Roux > > --- > > gdb/arm-tdep.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++ > > gdb/dwarf2/frame.c | 58 ++++++++++++++++++++++++---- > > gdb/dwarf2/frame.h | 9 +++++ > > 3 files changed, 153 insertions(+), 8 deletions(-) > > > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > > index 3a1b52c2380..48711cb2a65 100644 > > --- a/gdb/arm-tdep.c > > +++ b/gdb/arm-tdep.c > > @@ -4956,6 +4956,98 @@ arm_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum, > > } > > } > > +/* Return the value of the unspecified register REGNUM for THIS_FRAME. */ > > + > > +static struct value * > > +arm_dwarf2_frame_prev_unspecified_reg (struct frame_info *this_frame, > > + int regnum) > > +{ > > + struct gdbarch *gdbarch = get_frame_arch (this_frame); > > + arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch); > > + bool override_with_sp_value = false; > > + > > + if (tdep->have_sec_ext) > > + { > > + CORE_ADDR sp, msp_s, msp_ns, psp_s, psp_ns; > > + sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM); > > + msp_s = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_msp_s_regnum); > > + msp_ns = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_msp_ns_regnum); > > + psp_s = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_psp_s_regnum); > > + psp_ns = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_psp_ns_regnum); > > + > > + if (regnum == tdep->m_profile_msp_regnum && (msp_s == sp || msp_ns == sp)) > > + { > > + /* Use value of SP for MSP register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_msp_s_regnum && msp_s == sp) > > + { > > + /* Use value of SP for MSP_S register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_msp_ns_regnum && msp_ns == sp) > > + { > > + /* Use value of SP for MSP_NS register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_psp_regnum && (psp_s == sp > > + || psp_ns == sp)) > > + { > > + /* Use value of SP for PSP register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_psp_s_regnum && psp_s == sp) > > + { > > + /* Use value of SP for PSP_S register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_psp_ns_regnum && psp_ns == sp) > > + { > > + /* Use value of SP for PSP_NS register. */ > > + override_with_sp_value = true; > > + } > > + } > > + else if (tdep->is_m) > > + { > > + CORE_ADDR sp, msp, psp; > > + sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM); > > + msp = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_msp_regnum); > > + psp = get_frame_register_unsigned (this_frame, > > + tdep->m_profile_psp_regnum); > > + > > + if (regnum == tdep->m_profile_msp_regnum && sp == msp) > > + { > > + /* Use value of SP for MSP register. */ > > + override_with_sp_value = true; > > + } > > + else if (regnum == tdep->m_profile_psp_regnum && sp == psp) > > + { > > + /* Use value of SP for PSP register. */ > > + override_with_sp_value = true; > > + } > > + } > > + > > + if (override_with_sp_value) > > + { > > + CORE_ADDR prev_sp; > > + struct frame_info *prev_frame = get_prev_frame (this_frame); > > + if (prev_frame) > > + prev_sp = get_frame_register_unsigned (prev_frame, ARM_SP_REGNUM); > > + else > > + prev_sp = get_frame_base (this_frame); > > + > > + return frame_unwind_got_constant (this_frame, regnum, prev_sp); > > + } > > + > > + /* Don't know how to fetch this value, use generic method. */ > > + return NULL; > > +} > > + > > /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand > > the buffer to be NEW_LEN bytes ending at ENDADDR. Return > > NULL if an error occurs. BUF is freed. */ > > @@ -10438,6 +10530,8 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > gdbarch_init_osabi (info, gdbarch); > > dwarf2_frame_set_init_reg (gdbarch, arm_dwarf2_frame_init_reg); > > + dwarf2_frame_set_prev_unspecified_reg (gdbarch, > > + arm_dwarf2_frame_prev_unspecified_reg); > > /* Add some default predicates. */ > > if (is_m) > > diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c > > index 5878d72f922..c4b15b2d240 100644 > > --- a/gdb/dwarf2/frame.c > > +++ b/gdb/dwarf2/frame.c > > @@ -595,6 +595,10 @@ struct dwarf2_frame_ops > > /* Convert .eh_frame register number to DWARF register number, or > > adjust .debug_frame register number. */ > > int (*adjust_regnum) (struct gdbarch *, int, int); > > + > > + /* Return the value of the unspecified register REGNUM for THIS_FRAME. */ > > + struct value * (*prev_unspecified_reg) (struct frame_info *this_frame, > > + int regnum); > > }; > > /* Default architecture-specific register state initialization > > @@ -677,6 +681,50 @@ dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum, > > ops->init_reg (gdbarch, regnum, reg, this_frame); > > } > > +/* Set the architecture-specific unspecified register unwinding > > + function for GDBARCH to PREV_UNSPECIFIED_REG. */ > > + > > +void > > +dwarf2_frame_set_prev_unspecified_reg (struct gdbarch *gdbarch, > > + struct value * (*prev_unspecified_reg) > > + (struct frame_info *this_frame, > > + int regnum)) > > +{ > > + struct dwarf2_frame_ops *ops > > + = (struct dwarf2_frame_ops *) gdbarch_data (gdbarch, dwarf2_frame_data); > > + > > + ops->prev_unspecified_reg = prev_unspecified_reg; > > +} > > I'm wondering if we really need to introduce a hook here. I understand we may need to sidestep the dwarf unwinder > to do custom handling of the msp*/psp* registers, but wouldn't we be able to achieve the same with the existing > dwarf hooks? > > For example, we could register a hook with dwarf2_frame_set_init_reg (), like aarch64 (see gdb/aarch64-tdep.c:aarch64_dwarf2_frame_init_reg). > > With such a hook, we could define our own rules for individual registers through DWARF2_FRAME_REG_FN. > > Would that work? Yes, that's indeed a better and cleaner solution, we've tested it and it works fine. I'll submit a new patchset. -- Y.