From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Ro/iId5q8mL4uSMAWB0awg (envelope-from ) for ; Tue, 09 Aug 2022 10:10:38 -0400 Received: by simark.ca (Postfix, from userid 112) id 7BE721E5EA; Tue, 9 Aug 2022 10:10:38 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=UyfdjQTS; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [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 CCD901E13B for ; Tue, 9 Aug 2022 10:10:36 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5297B3856DEA for ; Tue, 9 Aug 2022 14:10:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5297B3856DEA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1660054236; bh=Lxp70kc3rz5wr4vde7Gbg6/jZi0aiseKIkZJ4BP7OAk=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=UyfdjQTSLrrC0Gn6UD+K/6dvfMOmncatylo7weOv7mGpJS/TVTULjH5/9zHSH9dmO /X7Zaxq3QfJx5s2IPahAX7ozYLM6mFi9VWmSMttdULrbweElSnvw7c+EFFiC/TH1AW 5J91JThUO/Xp/ozU0S7AXy0mU4KFt63g6eAkp+Ms= Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-eopbgr130085.outbound.protection.outlook.com [40.107.13.85]) by sourceware.org (Postfix) with ESMTPS id B0993385AC22 for ; Tue, 9 Aug 2022 14:09:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B0993385AC22 ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=lLNG/i9ROKkQdnhpoVgVO9mNSO6VIaMaLUbHrVQcTPoxIeH2JDgeds9jfcIb2C+rC88wCzfMu4yFd6y//qs3DD/kAib6k16QhT2iEho41/rG3kHlv+xP4IQJADaNMQos/0ruS2kazhASHbeVqy6AuJXA70YVN1GucU7RorSbLOmO5nUtd4tul5QdkQaoAraijUwU8aBQ9aLEYxe1za5txWEBnpiDxRjkjfKej8aQKFDlkOMACtBzEd6MW/eCBaDd/Dy1JftucOzQM8oKn/Fdi9m4qpujebCVqiAAPHpvIE84uq8l+oVTa0Z5xelIv1s+JWQ4gnSW5iKxaF8RgpYhlw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Lxp70kc3rz5wr4vde7Gbg6/jZi0aiseKIkZJ4BP7OAk=; b=Alhrw1k8hRK5QqX9NMD/JKLTQuxuV8q87RhjC1fQ1aZv62ChGTDjxTnS7yxbxr7CyOKhF8PZVdK9wfOAn8oEuIxqBQCF/3AxXMspoAxY62NCQcAff9TS/HLw4MbxhxkEnuq9OEEGkTYWbIbUr6G4oSbtdlFVXyAqf5GaCo0tYXjjvhghMucmvHtvpBXzn90FgmsKiPynk/Wkn+5p2GbqVb4g3TP7hQ3Hj2rVBEDtjZY4RUVPyDIk4WYquuO/n3RPIewVv/jHzGDRsh2rerwtMJ48SV8+SNC3k2vcLoYACm+a9tlaxVfCXoCIcfMdv81KSLAeGOqX+O4OY/zOwARbSw== ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=sourceware.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com]) Received: from AS9PR06CA0137.eurprd06.prod.outlook.com (2603:10a6:20b:467::29) by DB7PR08MB3226.eurprd08.prod.outlook.com (2603:10a6:5:22::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.17; Tue, 9 Aug 2022 14:09:51 +0000 Received: from AM5EUR03FT043.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:467:cafe::89) by AS9PR06CA0137.outlook.office365.com (2603:10a6:20b:467::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.15 via Frontend Transport; Tue, 9 Aug 2022 14:09:51 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT043.mail.protection.outlook.com (10.152.17.43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.16 via Frontend Transport; Tue, 9 Aug 2022 14:09:51 +0000 Received: ("Tessian outbound 63c09d5d38ac:v123"); Tue, 09 Aug 2022 14:09:51 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: eea16988e551a969 X-CR-MTA-TID: 64aa7808 Received: from f2d28d758268.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 4390C0DB-2DB5-44B3-8E38-024276DF644E.1; Tue, 09 Aug 2022 14:09:43 +0000 Received: from EUR04-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id f2d28d758268.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 09 Aug 2022 14:09:43 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SzClzdwinX/nbT7gdbOf1cuR2cO/07ySJuwpHhXAWxUjV2aOSY+8JSgKWp1k7UpsNZjNq+qSZ+rWOaoiqZDx9zYh/rJQKe9EYGzUcIVt0WC6RDhLimPWIH4/w0j1U59nVXJ16h3K2qT5TsadLSkO2q7WpQXf62JtTvGmIf1mk7xsz+9fooGiZxy50U1qxtUUupd2S4rpCNjaf8jhqGVqQBDrnlDLdGdyPuJ0aw9YO5h8S7pYGsTNfVP5gY2XyOFvFu984KvJw3otI8NGPxu+4fO66ALfC6u3Ge3TLlvz1sEB3dw2q6YqPHrqIet46paWM2MgLZx+lp6JZhDj4YB1Rg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Lxp70kc3rz5wr4vde7Gbg6/jZi0aiseKIkZJ4BP7OAk=; b=GKHnuMM3hifaT3SZcmXfOSG1kj2CHy/wboCByaLXJguq/WINAWLtUHdq+hWU0Y+akn4QE9/D5gHM00I5v9yLxfNX5/NmenwaBHw/dlDy2E5jL17b6vlNHACCPw6r53T1Noiv6wJw1S8kAb9SzPsBBPxSKTNXDl3889/UUc3sREdlWVM1JQI76hE2fo9gLUYtC9eUIyGgweLcOIhf4I3irbXZXGwiWqfCZf9igy6oGW0TEp1qD4ZoO9uCTSIKcEulidhP2uQhX4gcZITAOA5LaGANq4UL73xbQbVGSPTpcxyS3WWqzjo9412f+It+JQGEN6Oq4AjnxBkvShNhbBf8MA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) by GVXPR08MB7824.eurprd08.prod.outlook.com (2603:10a6:150:1::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.21; Tue, 9 Aug 2022 14:09:41 +0000 Received: from VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::cc64:9170:b12d:de8]) by VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::cc64:9170:b12d:de8%4]) with mapi id 15.20.5504.019; Tue, 9 Aug 2022 14:09:41 +0000 Message-ID: <6d86ec67-4ea9-5777-d82c-94efb4cff179@arm.com> Date: Tue, 9 Aug 2022 15:09:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] gdb/arm: Cleanup of arm_m_exception_cache Content-Language: en-US To: Torbjorn SVENSSON , gdb-patches@sourceware.org References: <20220808075624.3126293-1-torbjorn.svensson@foss.st.com> <3e0260c4-8bfe-b398-2fd2-3af15a55b3d5@arm.com> <73982902-fa33-aa32-5bb4-14e92979623b@foss.st.com> In-Reply-To: <73982902-fa33-aa32-5bb4-14e92979623b@foss.st.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P265CA0483.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:13a::8) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: b9c9f184-dc4e-4b2c-1fdd-08da7a10d383 X-MS-TrafficTypeDiagnostic: GVXPR08MB7824:EE_|AM5EUR03FT043:EE_|DB7PR08MB3226:EE_ x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: szoToXpviby9KVgeb1yjH2PSp+B00GuA1nzq1pa1F5ieyY8V/n65CwPCOxJWKILuvnPfXnNnSZjpaBy1kP8Q7rFnTUuhLcm70meKlcK1043WPygxJSReUw7sa6OHM5PMx9ec/57527eeSOEkvN2ZUY5IceNTyO9q7gbyQIc/xkd3sxbbDgeKCh7mS5LvhZ8MrDt76N7RzaKSpEbbRm6sKcI0Uj54ZbZqZ/e8KCTY4PYXe6a8xW06yUE/WrBH8yWwbdwnMNfvtYrT2wg0+g+Ak34mUAp7lN99M86NnTm5FKSQrD8OE/GBYoAk16a1Dxgp4jK9/+nVg0pajSNAw3Ve8xn8qF7t+xM/96yWQLKsN6c48UPDvpXD7FATAwhdn+E7IrNb7ArsIHUsa1wcTwun7fuXYZx9PKoqPouki9aYrSmfmRZJTs+3OiLdfMEq3oRMbl280ghIcQdIWLfPixZBCPFrcKnEsZp6MYE2oUGaN8eViGuIfZN2s0WWgsDZqCmqjm+oRG45aC5SmkkMFZLTfLp9fFaEbPTCDfTkXrEg1/BJQW5dVmBWYcECiL6yRtajSfbnrbI1xHsY/mW/bmQsYOhIgA3MA1kaIsPS/kRpUwInDIVY9rgftXUXD0UFmYzXVxca5njL5fR6UvhyEY0CMnUOwlHvYK04MPbQ3El9efEhlPwHFulRAugAKRUCyt7emwTji/7UbCBuZuz94LL4b4LI7P43R/RI91CjTue518eOr/U7CUU5Q8lSfNJdwKlpTTVG0drpwVP3rF2LbKtPicUUUKQvi4NhCTCKZxGr0Pc7nnzgTaDsHfp6/UH/TvgnDWGi0I58e0amdmgOM3fovg== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR08MB3919.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230016)(4636009)(376002)(396003)(39860400002)(136003)(346002)(366004)(5660300002)(8936002)(2616005)(186003)(30864003)(36756003)(31686004)(53546011)(38100700002)(44832011)(316002)(86362001)(8676002)(66476007)(66556008)(31696002)(66946007)(478600001)(6486002)(6512007)(26005)(41300700001)(2906002)(83380400001)(6506007)(66574015)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR08MB7824 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT043.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 7e8e0293-a36d-4c7b-b9c6-08da7a10cd82 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: g8yVX22SYKLHFDBaJfY+himE1FCQvfve0qDbnzN+yP++lU49Q0ARfGCCxcY8+6aCdFzb5GVduY6Me8+1YPNz0vJomP+nm9DC+4319WWApnA1UlX9uMvhxpw+j57Ek2bcps5bYBdk0r1Sa+wgiu+2u2OC03PbibzE2MDirbQkCy3nslOG7CY56/cGoRvEKMdUtjGBCsTO7RQ5G6zfjhiAWHhyb57/KYjRU7BUg0GjkSqHNdYKoRYdVXCmfilIlbXMaSnzpH+WqLdefQgOZQibRbREFhi6HX5QyoIcMcfXQRmibqLggMOf2qLeOw/6sISGipCERQJkuWrI9ib193taKR5wrq184cqjlWSKfwaMQtFh1XDyBQQZtuAXuH+FifoJgeBFJh3BpGQZ5SuwMTGwMpKsqWly2Bb1RXto6y/gBzZHxpgLEHIE3CsupVJI9O34a/WF9qva4KXRx71r+FXvCeRrwJhDqkR5qUagV50OhGZHoEcx1oPZNp5xjmv2+NM+LPltItgG7rfQIQzj3zNRowmHXPmdeAoz9TmbiX4Eb6AcIEETU5PXL8nPKrMGpIiaUAM9FdsIDIdg1YkqfT1MT8m12dnPHVHqiRSbljMnrCJhqDS6SKExaLJFbrwyyrzuxN+FRN6z44Lo1AkKRCBcy/NO8rXPZo5cke8AyKpLiIHFTMtCJ/iSutujfu4USzTy1rlVFSimRVjc4Dlfr6rx/kP5UAIXr2a6WfLkss/lMXrGGNU9iHR+EmgvGLdsjLfWr74wci5ttHdJJVRF4mYRvf+EYvSxcloYPIKKj9U3IxxT/8uzC4bNi8WSsUYaK6GANXlj4/DmLbfabzUGFqpwVQ== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(13230016)(4636009)(376002)(346002)(39860400002)(136003)(396003)(40470700004)(36840700001)(46966006)(81166007)(356005)(31686004)(82740400003)(186003)(2616005)(6486002)(336012)(66574015)(47076005)(26005)(6512007)(6506007)(53546011)(41300700001)(82310400005)(36756003)(40480700001)(31696002)(478600001)(86362001)(8676002)(40460700003)(316002)(5660300002)(70206006)(36860700001)(2906002)(83380400001)(70586007)(8936002)(30864003)(44832011)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2022 14:09:51.2060 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: b9c9f184-dc4e-4b2c-1fdd-08da7a10d383 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT043.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR08MB3226 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+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 8/9/22 14:43, Torbjorn SVENSSON wrote: > Hello, > > Thanks for the review. Please see my comments below. > > On 8/9/22 15:02, Luis Machado wrote: >> Hi, >> >> On 8/8/22 08:56, Torbjörn SVENSSON via Gdb-patches wrote: >>> With this change, only valid content of LR is accepted for the current >>> target. If the content for LR is anything but EXC_RETURN or FNC_RETURN >>> will cause GDB to assert since it's an invalid state for the unwinder. >>> FNC_RETURN pattern requires Security Extensions to be enabled or GDB >>> will assert due to the bad state of the unwinder. >> >> >> If we have corrupt data, do we risk running into this assertion? > > See my comments below on individual cases. > >> >>> >>> Signed-off-by: Torbjörn SVENSSON >>> --- >>>   gdb/arm-tdep.c | 343 ++++++++++++++++++++++++++----------------------- >>>   1 file changed, 183 insertions(+), 160 deletions(-) >>> >>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >>> index 7d8d040f8f1..ad5ada39aea 100644 >>> --- a/gdb/arm-tdep.c >>> +++ b/gdb/arm-tdep.c >>> @@ -3345,19 +3345,13 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>   { >>>     struct gdbarch *gdbarch = get_frame_arch (this_frame); >>>     arm_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); >>> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >>>     struct arm_prologue_cache *cache; >>>     CORE_ADDR lr; >>>     CORE_ADDR sp; >>>     CORE_ADDR unwound_sp; >>> -  uint32_t sp_r0_offset = 0; >>> -  LONGEST xpsr; >>> -  uint32_t exc_return; >>> +  ULONGEST xpsr; >>> +  bool exc_return; >>>     bool fnc_return; >>> -  uint32_t extended_frame_used; >>> -  bool secure_stack_used = false; >>> -  bool default_callee_register_stacking = false; >>> -  bool exception_domain_is_secure = false; >>>       cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache); >>>     arm_cache_init (cache, this_frame); >>> @@ -3380,9 +3374,13 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>         return cache; >>>       } >>>   +  /* Check FNC_RETURN indicator bits (24-31).  */ >>>     fnc_return = (((lr >> 24) & 0xff) == 0xfe); >>> -  if (tdep->have_sec_ext && fnc_return) >>> +  if (fnc_return) >>>       { >>> +      /* FNC_RETURN is only valid for targets with Security Extension.  */ >>> +      gdb_assert (tdep->have_sec_ext); >>> + >> >> An assertion is a bit of a strong hand here. It will crash GDB, essentially. Should we go for an >> error instead? That will stop the unwinder dead in its tracks and return. >> >> Unwinders may get corrupt data, so it is hard to rule out issues, even if GDB is doing the right thing. > > Okay, lets do an error message instead and abort the unwind. > > What do you think the error message should say? > > "FNC_RETURN pattern is only valid for targets with Security Extension." - is that okay or do you have any other phase in mind? > For the sake of the user, I think something less low level. "While unwinding an exception frame, found unexpected Link Register value "? Given this is a showstopper, might as well add more to it. "This should not happen and may be caused by corrupt data or a bug in GDB". What do you think? >> >>>         if (!arm_unwind_secure_frames) >>>       { >>>         warning (_("Non-secure to secure stack unwinding disabled.")); >>> @@ -3428,6 +3426,14 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>     exc_return = (((lr >> 24) & 0xff) == 0xff); >>>     if (exc_return) >>>       { >>> +      int sp_regnum; >>> +      uint32_t sp_r0_offset = 0; >>> +      bool extended_frame_used; >> >> extended_frame_used could be defined further down or removed. >> >> Since we're touching this code, it would be nice to get the variable declarations closer to >> where they're used. This makes things cleaner now that we can do it. >> >>> +      bool secure_stack_used = false; >>> +      bool default_callee_register_stacking = false; >>> +      bool exception_domain_is_secure = false; >>> +      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >>> + >>>         /* Check EXC_RETURN bit SPSEL if Main or Thread (process) stack used.  */ >>>         bool process_stack_used = ((lr & (1 << 2)) != 0); >>>   @@ -3455,188 +3461,205 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>           { >>>             if (secure_stack_used) >>>           /* Secure thread (process) stack used, use PSP_S as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum); >>> +        sp_regnum = tdep->m_profile_psp_s_regnum; >>>             else >>>           /* Non-secure thread (process) stack used, use PSP_NS as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_ns_regnum); >>> +        sp_regnum = tdep->m_profile_psp_ns_regnum; >>>           } >>>         else >>>           { >>>             if (secure_stack_used) >>>           /* Secure main stack used, use MSP_S as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum); >>> +        sp_regnum = tdep->m_profile_msp_s_regnum; >>>             else >>>           /* Non-secure main stack used, use MSP_NS as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_ns_regnum); >>> +        sp_regnum = tdep->m_profile_msp_ns_regnum; >>>           } >>>       } >>>         else >>>       { >>>         if (process_stack_used) >>>           /* Thread (process) stack used, use PSP as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_regnum); >>> +        sp_regnum = tdep->m_profile_psp_regnum; >>>         else >>>           /* Main stack used, use MSP as SP.  */ >>> -        arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_regnum); >>> -    } >>> -    } >>> - >>> -  /* Fetch the SP to use for this frame.  */ >>> -  unwound_sp = arm_cache_get_prev_sp_value (cache, tdep); >>> - >>> -  /* Exception entry context stacking are described in ARMv8-M (section B3.19) >>> -     and ARMv7-M (sections B1.5.6 and B1.5.7) Architecture Reference Manuals. >>> - >>> -     The following figure shows the structure of the stack frame when Security >>> -     and Floating-point extensions are present. >>> - >>> -                          SP Offsets >>> -            Without                         With >>> -          Callee Regs                    Callee Regs >>> -                                    (Secure -> Non-Secure) >>> -                    +-------------------+ >>> -             0xA8   |                   |   0xD0 >>> -                    +===================+         --+  <-- Original SP >>> -             0xA4   |        S31        |   0xCC    | >>> -                    +-------------------+           | >>> -                             ...                    | Additional FP context >>> -                    +-------------------+           | >>> -             0x68   |        S16        |   0x90    | >>> -                    +===================+         --+ >>> -             0x64   |      Reserved     |   0x8C    | >>> -                    +-------------------+           | >>> -             0x60   |       FPSCR       |   0x88    | >>> -                    +-------------------+           | >>> -             0x5C   |        S15        |   0x84    |  FP context >>> -                    +-------------------+           | >>> -                             ...                    | >>> -                    +-------------------+           | >>> -             0x20   |         S0        |   0x48    | >>> -                    +===================+         --+ >>> -             0x1C   |       xPSR        |   0x44    | >>> -                    +-------------------+           | >>> -             0x18   |  Return address   |   0x40    | >>> -                    +-------------------+           | >>> -             0x14   |      LR(R14)      |   0x3C    | >>> -                    +-------------------+           | >>> -             0x10   |        R12        |   0x38    |  State context >>> -                    +-------------------+           | >>> -             0x0C   |         R3        |   0x34    | >>> -                    +-------------------+           | >>> -                             ...                    | >>> -                    +-------------------+           | >>> -             0x00   |         R0        |   0x28    | >>> -                    +===================+         --+ >>> -                    |        R11        |   0x24    | >>> -                    +-------------------+           | >>> -                             ...                    | >>> -                    +-------------------+           | Additional state context >>> -                    |         R4        |   0x08    |  when transitioning from >>> -                    +-------------------+           |  Secure to Non-Secure >>> -                    |      Reserved     |   0x04    | >>> -                    +-------------------+           | >>> -                    |  Magic signature  |   0x00    | >>> -                    +===================+         --+  <-- New SP  */ >>> - >>> -  /* With the Security extension, the hardware saves R4..R11 too.  */ >>> -  if (exc_return && tdep->have_sec_ext && secure_stack_used >>> -      && (!default_callee_register_stacking || exception_domain_is_secure)) >>> -    { >>> -      /* Read R4..R11 from the integer callee registers.  */ >>> -      cache->saved_regs[4].set_addr (unwound_sp + 0x08); >>> -      cache->saved_regs[5].set_addr (unwound_sp + 0x0C); >>> -      cache->saved_regs[6].set_addr (unwound_sp + 0x10); >>> -      cache->saved_regs[7].set_addr (unwound_sp + 0x14); >>> -      cache->saved_regs[8].set_addr (unwound_sp + 0x18); >>> -      cache->saved_regs[9].set_addr (unwound_sp + 0x1C); >>> -      cache->saved_regs[10].set_addr (unwound_sp + 0x20); >>> -      cache->saved_regs[11].set_addr (unwound_sp + 0x24); >>> -      sp_r0_offset = 0x28; >>> -    } >>> - >>> -  /* The hardware saves eight 32-bit words, comprising xPSR, >>> -     ReturnAddress, LR (R14), R12, R3, R2, R1, R0.  See details in >>> -     "B1.5.6 Exception entry behavior" in >>> -     "ARMv7-M Architecture Reference Manual".  */ >>> -  cache->saved_regs[0].set_addr (unwound_sp + sp_r0_offset); >>> -  cache->saved_regs[1].set_addr (unwound_sp + sp_r0_offset + 0x04); >>> -  cache->saved_regs[2].set_addr (unwound_sp + sp_r0_offset + 0x08); >>> -  cache->saved_regs[3].set_addr (unwound_sp + sp_r0_offset + 0x0C); >>> -  cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x10); >>> -  cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x14); >>> -  cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x18); >>> -  cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x1C); >>> - >>> -  /* Check EXC_RETURN bit FTYPE if extended stack frame (FPU regs stored) >>> -     type used.  */ >>> -  extended_frame_used = ((lr & (1 << 4)) == 0); >>> -  if (exc_return && extended_frame_used) >>> -    { >>> -      int i; >>> -      int fpu_regs_stack_offset; >>> -      ULONGEST fpccr; >>> - >>> -      /* Read FPCCR register.  */ >>> -      gdb_assert (safe_read_memory_unsigned_integer (FPCCR, >>> - ARM_INT_REGISTER_SIZE, >>> - byte_order, &fpccr)); >>> -      bool fpccr_ts = bit (fpccr,26); >>> +        sp_regnum = tdep->m_profile_msp_regnum; >>> +    } >>>   -      /* This code does not take into account the lazy stacking, see "Lazy >>> -     context save of FP state", in B1.5.7, also ARM AN298, supported >>> -     by Cortex-M4F architecture. >>> -     To fully handle this the FPCCR register (Floating-point Context >>> -     Control Register) needs to be read out and the bits ASPEN and LSPEN >>> -     could be checked to setup correct lazy stacked FP registers. >>> -     This register is located at address 0xE000EF34.  */ >>> +      /* Set the active SP regnum.  */ >>> +      arm_cache_switch_prev_sp (cache, tdep, sp_regnum); >>>   -      /* Extended stack frame type used.  */ >>> -      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20; >>> -      for (i = 0; i < 8; i++) >>> -    { >>> -      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset); >>> -      fpu_regs_stack_offset += 8; >>> -    } >>> -      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60); >>> -      fpu_regs_stack_offset += 4; >>> +      /* Fetch the SP to use for this frame.  */ >>> +      unwound_sp = arm_cache_get_prev_sp_value (cache, tdep); >>>   -      if (tdep->have_sec_ext && !default_callee_register_stacking && fpccr_ts) >>> +      /* Exception entry context stacking are described in ARMv8-M (section >>> +     B3.19) and ARMv7-M (sections B1.5.6 and B1.5.7) Architecture Reference >>> +     Manuals. >>> + >>> +     The following figure shows the structure of the stack frame when >>> +     Security and Floating-point extensions are present. >>> + >>> +                  SP Offsets >>> +        Without                         With >>> +          Callee Regs                    Callee Regs >>> +                    (Secure -> Non-Secure) >>> +            +-------------------+ >>> +         0xA8   |                   |   0xD0 >>> +            +===================+         --+  <-- Original SP >>> +         0xA4   |        S31        |   0xCC    | >>> +            +-------------------+           | >>> +                 ...                    |  Additional FP context >>> +            +-------------------+           | >>> +         0x68   |        S16        |   0x90    | >>> +            +===================+         --+ >>> +         0x64   |      Reserved     |   0x8C    | >>> +            +-------------------+           | >>> +         0x60   |       FPSCR       |   0x88    | >>> +            +-------------------+           | >>> +         0x5C   |        S15        |   0x84    |  FP context >>> +            +-------------------+           | >>> +                 ...                    | >>> +            +-------------------+           | >>> +         0x20   |         S0        |   0x48    | >>> +            +===================+         --+ >>> +         0x1C   |       xPSR        |   0x44    | >>> +            +-------------------+           | >>> +         0x18   |  Return address   |   0x40    | >>> +            +-------------------+           | >>> +         0x14   |      LR(R14)      |   0x3C    | >>> +            +-------------------+           | >>> +         0x10   |        R12        |   0x38    |  State context >>> +            +-------------------+           | >>> +         0x0C   |         R3        |   0x34    | >>> +            +-------------------+           | >>> +                 ...                    | >>> +            +-------------------+           | >>> +         0x00   |         R0        |   0x28    | >>> +            +===================+         --+ >>> +            |        R11        |   0x24    | >>> +            +-------------------+           | >>> +                 ...                    | >>> +            +-------------------+           |  Additional state >>> +            |         R4        |   0x08    |  context when >>> +            +-------------------+           |  transitioning from >>> +            |      Reserved     |   0x04    |  Secure to Non-Secure >>> +            +-------------------+           | >>> +            |  Magic signature  |   0x00    | >>> +            +===================+         --+  <-- New SP */ >>> + >>> +      /* With the Security extension, the hardware saves R4..R11 too.  */ >>> +      if (tdep->have_sec_ext && secure_stack_used >>> +      && (!default_callee_register_stacking || exception_domain_is_secure)) >>> +    { >>> +      /* Read R4..R11 from the integer callee registers.  */ >>> +      cache->saved_regs[4].set_addr (unwound_sp + 0x08); >>> +      cache->saved_regs[5].set_addr (unwound_sp + 0x0C); >>> +      cache->saved_regs[6].set_addr (unwound_sp + 0x10); >>> +      cache->saved_regs[7].set_addr (unwound_sp + 0x14); >>> +      cache->saved_regs[8].set_addr (unwound_sp + 0x18); >>> +      cache->saved_regs[9].set_addr (unwound_sp + 0x1C); >>> +      cache->saved_regs[10].set_addr (unwound_sp + 0x20); >>> +      cache->saved_regs[11].set_addr (unwound_sp + 0x24); >>> +      sp_r0_offset = 0x28; >>> +    } >>> + >>> +      /* The hardware saves eight 32-bit words, comprising xPSR, >>> +     ReturnAddress, LR (R14), R12, R3, R2, R1, R0.  See details in >>> +     "B1.5.6 Exception entry behavior" in >>> +     "ARMv7-M Architecture Reference Manual".  */ >>> +      cache->saved_regs[0].set_addr (unwound_sp + sp_r0_offset); >>> +      cache->saved_regs[1].set_addr (unwound_sp + sp_r0_offset + 0x04); >>> +      cache->saved_regs[2].set_addr (unwound_sp + sp_r0_offset + 0x08); >>> +      cache->saved_regs[3].set_addr (unwound_sp + sp_r0_offset + 0x0C); >>> +      cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + sp_r0_offset >>> +                         + 0x10); >>> +      cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + sp_r0_offset >>> +                         + 0x14); >>> +      cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + sp_r0_offset >>> +                         + 0x18); >>> +      cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + sp_r0_offset >>> +                         + 0x1C); >>> + >>> +      /* Check EXC_RETURN bit FTYPE if extended stack frame (FPU regs stored) >>> +     type used.  */ >>> +      extended_frame_used = ((lr & (1 << 4)) == 0); >> >> Declaring extended_frame_used here would be nice. That's the only place where it is >> used. >> >> Alternatively, you could check for bit 4 in the conditional expression below and add a >> comment explaining this is checking if the extended frame is being used. > I'll move the declaration down here as I think the variable is helpful if every debugging this code. >> >>> +      if (extended_frame_used) >>>       { >>> -      /* Handle floating-point callee saved registers.  */ >>> -      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x68; >>> -      for (i = 8; i < 16; i++) >>> -        { >>> -          cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset); >>> +      int i; >>> +      int fpu_regs_stack_offset; >>> +      ULONGEST fpccr; >>> + >>> +      /* Read FPCCR register.  */ >>> +      gdb_assert (safe_read_memory_unsigned_integer (FPCCR, >>> +                             ARM_INT_REGISTER_SIZE, >>> +                             byte_order, &fpccr)); >>> +      bool fpccr_ts = bit (fpccr,26); >>> + >>> +      /* This code does not take into account the lazy stacking, see "Lazy >>> +         context save of FP state", in B1.5.7, also ARM AN298, supported >>> +         by Cortex-M4F architecture. >>> +         To fully handle this the FPCCR register (Floating-point Context >>> +         Control Register) needs to be read out and the bits ASPEN and >>> +         LSPEN could be checked to setup correct lazy stacked FP registers. >>> +         This register is located at address 0xE000EF34.  */ >>> + >>> +      /* Extended stack frame type used.  */ >>> +      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20; >>> +      for (i = 0; i < 8; i++) >>> +        { >>> +          cache->saved_regs[ARM_D0_REGNUM + i] >>> +        .set_addr (fpu_regs_stack_offset); >>>             fpu_regs_stack_offset += 8; >>>           } >>> +      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp >>> +                            + sp_r0_offset + 0x60); >>> +      fpu_regs_stack_offset += 4; >>>   -      arm_cache_set_active_sp_value (cache, tdep, >>> -                     unwound_sp + sp_r0_offset + 0xA8); >>> +      if (tdep->have_sec_ext && !default_callee_register_stacking >>> +          && fpccr_ts) >>> +        { >>> +          /* Handle floating-point callee saved registers.  */ >>> +          fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x68; >>> +          for (i = 8; i < 16; i++) >>> +        { >>> +          cache->saved_regs[ARM_D0_REGNUM + i] >>> +            .set_addr (fpu_regs_stack_offset); >>> +          fpu_regs_stack_offset += 8; >>> +        } >>> + >>> +          arm_cache_set_active_sp_value (cache, tdep, >>> +                         unwound_sp + sp_r0_offset + 0xA8); >>> +        } >>> +      else >>> +        { >>> +          /* Offset 0x64 is reserved.  */ >>> +          arm_cache_set_active_sp_value (cache, tdep, >>> +                         unwound_sp + sp_r0_offset + 0x68); >>> +        } >>>       } >>>         else >>>       { >>> -      /* Offset 0x64 is reserved.  */ >>> +      /* Standard stack frame type used.  */ >>>         arm_cache_set_active_sp_value (cache, tdep, >>> -                     unwound_sp + sp_r0_offset + 0x68); >>> +                     unwound_sp + sp_r0_offset + 0x20); >>>       } >>> -    } >>> -  else >>> -    { >>> -      /* Standard stack frame type used.  */ >>> -      arm_cache_set_active_sp_value (cache, tdep, >>> -                     unwound_sp + sp_r0_offset + 0x20); >>> -    } >>>   -  /* If bit 9 of the saved xPSR is set, then there is a four-byte >>> -     aligner between the top of the 32-byte stack frame and the >>> -     previous context's stack pointer.  */ >>> -  if (safe_read_memory_integer (unwound_sp + sp_r0_offset + 0x1C, 4, >>> -                byte_order, &xpsr) >>> -      && (xpsr & (1 << 9)) != 0) >>> -    arm_cache_set_active_sp_value (cache, tdep, >>> -                   arm_cache_get_prev_sp_value (cache, tdep) + 4); >>> +      /* If bit 9 of the saved xPSR is set, then there is a four-byte >>> +     aligner between the top of the 32-byte stack frame and the >>> +     previous context's stack pointer.  */ >>> +      gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[ >>> +                             ARM_PS_REGNUM].addr (), 4, >>> +                             byte_order, &xpsr)); >>> +      if ((xpsr & (1 << 9)) != 0) >>> +    { >>> +      CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4; >>> +      arm_cache_set_active_sp_value (cache, tdep, new_sp); >>> +    } >>>   -  return cache; >>> +      return cache; >>> +    } >>> + >>> +  gdb_assert_not_reached ("Invalid LR contet"); >> >> "content". Again, this will crash GDB. Should we error out instead? > > This place is different from the security extension check above. > > When this line is reached, there is an internal error in the GDB source code as the sniffer has decided that $lr contains an EXC_PATTERN or FNC_PATTERN, but if you reach this line, it would mean that the sniffer and this function are out of sync. > > Do you think it's sane to assert or do you still want to have an error message instead? > Given unwinders deal with potentially corrupt data, an error message here would allow a user to still interact with GDB in a limited way. They would be able to see register values, memory etc. If we assert, that will kill GDB and prevent any form of interaction. Unless we are sure this is a situation that only happens if GDB is faulty, I'd say we should error out with a descriptive message. >> >>>   } >>>     /* Implementation of function hook 'this_id' in >>