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 2C82F3858C60 for ; Sun, 28 Jan 2024 20:56:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2C82F3858C60 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 2C82F3858C60 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=1706475399; cv=none; b=ddxhqcWQtxD6oXZegxlGWKHaJlGgRPijZDZUTvAx91W83Mfl8OG/1ndi1Li053XL2uMLqJskTRc/eX5T5W9aiPnkH5GAR4pVhIccWDycY6fVLF7GixbaHbdUwMiT/QpwRB3katNoSt/zLvWOaDr44NtFnJ4PG+j5Eqmp4dfrMTs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706475399; c=relaxed/simple; bh=ttTiZnx8Spc5X7onrlHT22huonhnEthtxP0uAtA4/4Q=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=eOYCkt/V7LwccmN2MS6/eA3LIbNPty3jW/MMOWpgYHtFzhaK4BgirMF77xfjsNvJGfkaLW9emPTa2A0JR15B9FICYssqqPQ+McD8j0N6VGwbhy6Bz7zPhfN7OFIOnm6MMmj9XnzAesKHYjU1kMbV6id2sSXTqv2wsDanlxmLsfY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706475396; 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=tTvL/frXxgLEUy4jBiFJtVUZcdpd88lo4J/Hb7pc6ps=; b=agesqbLgiWUkSE8YD/nO+DcG+I9lGCEwKZU0e9cTpfIaxTQixXIQNw60i4mbvHDguvn2d4 6aVte65FDtTPIiD7jTdFLL0k5rzO7yToFw+VyxZZwL49ltN1ayI+ReXmVVxdDELuwFoMM+ VxgmxierBT/U5WE86sfSExrNRDmSnCM= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-696-F7uMMb0xMm2sJscmQzbgjg-1; Sun, 28 Jan 2024 15:56:33 -0500 X-MC-Unique: F7uMMb0xMm2sJscmQzbgjg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (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 1497629AA38E for ; Sun, 28 Jan 2024 20:56:33 +0000 (UTC) Received: from f39-zbm-amd (unknown [10.22.16.87]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AD8AD2166B31; Sun, 28 Jan 2024 20:56:32 +0000 (UTC) Date: Sun, 28 Jan 2024 13:56:31 -0700 From: Kevin Buettner To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/unwinders: better support for $pc not saved Message-ID: <20240128135631.5676324b@f39-zbm-amd> In-Reply-To: <4d3be44a23e2e5853d966c081bccbeb751004310.1706366387.git.aburgess@redhat.com> References: <4d3be44a23e2e5853d966c081bccbeb751004310.1706366387.git.aburgess@redhat.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 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=-6.0 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 Sat, 27 Jan 2024 15:26:32 +0000 Andrew Burgess wrote: > This started with a Red Hat bug report which can be seen here: > > https://bugzilla.redhat.com/show_bug.cgi?id=1850710 > > The problem reported here was using GDB on GNU/Linux for S390, the > user stepped into JIT generated code. As they enter the JIT code GDB > would report 'PC not saved', and this same message would be reported > after each step/stepi. > > Additionally, the user had 'set disassemble-next-line on', and once > they entered the JIT code this output was not displayed, nor were any > 'display' directives displayed. > > The user is not making use of the JIT plugin API to provide debug > information. But that's OK, they aren't expecting any source level > debug here, they are happy to use 'stepi', but the missing 'display' > directives are a problem, as is the constant 'PC not saved' (error) > message. > > What is happening here is that as GDB is failing to find any debug > information for the JIT generated code, it is falling back on to the > S390 prologue unwinder to try and unwind frame #0. Unfortunately, > without being able to identify the function boundaries, the S390 > prologue scanner can't help much, in fact, it doesn't even suggest an > arbitrary previous $pc value (some targets that use a link-register > will, by default, assume the link-register contains the previous $pc), > instead the S390 will just say, "sorry, I have no previous $pc value". > > The result of this is that when GDB tries to find frame #1 we end > throwing an error from frame_unwind_pc (the 'PC not saved' error). > This error is not caught anywhere except at the top-level interpreter > loop, and so we end up skipping all the 'display' directive handling. > > While thinking about this, I wondered, could I trigger the same error > using the Python Unwinder API? What happens if a Python unwinder > claims a frame, but then fails to provide a previous $pc value? > > Turns out that exactly the same thing happens, which is great, as that > means we now have a way to reproduce this bug on any target. And so > the test included with this patch does just this. I have a Python > unwinder that claims a frame, but doesn't provide any previous > register values. > > I then do two tests, first I stop in the claimed frame (i.e. frame #0 > is the frame that can't be unwound), I perform a few steps, and check > the backtrace. And second, I stop in a child of the problem > frame (i.e. frame #1 is the frame that can't be unwound), and from > here I check the backtrace. > > While all this is going on I have a 'display' directive in place, and > each time GDB stops I check that the display directive triggers. > > Additionally, when checking the backtrace, I am checking that the > backtrace finishes with the message 'Backtrace stopped: frame did not > save the PC'. > > As for the fix I chose to add a call to frame_unwind_pc directly to > get_prev_frame_always_1. Calling frame_unwind_pc will cache the > unwound $pc value, so this doesn't add much additional work as > immediately after the new frame_unwind_pc call, we call > get_prev_frame_maybe_check_cycle, which actually generates the > previous frame, which will always (I think) require a call to > frame_unwind_pc anyway. > > The reason for adding the frame_unwind_pc call into > get_prev_frame_always_1, is that if the frame_unwind_pc call fails we > want to set the frames 'stop_reason', and get_prev_frame_always_1 > seems to be the place where this is done, so I wanted to keep the new > stop_reason setting code next to all the existing stop_reason setting > code. > > Additionally, once we enter get_prev_frame_maybe_check_cycle we > actually create the previous frame, then, if it turns out that the > previous frame can't be created we need to remove the frame .. this > seemed more complex than just making the check in > get_prev_frame_always_1. > > With this fix in place the original S390 bug is fixed, and also the > test added in this commit, that uses the Python API, is also fixed. Thanks for the detailed commit log! This approach looks reasonable to me. Reviewed-by: Kevin Buettner