From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00071.outbound.protection.outlook.com [40.107.0.71]) by sourceware.org (Postfix) with ESMTPS id 19003385843A for ; Wed, 10 Aug 2022 10:53:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 19003385843A ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=KYnPvDLXYkVmmvNynqJpg4vtVo5RRdK43f8DGhIqTnB3f2P505tak/BzljP+moGDyw3++Y4tvYUTzTwPQeacfxf6XR0vG2LveXtj3kj7sehucSOrdtCbaQfpq5Rwe7I7OJbEHRDcLjzseynLoRvU/WBp2y6dAJKIhMM5IKJZnBzYsp8k/O9ZdfJ//Fv0xMEIfV4IXzx/Veo4gJguc4611vEu98uWpJEQWHzP535q9GaxI27CTbjaSv4jS9dD8owkQwvyKfgsGSm16rlSA3v7tOW4sr6TcEWbzbRo/ru1hgRoXzhDQLq2FEUfxVpVjiGrV13EVVngSri305zddEi+1w== 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=78+HYVsB55on2jvTfwL3wCBTYA251zYy6E39GIXQU+g=; b=IxtZU0yfkSRdCls+FAf1j2uN7ooxO/ifEyKTIt4GUbWIXkX6scnuXvvrU9bzZTmzLGA0eJCr1oGHNxgxflTeTLEHp1+c8GjvvFcnf+NZLuFEsaqWtzAKYbEHAI6LAZotSPduKb0JTym/m0AGFLmwhX7DhP8fX5wqJeEDPLexBEoZf/FtaaU/aMT6krtPeybjmdlpTiC2DaPgIY6zdFrjIhj185DE6fye7gVpC2Ci6fjrYAenBagYexnt4ATcGQe+Y4g4GI3QxTHFdxlmB3dYlMD6eE4tULNPUvXT+ilkGvQ+vu8bSIl5HE9zGzeZY8WdCoxavYyA8tGskXlEM3669w== 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 AS9PR06CA0260.eurprd06.prod.outlook.com (2603:10a6:20b:45f::25) by HE1PR0802MB2395.eurprd08.prod.outlook.com (2603:10a6:3:db::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.14; Wed, 10 Aug 2022 10:53:17 +0000 Received: from VE1EUR03FT015.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:45f:cafe::dd) by AS9PR06CA0260.outlook.office365.com (2603:10a6:20b:45f::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5525.11 via Frontend Transport; Wed, 10 Aug 2022 10:53:17 +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 VE1EUR03FT015.mail.protection.outlook.com (10.152.18.176) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.16 via Frontend Transport; Wed, 10 Aug 2022 10:53:16 +0000 Received: ("Tessian outbound 6a3290ff1310:v123"); Wed, 10 Aug 2022 10:53:16 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 8e6b927b9ce802d3 X-CR-MTA-TID: 64aa7808 Received: from 0be5cc41dd4d.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 33EADB3B-8571-4A02-81C9-25027B328A7A.1; Wed, 10 Aug 2022 10:53:10 +0000 Received: from EUR05-DB8-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 0be5cc41dd4d.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 10 Aug 2022 10:53:10 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dcf7DrnB4XXd2DKXe097TqoWz9LIk1GuAdrBw7GqIRGODe5TJSh+RrGS7vLwqkAJyucsDqL3v/m6f92GqzYO+v5d0VPnHQgtQ2VceiqRnTFIobUV1SHe8tVeB1rUfkB/iNvUzKBgiDfM4EwY+IZCdRIa2LZZUzd69ozQ7K91ZbXGwkuBRXZJlGThxE/HLoNY7XQkJM0ZwfzJ+e19mchWIuSwY3KIfXKeVmLxFMqKOaQIQeHXlnmEu5vQDnDFD9MnREhxluJuOjyKozjsUgsEWsUHNlXLhpe+783VLYnyLrM9hQ/jQGYRca2pFuwCOC64Dy1+boMvKkJFDvjLO7LniQ== 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=78+HYVsB55on2jvTfwL3wCBTYA251zYy6E39GIXQU+g=; b=eA60IWL4mhCWybJwoWtClPudu21z9dQBhdbEKpXIkaKEr/+5bzAVP6npC6aybLSPqq3CTy9l09yC09ahhlLdudl6Q/MVF/5NT/4+4FIkD0cXEW93hu8xzU+Fb1CBC5gyc+/9w2/oJfXtF+KwPEfNOCE16BslfhhLopK+zSoasYgoRVTix4zUJq+wugMCqrWlf9+DGvC7gts2vwKuzHJfOlVXDo0VpDZwR2oTwtUikd9HxjhLwlqSzpCtHyUsH5tHJT4hFieejgMSNdnfkgg9JavznqIOu5+ZKk0C4A+llde+t+uaWq0PJ5lQ6Gl92j0kAN4PM/U8R6nWZnQXzOJEaQ== 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 PR3PR08MB5708.eurprd08.prod.outlook.com (2603:10a6:102:84::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.14; Wed, 10 Aug 2022 10:53:08 +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; Wed, 10 Aug 2022 10:53:07 +0000 Message-ID: <8f9e2323-0207-6d3e-43b8-7916523bda89@arm.com> Date: Wed, 10 Aug 2022 11:53:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v2] gdb/arm: Cleanup of arm_m_exception_cache Content-Language: en-US To: Torbjorn SVENSSON , gdb-patches@sourceware.org References: <20220809153006.3249562-1-torbjorn.svensson@foss.st.com> <136b08eb-3680-b166-2ad8-1d8acdff6b34@arm.com> From: Luis Machado In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P265CA0270.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a1::18) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: ab6563d8-a727-4e1c-7487-08da7abe8809 X-MS-TrafficTypeDiagnostic: PR3PR08MB5708:EE_|VE1EUR03FT015:EE_|HE1PR0802MB2395: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: OYhukwxTLoV+xthPAuGn7Zy745nuClmtbYe+OZTqzRNb2upg1ZfGZNQqkyv11fCvWgopNZ1IbYrpiQb667dckbLr0Ys15vXU7Zri5wcNa6Ijxjnamp4zxeD2zUqR6ZQ1wKY8pn0Hw5ULiMVYOzGplCUsiYc3uOPS03VubqjD7FdDSaNZSuUsRRApEDAr+WqwAds8jEEVIq9QbZ67BXmX/3n1rLaEya3pLmxDclP3dXruPNem9Cpu6YxkdEpRU4nb+x4UHxyautDS2V7tO9ZN95F5axz8umLutUZFlYBm/StjWbf1JPWyN73ImOtHti7PxO8k4GGAt4FPh+X+wNFLpZiPZn63F0J8gRdn29rF453I/+VIEWW3HVnGRBP3PN2hae/LesNLrPg6g6F2Ib9vFDmreg6SGmmqtpWWqrCh31h/J4mOUME3pm0+65og32UrqoOMQhB+YktwjOV8loHMvyqOIvoNvl06/ir28kL3YJ5SQSP6nWRdEulJpUH0NRyUpBlIWCEDowcdfi/MtpOXGCx6ytY6aEI5iO6hx9+5H5dxChIoKJUN/q06iGmvmR0htAf/VV3SCrjcr577ZLZlXd6+x3X6KbBOD81WqMfFHXnLAuAkvTpSAiJwcLNpuJOp39dRO0g/jLMFZyRb9VsYjgweoVSq5me2Kw1dk18hjXwqOguFB38jJurw61tn9Y8IZBWp+phKdq4NEHt+YntUX1rwRCIBj76doLVvlH0xFp5ijDDY5rLcMRstNfeWYw8d3Ne2bvCZOPUjHPbHGXoWW6rlJEazeUpraGt5txg4c6tFrlF42YL9/i/1vF3JkTAVgDQRgJZYIz2Z+XWM+19qCA== 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)(366004)(136003)(346002)(39860400002)(66946007)(66476007)(44832011)(30864003)(8936002)(66556008)(38100700002)(8676002)(66574015)(478600001)(5660300002)(186003)(41300700001)(6506007)(6512007)(26005)(53546011)(316002)(2616005)(2906002)(31696002)(6486002)(36756003)(83380400001)(86362001)(31686004)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: PR3PR08MB5708 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: VE1EUR03FT015.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 216d5237-eb97-40a4-616b-08da7abe824d X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: UyGYhB1l0tq8Lqt8WZNK56vbMKDTM277xS136mTI/oXFQJUXFMJjFEoZw+G18ATvwvc8Knhpg07Ibr7CNAHu6GWuIPHMsfBXD31ec59WRY+i5rctXSye/NCZHUjLFr8+qbU6FI3lBXreCfabZy2PhlqPVaHYKdGESILCBRF7Gngmg1uGxQEsJfTCrDe4/JJhXEgqxZ5mGfl4wVCCz1TRGuxpMICjik9UYwAQk9AejcYejM4Zp70n08TYlI2phfVxzZtJ0ikx32TA7a3bjaw5Cqsq8l0ngmAp6JSK1PEUl5kXLKFjs6xMLJC0B51NFNG1U9OCGgtm5Zb3iLO1P86EZjqW+bzzlIzTyneVFg0s5uVol2Uq60UWP+Wxee0hhKTa1QmTDEgSirOy68C5J5oJh5ZtzX1yRPzFLAOxrA5p0QTapzVyaXKa0DmPiLJWkQCmKDg3SOjekJFZZUs8QvT7DzJrOGfRDn2xw4N0LcYgXQ9NSvJedOQEbXWCtYG4ZqCvSx83qb7TO9Lj613k2jZG2mhQkNVAz5nSLsFdSrDQpyUTafJRwsubbSA+B0mK4FUY9jjX9Hz6ZP0LM75lqE/doR/rcVixLtqTgZm1BZ0y3jXpePMTXa5Y9ed7M4p6aFuXIqHiljvTwjKIH7bBxxPsrqjcPlvj7yBKkRamRX11gFEt/E0Kyzr5+fXYlkeUZHHr5c19PsctuJtFDgQXpRqIcQaT/lKb/Hy1k3UlfWZoukiSvhGVEz82FVQkWTEitRikBOCKms9YDL0ZsI7Z13vCOHcFq8+1UCyplfi28N03yLm24Zrvv6H6NnWkCZ3I8WI+AXisvQRSweQNJhdBY3qmnA== 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)(136003)(346002)(376002)(39860400002)(396003)(46966006)(40470700004)(36840700001)(26005)(2906002)(83380400001)(36756003)(6512007)(53546011)(66574015)(186003)(336012)(31686004)(40480700001)(44832011)(41300700001)(82310400005)(2616005)(36860700001)(47076005)(82740400003)(81166007)(30864003)(5660300002)(8936002)(356005)(478600001)(70206006)(6506007)(6486002)(40460700003)(316002)(86362001)(31696002)(70586007)(8676002)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Aug 2022 10:53:16.9636 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: ab6563d8-a727-4e1c-7487-08da7abe8809 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: VE1EUR03FT015.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0802MB2395 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, FORGED_SPF_HELO, GIT_PATCH_0, KAM_DMARC_NONE, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY 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: Wed, 10 Aug 2022 10:53:25 -0000 On 8/9/22 18:13, Torbjorn SVENSSON wrote: > In addition to comments below, v3 will also contain a rephrased commit message to make it clear that GDB does not call assert, but instead errors out. > > On 8/9/22 18:15, Luis Machado wrote: >> On 8/9/22 16:30, Torbjörn SVENSSON 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. >>> >>> Signed-off-by: Torbjörn SVENSSON >>> --- >>>   gdb/arm-tdep.c | 380 ++++++++++++++++++++++++++----------------------- >>>   1 file changed, 204 insertions(+), 176 deletions(-) >>> >>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >>> index cf8b610a381..299c416fe52 100644 >>> --- a/gdb/arm-tdep.c >>> +++ b/gdb/arm-tdep.c >>> @@ -3346,19 +3346,7 @@ 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; >>> -  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); >>> @@ -3367,8 +3355,8 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>        describes which bits in LR that define which stack was used prior >>>        to the exception and if FPU is used (causing extended stack frame).  */ >>>   -  lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM); >>> -  sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM); >>> +  CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM); >>> +  CORE_ADDR sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM); >>>       /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers" >>>        states that LR is set to 0xffffffff on reset.  ARMv8-M Architecture >>> @@ -3381,9 +3369,22 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>         return cache; >>>       } >>>   -  fnc_return = (((lr >> 24) & 0xff) == 0xfe); >>> -  if (tdep->have_sec_ext && fnc_return) >>> +  /* Check FNC_RETURN indicator bits (24-31).  */ >>> +  bool fnc_return = (((lr >> 24) & 0xff) == 0xfe); >>> +  if (fnc_return) >>>       { >>> +      /* FNC_RETURN is only valid for targets with Security Extension.  */ >>> +      if (!tdep->have_sec_ext) >>> +    { >>> +      error (_("While unwinding an exception frame, found unexpected Link " >>> +           "Register value 0x%lx.  This should not happen and may be " >> >> You should use the %s format here and use phex to turn the 32-bit value to hex. >> >> Also, since this is checking explicitly for a value and the Security Extension, we should >> add that to the error message to make it more obvious what is failing. >> >> "While unwinding an exception frame, found unexpected Link Register value %s that requires the security extension, but the extension was not found or is disabled.  This should not happen and may be caused by corrupt data or a bug in GDB." > Ok, string updated in v3. >> >>> +           "caused by corrupt data or a bug in GDB."), lr); >>> + >>> +      /* Terminate any further stack unwinding by referring to self.  */ >>> +      arm_cache_set_active_sp_value (cache, tdep, sp); >>> +      return cache; >> >> Since you errored out, there's no use returning or executing any other statements after error. > Was uncertain if error() would bail or if it would just print the message and continue. Thanks for letting me know! >> >>> +    } >>> + >>>         if (!arm_unwind_secure_frames) >>>       { >>>         warning (_("Non-secure to secure stack unwinding disabled.")); >>> @@ -3393,7 +3394,7 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>         return cache; >>>       } >>>   -      xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM); >>> +      ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM); >>>         if ((xpsr & 0xff) != 0) >>>       /* Handler mode: This is the mode that exceptions are handled in.  */ >>>       arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum); >>> @@ -3401,7 +3402,7 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>       /* Thread mode: This is the normal mode that programs run in.  */ >>>       arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum); >>>   -      unwound_sp = arm_cache_get_prev_sp_value (cache, tdep); >>> +      CORE_ADDR unwound_sp = arm_cache_get_prev_sp_value (cache, tdep); >>>           /* Stack layout for a function call from Secure to Non-Secure state >>>        (ARMv8-M section B3.16): >>> @@ -3426,17 +3427,23 @@ arm_m_exception_cache (struct frame_info *this_frame) >>>       } >>>       /* Check EXC_RETURN indicator bits (24-31).  */ >>> -  exc_return = (((lr >> 24) & 0xff) == 0xff); >>> +  bool exc_return = (((lr >> 24) & 0xff) == 0xff); >>>     if (exc_return) >>>       { >>> +      int sp_regnum; >>> +      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); >>> +      bool process_stack_used = (bit (lr,2) != 0); >>>           if (tdep->have_sec_ext) >>>       { >>> -      secure_stack_used = ((lr & (1 << 6)) != 0); >>> -      default_callee_register_stacking = ((lr & (1 << 5)) != 0); >>> -      exception_domain_is_secure = ((lr & (1 << 0)) == 0); >>> +      secure_stack_used = (bit (lr,6) != 0); >> >> Could you please address the formatting issues? space before `(`, space after `,` > I copied an existing usage of bit(), but sure. Will be corrected in v3. >> >>> +      default_callee_register_stacking = (bit (lr,5) != 0);> + exception_domain_is_secure = (bit (lr,0) == 0); >>>           /* Unwinding from non-secure to secure can trip security >>>            measures.  In order to avoid the debugger being >>> @@ -3456,187 +3463,208 @@ 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); >>> - >>> -      /* 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; >>> +        sp_regnum = tdep->m_profile_msp_regnum; >>> +    } >>> + >>> +      /* Set the active SP regnum.  */ >>> +      arm_cache_switch_prev_sp (cache, tdep, sp_regnum); >>> + >>> +      /* Fetch the SP to use for this frame.  */ >>> +      CORE_ADDR 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 >>> +            |         R4        |   0x08    |  context when >>> +            +-------------------+           |  transitioning from >>> +            |      Reserved     |   0x04    |  Secure to Non-Secure >>> +            +-------------------+           | >>> +            |  Magic signature  |   0x00    | >>> +            +===================+         --+  <-- New SP */ >>> + >>> +      uint32_t sp_r0_offset = 0; >>> + >>> +      /* 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.  */ >>> +      bool extended_frame_used = (bit (lr,4) == 0); >>> +      if (extended_frame_used) >>> +    { >>> +      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); >> >> Space after `,` >> >>> + >>> +      /* 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.  */ >>> +      CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20; >>> +      for (int i = 0; i < 8; i++) >>> +        { >>> +          cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); >>> +          addr += 8; >>> +        } >>> +      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp >>> +                            + sp_r0_offset + 0x60); >>> + >>> +      if (tdep->have_sec_ext && !default_callee_register_stacking >>> +          && fpccr_ts) >>> +        { >>> +          /* Handle floating-point callee saved registers.  */ >>> +          addr = unwound_sp + sp_r0_offset + 0x68; >>> +          for (int i = 8; i < 16; i++) >>> +        { >>> +          cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); >>> +          addr += 8; >>> +        } >>>   -      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++) >>> +          arm_cache_set_active_sp_value (cache, tdep, >>> +                         unwound_sp + sp_r0_offset + 0xA8); >>> +        } >>> +      else >>>           { >>> -          cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset); >>> -          fpu_regs_stack_offset += 8; >>> +          /* Offset 0x64 is reserved.  */ >>> +          arm_cache_set_active_sp_value (cache, tdep, >>> +                         unwound_sp + sp_r0_offset + 0x68); >>>           } >>> - >>> -      arm_cache_set_active_sp_value (cache, tdep, >>> -                     unwound_sp + sp_r0_offset + 0xA8); >>>       } >>>         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.  */ >>> +      ULONGEST xpsr; >>> +      gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[ >>> +                             ARM_PS_REGNUM].addr (), 4, >>> +                             byte_order, &xpsr)); >>> +      if (bit (xpsr,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; >>>       } >>>   -  /* 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); >>> +  error (_("While unwinding an exception frame, found unexpected Link Register " >>> +       "value 0x%lx.  This should not happen and may be caused by corrupt " >>> +       "data or a bug in GDB."), lr); >> >> Same comment about using %s and phex as opposed to %lx. >> >> What does this case have that is different from the previous error? Does it contain an >> unrecognized LR value? If so, we should mention that explicitly to make it as helpful to >> the user as possible. > > The difference is that for the security extension check above, that is a wrongful state of the hardware. This case down here is when LR does not match an EXC_RETURN or FNC_RETURN pattern, thus is an inconsistency between the sniffer and the function that creates the cache object. > > The error case in the bottom should never happen as the sniffer has checked that LR matches either EXC_RETURN or FNC_RETURN, so it's more of a safe guard in case something is broken in GDB itself. The case with the security extension check should not happen neither, but I suppose it could happen if the data that is fetched from the target is corrupt in a certain way... > > Maybe the last one should be internal_error() instead of error()> From your description, it sounds like it. >> >> >>>   +  /* Terminate any further stack unwinding by referring to self.  */ >>> +  arm_cache_set_active_sp_value (cache, tdep, sp); >>>     return cache; >> >> This is dead code now. Nothing gets executed after error (). >> >>>   } >>