From patchwork Thu Dec 28 19:33:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lancelot SIX X-Patchwork-Id: 82940 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6F6DC3858CD1 for ; Thu, 28 Dec 2023 19:34:57 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2050.outbound.protection.outlook.com [40.107.220.50]) by sourceware.org (Postfix) with ESMTPS id F0F6E3858D38 for ; Thu, 28 Dec 2023 19:34:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F0F6E3858D38 Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=amd.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org F0F6E3858D38 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.220.50 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1703792070; cv=pass; b=HaPHfn0HOml0RXpc74w7S4W2IjV/Hj6/GCPv89YCzWjCHQg7SkAs1JRHlkUVGJdEfW6RUklO+5EKLYoJfnPw6LWp+D6l4HfHSnKmQEBifqegnKqS3K9DdvP5rGsbzK+It1tDhCgEbIHoQxYk9bKn8lmFsKzI/mDR8SHZzHtWPd0= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1703792070; c=relaxed/simple; bh=czGuICkgvktDc3RNr12MN0IXyw0pD1MFBWmTh+hzMvA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ltjsDnHB5F6CdWPWI5btAKbpgrhn5aWz9w2oV1RqDvebxFq1bpVmFaa5+ceLhYBdSuPs0Vg03YNruLcOY642oTIv4XaNLlSIGzzOXtXX3M5jgAjstCsDWCOgxzW/HYEp0tSskAtv3CqgE3QxrPwWYG0j0LywydGJX4B09wxX/3s= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=B4/on87+ejnudc/ErtQsiVo9DUDdD0aSa237DfFU9DXcST4+IfX8x9ebHScMsMD+tfLNp2QINw24mmz535dxQXfCphyWvgUU9aCS5ABRKp6bMAM8WRvMzhm1otFXsQxOZIB1VdYD8o+fwNK8XlehVCXRr5yyZrkGCAV/QgpTrZYXI7xKu4j2A/0rgf4nBybELoPXhcufhklb4QVlll08WFOtid8Q/GAj8nqI2+Uq7gogJlTfpNbgppsa+IWqGOsGe9e22FejLBFGpoUEyuNz4dAqZjPWeS8X2HBTOnikrKfAuTiy4plZY9P0XJc/Zqft3FWtDDSc4L4T00lcHxdv0Q== 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=F7BBiGlHXJpG3L/mDEA25NZuP9w+GPEvcIOrP+q2coU=; b=GHa/vkvrgdk4S5CQuztF6g7dD1c7vHwq49Leoa/pKbg6KAiW6yU1Cl26rAFA4o1h4zLLkxtngkFyW4epaFbCpsUMrZt41ac6BuXz0em6NX3ZV8droFDK/GEn3P3zi8xK+wOgk7fouziun9a+izVQsz81nOfUfc845dZOIFNm9gffYQeNiAmU00FqhwpRYgQs0iVUf2TeScOcdV97Gh0t8rC6+7oRK5wUxIvRDsLgVxDnqSXLNehgZa7K++OliCFpE93uikHTzwKSyPWCn/gExZ3zkVkzL0TizGefS1SYKfvoGp2lX8KaRuV0COF6P/Oxwc9kWiDYPnmzpxQ2l6sf3w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=sourceware.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=F7BBiGlHXJpG3L/mDEA25NZuP9w+GPEvcIOrP+q2coU=; b=AveoV1rz1JJZ/nRou05087ilYQ6vCLRLjKeWbVZAgnVHZr0DjzrxpyRRYeVHXkbYHSWoX/nD85HfxM8vTFMXQPeTTminBnKVbvPSKLZkKVlCH5Jkub9wcOzmd1i95hgNnRDaQmBSjC4TFH534hv8OXdGHO3B2/1B+D9CX+OIyeU= Received: from DM6PR06CA0064.namprd06.prod.outlook.com (2603:10b6:5:54::41) by MN2PR12MB4536.namprd12.prod.outlook.com (2603:10b6:208:263::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7113.28; Thu, 28 Dec 2023 19:34:24 +0000 Received: from DS2PEPF00003441.namprd04.prod.outlook.com (2603:10b6:5:54:cafe::4c) by DM6PR06CA0064.outlook.office365.com (2603:10b6:5:54::41) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7135.18 via Frontend Transport; Thu, 28 Dec 2023 19:34:24 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by DS2PEPF00003441.mail.protection.outlook.com (10.167.17.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.7135.14 via Frontend Transport; Thu, 28 Dec 2023 19:34:24 +0000 Received: from khazad-dum.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 28 Dec 2023 13:34:22 -0600 From: Lancelot SIX To: CC: Guinevere Larsen , , "Lancelot SIX" Subject: [PATCH] gdb/infrun: lazily load curr_frame_id in process_event_stop_test Date: Thu, 28 Dec 2023 19:33:59 +0000 Message-ID: <20231228193359.3888031-1-lancelot.six@amd.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS2PEPF00003441:EE_|MN2PR12MB4536:EE_ X-MS-Office365-Filtering-Correlation-Id: 87dd835c-939d-48c9-f075-08dc07dbff93 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: DOh1A93MtAuSpLMcV/tCTALLkTaIxzv/+XjQww93zNbx1uREUmVeB+3p5g0PAGV65IPv2M4HEzGPeu/l3m94SVU0vJBjeV0e3wja4E6aFgaQLNK42+9J4InjE8ig9mb9c99pEUOx0IZF8VOWTJz8gpdTIsK7OhDQbThacZZ24aGaxom6gAQwvFHkb7tT8IlWl170E8W4Yp7+H4XXVTUw+sUL35ewMMcBAszF5XmB2bHGOD+fNa0v7d6AelYIerQ0ZS8ZLe5UXdRuZhWLfQwqJuRT3cE9UvndYuCu3f8+LSiUIThmA3qkKj/cHQbk1KM+ch2OOym4wwkdNHWukNJNZgTRYzKN3Tm/3Ft9IP+R2CvD2VmUzj3sBpDR0DB72VyIDYwWLeSC8e/tKCXnXObLdseDCr9xjzbuERYgnZx5v6z/9NMP3cN+b2BZCyQIVxRIQ9w9oh8pQ7crpjvpxa05/34vDNZkJQNHhWpPCIjY3/bGKVKl/zqUZbZ+IXcqmoGF7JLdFL/eGQQpReLsxhUKnTh6PKtwGxDTTzoDTFfy2ncSZBD41IwJpJ9Y5iYQTXeZP5TWfVPO+BYHqvMm/BGRJ6Az59xkC2MyTw71NXrzJIz1CKPcCXM+bxqSjhxkyBQgv8atsGuNgEjkSKCMcTmLSnOMeA9tuLlPE7/Y4SM8yBr3TJMjxyOG/U3b2nYZDJ4rJmFJRA6I57DB2gR2DshLhGUNXg5By+k7ssgJ8EqjkA/KjqAo8dgvHmkpfoULszIcLvqddMlGDpJWnRr8pr2ZDA== X-Forefront-Antispam-Report: CIP:165.204.84.17; CTRY:US; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:SATLEXMB04.amd.com; PTR:InfoDomainNonexistent; CAT:NONE; SFS:(13230031)(4636009)(376002)(396003)(136003)(39850400004)(346002)(230922051799003)(451199024)(186009)(1800799012)(82310400011)(64100799003)(36840700001)(40470700004)(46966006)(2906002)(4326008)(8676002)(8936002)(83380400001)(1076003)(16526019)(26005)(5660300002)(426003)(336012)(356005)(81166007)(2616005)(36860700001)(966005)(478600001)(40460700003)(40480700001)(6666004)(47076005)(36756003)(7696005)(86362001)(82740400003)(54906003)(316002)(41300700001)(70206006)(70586007)(6916009)(36900700001); DIR:OUT; SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Dec 2023 19:34:24.6260 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 87dd835c-939d-48c9-f075-08dc07dbff93 X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d; Ip=[165.204.84.17]; Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: DS2PEPF00003441.namprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4536 X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FORGED_SPF_HELO, GIT_PATCH_0, KAM_LOTSOFHASH, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_NONE, 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org A recent(ish) change in gdb/infrun.c made process_event_stop_test load debug information where it would not have done so previously. The change is: commit bf2813aff8f2988ad3d53e819a0415abf295c91f AuthorDate: Fri Sep 1 13:47:32 2023 +0200 CommitDate: Mon Nov 20 10:54:03 2023 +0100 gdb/record: print frame information when exiting a recursive call Currently, when GDB is reverse stepping out of a function into the same function due to a recursive call, it doesn't print frame information, as reported by PR record/29178. This happens because when the inferior leaves the current frame, GDB decides to refresh the step information, clobbering the original step_frame_id, making it impossible to figure out later on that the frame has been changed. This commit changes GDB so that, if we notice we're in this exact situation, we won't refresh the step information. Because of implementation details, this change can cause some debug information to be read when it normally wouldn't before, which showed up as a regression on gdb.dwarf2/dw2-out-of-range-end-of-seq. Since that isn't a problem, the test was changed to allow for the new output. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29178 Although there is nothing wrong with this change in principle, it happens to break most of the tests in gdb/testsuite/gdb.rocm/*.exp. This is because those tests do rely on GDB not loading debug information. This is necessary because the debug information produced for AMDGPU code is using DWARF extensions which are not supported by GDB at this point. In this patch, I propose to use a lazy loading mechanism so the frame_id for the current frame is only computed when required instead of when entering process_event_stop_test. The lazy_loader class is currently defined locally in infrun.c, but if it turns out to be useful elsewhere, it could go somewhere under gdbsupport. This patch should restore the behavior GDB had before bf2813aff8f2988ad3d53e819a0415abf295c91f when it comes to load debug info. Another approach could have been to revert fb84fbf8a51f5be2e78765508ebd9753af96b492 (gdb/infrun: simplify process_event_stop_test) and adjust the implementation of bf2813aff8f2988ad3d53e819a0415abf295c91f (gdb/record: print frame information when exiting a recursive call). However, I think that the lazy loading works well with the simplification done recently, so I went down that route. Regression tested on x86_64-linux (Ubuntu 22.04) with AMDGPU support. Change-Id: Ib63a162128130d1786a77c98623e9e3dcbc363b7 --- gdb/infrun.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 9 deletions(-) base-commit: 3bb1944a5a0527a38702084ac301d9933b0130bb diff --git a/gdb/infrun.c b/gdb/infrun.c index 1d863896c40..f7f83d540e2 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7295,6 +7295,37 @@ handle_signal_stop (struct execution_control_state *ecs) process_event_stop_test (ecs); } +namespace { + +/* Helper class for process_event_stop_test implementing lazy evaluation. */ +template +class lazy_loader +{ + using fetcher_t = std::function; + +public: + explicit lazy_loader (fetcher_t &&f) : m_loader (std::move (f)) + { } + + T &operator* () + { + if (!m_value.has_value ()) + m_value.emplace (m_loader ()); + return m_value.value (); + } + + T *operator-> () + { + return &**this; + } + +private: + std::optional m_value; + fetcher_t m_loader; +}; + +} + /* Come here when we've got some debug event / signal we can explain (IOW, not a random signal), and test whether it should cause a stop, or whether we should resume the inferior (transparently). @@ -7335,7 +7366,8 @@ process_event_stop_test (struct execution_control_state *ecs) /* Shorthand to make if statements smaller. */ struct frame_id original_frame_id = ecs->event_thread->control.step_frame_id; - struct frame_id curr_frame_id = get_frame_id (get_current_frame ()); + lazy_loader curr_frame_id + ([] () { return get_frame_id (get_current_frame ()); }); switch (what.main_action) { @@ -7423,7 +7455,7 @@ process_event_stop_test (struct execution_control_state *ecs) if (init_frame) { - if (curr_frame_id == ecs->event_thread->initiating_frame) + if (*curr_frame_id == ecs->event_thread->initiating_frame) { /* Case 2. Fall through. */ } @@ -7596,7 +7628,7 @@ process_event_stop_test (struct execution_control_state *ecs) if (pc_in_thread_step_range (ecs->event_thread->stop_pc (), ecs->event_thread) && (execution_direction != EXEC_REVERSE - || curr_frame_id == original_frame_id)) + || *curr_frame_id == original_frame_id)) { infrun_debug_printf ("stepping inside range [%s-%s]", @@ -8039,7 +8071,7 @@ process_event_stop_test (struct execution_control_state *ecs) frame machinery detected some skipped call sites, we have entered a new inline function. */ - if ((curr_frame_id == original_frame_id) + if ((*curr_frame_id == original_frame_id) && inline_skipped_frames (ecs->event_thread)) { infrun_debug_printf ("stepped into inlined function"); @@ -8087,7 +8119,7 @@ process_event_stop_test (struct execution_control_state *ecs) through a more inlined call beyond its call site. */ if (get_frame_type (get_current_frame ()) == INLINE_FRAME - && (curr_frame_id != original_frame_id) + && (*curr_frame_id != original_frame_id) && stepped_in_from (get_current_frame (), original_frame_id)) { infrun_debug_printf ("stepping through inlined function"); @@ -8118,7 +8150,7 @@ process_event_stop_test (struct execution_control_state *ecs) end_stepping_range (ecs); return; } - else if (curr_frame_id == original_frame_id) + else if (*curr_frame_id == original_frame_id) { /* We are not at the start of a statement, and we have not changed frame. @@ -8144,9 +8176,9 @@ process_event_stop_test (struct execution_control_state *ecs) } } else if (execution_direction == EXEC_REVERSE - && curr_frame_id != original_frame_id - && original_frame_id.code_addr_p && curr_frame_id.code_addr_p - && original_frame_id.code_addr == curr_frame_id.code_addr) + && *curr_frame_id != original_frame_id + && original_frame_id.code_addr_p && curr_frame_id->code_addr_p + && original_frame_id.code_addr == curr_frame_id->code_addr) { /* If we enter here, we're leaving a recursive function call. In this situation, we shouldn't refresh the step information, because if we