From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 9247838582B3 for ; Wed, 10 Jan 2024 20:04:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9247838582B3 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9247838582B3 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704917062; cv=none; b=ZETnZ/s6L9pV9ySPNrfJPvujMonCEs+u23gGl5R17+fLLJQZshgZ8/pBxsk5bc+0RgZvT+PapXYy8ASBcOs4rEkoI44CthqbaS0oSfozYiHvU8q5e8zcLbvhhCLScyXj3wYa3rud2h3SIgpjUWE3x7QctAp7KRN4RcIcMS8ubSo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704917062; c=relaxed/simple; bh=dbbwEcu4t2qCRVi7PIFvQ8LhI286iSNUaFl+DHr4Puk=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=ND0RZZnM3lBsj+AGTRC+Laez3963nd3vjKpkk44nxvvtFJCgm9Y2It8HQqsKsl02Es6iX+RtgV4scRKbesRr2HIEKY37vu69wJa2+9iKyDdMH4pU+4MScH9MvddpnLEDfG/KQLvGN+beltpB4ruZq8MR3SnVCXui4ily5DDdzH0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704917060; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vu3KM5Zs8QQSQA3rv7aXJ958oLxunn3GPNSeaErHZy8=; b=Lq+x3ieB2gcGis5pIavF8NJpuJ6iK1I4UwPS7un0CeY7uYM4M+qeAQUXUfO2+7hKRJx3/s zMzArzezv2P6qWUSFRnFR8CDff2KdMIJmq01hf/SAKKoveUAgTZFyTY8QkaD4pTyrwPz1z n509PvZvv9EwGf97iPRJpUn6KsvdEZs= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-208-3ZPDsMfWPpCZbr_EmElM3w-1; Wed, 10 Jan 2024 15:04:17 -0500 X-MC-Unique: 3ZPDsMfWPpCZbr_EmElM3w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id B597085A58C; Wed, 10 Jan 2024 20:04:16 +0000 (UTC) Received: from f39-zbm-amd (unknown [10.22.32.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 15E0A2026D66; Wed, 10 Jan 2024 20:04:16 +0000 (UTC) Date: Wed, 10 Jan 2024 13:04:14 -0700 From: Kevin Buettner To: Lancelot SIX Cc: gdb-patches@sourceware.org, Guinevere Larsen , Subject: Re: [PATCH] gdb/infrun: lazily load curr_frame_id in process_event_stop_test Message-ID: <20240110130414.1f95f367@f39-zbm-amd> In-Reply-To: <20231228193359.3888031-1-lancelot.six@amd.com> References: <20231228193359.3888031-1-lancelot.six@amd.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,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 List-Id: On Thu, 28 Dec 2023 19:33:59 +0000 Lancelot SIX wrote: > 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. I remember reviewing that change from Guinevere last year wondering if there might be any unexpected fallout from reading debug info earlier than it had been previously. Now we know... I'm okay with your approach, but I'd like to give other maintainers a chance to weigh in. So, please wait a few more days before you push this change. Approved-by: Kevin Buettner Kevin