From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id kAUALtUgtWWi4QkAWB0awg (envelope-from ) for ; Sat, 27 Jan 2024 10:27:17 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=bBF3cs2w; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id A839E1E0C3; Sat, 27 Jan 2024 10:27:17 -0500 (EST) Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 8CFFA1E092 for ; Sat, 27 Jan 2024 10:27:15 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D0141385841D for ; Sat, 27 Jan 2024 15:27:14 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 9951A3858D28 for ; Sat, 27 Jan 2024 15:26:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9951A3858D28 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 9951A3858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706369203; cv=none; b=SYISezvxdilGlkzUP/j+PMCqX+eIl4lU298t1SQT7HAsoIfRtFFGbWrtB5rSZCewJI0n/ZzjuIwTe/l5ZMWggXl8G8/RsfhYRZ0H8/i07EzLyQ3bcNQB6KW//1mBEOXl7uBCeS64T3c/MBiOnRLfwYUo0x9nN+Xd1HfGSFMtBQk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706369203; c=relaxed/simple; bh=IWS9kkdKFY3a6FWSy+ao/kDl4aQM1tzJyazPCFYbyVE=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=jLp9gyMCJPl0Hqps8xl7rAULUtAXkDmT4CJzSqojp1nIHUwG1BkKcjI7gJxsIueX6wnuubIJAxsEgYgmU7Pf1gpWZuNwMaXpt9CQybrjbnf1uBKpdz49rH8Iag7juFgedU1mhm2m/8fM3v0MYsE3y6/wtEy8waSyfuhFNKZaI/w= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706369198; 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; bh=ETWpcccMVgz61vpYQOey8NqCJtnxR0zXQgpXzalzQic=; b=bBF3cs2wNy4dKGF3tqs+LgVS3vD4FuHJlOYpf9lHXC6F+vIl+VbyC7y0v04O+3VriWFT0O 1zCv2Tn3ARVfQN8LKN6gOos5EySphSvrXSqs3q8RZQNGEVoeyTieHAU7AFAY2Vvv69Dy4O g7MSLfbP9JMmmLgVjU49ueGmp98TM98= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-365-EDs6wyj4NqKG_5KevgxAUg-1; Sat, 27 Jan 2024 10:26:37 -0500 X-MC-Unique: EDs6wyj4NqKG_5KevgxAUg-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-40e89a9a76fso7996675e9.1 for ; Sat, 27 Jan 2024 07:26:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706369195; x=1706973995; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ETWpcccMVgz61vpYQOey8NqCJtnxR0zXQgpXzalzQic=; b=eQKBrfDRni5NsX/9sbYHpfWi3IIXLSH4q7wZfMZGlobJLbsTVcVVgfThJgMTH0x7cd qJkFlWHDf/RrRH0C8t+ajI2IqjkpYfZxIIeIO+nuvuG3bqnSMDtX2TrEsQG0L1ICTgZp 06Sqv5NHezUHG7ermDNviwV8cxa7XPWRaTjdeurKQU7X6qe/qzv7sfjtn63Jq+IYtbZW wR/PoviEQMdz6pSVJWpzJWO8lB7qUX2XhXVtw549EnYkibBfryRAh0vHlMhgSsFRB/D+ sCkYLQwW3ziB0rgzGTwiybcrCmo1hKf9GuvhU83j18Cyd2GftCmtLjZO2dmdfATZi5DQ csoA== X-Gm-Message-State: AOJu0YwZ2VTmvQS7Pc+o77kYrQmZ+sAQs4ibtrIwdQlghsaIfRZ/mSRP YFbGp5R5XCShlvMal5VVKsaqv3mUy6ORKCZ3H86oywV+z9tAq7DrLH/Ydo42MFZRmYY0dRGrWs8 71zVoWDcJLa78NyY5rjV8ex9gIjeWuzJSi7AF546E2oy1g14OC+/26EFM00C5F6OahUnQcCbnm3 8yYLz5GXWztw3swWhIuJIjkAsg71YwlJJPyoechS0ILqc= X-Received: by 2002:adf:ec10:0:b0:336:7a5c:ffdf with SMTP id x16-20020adfec10000000b003367a5cffdfmr1696747wrn.13.1706369194940; Sat, 27 Jan 2024 07:26:34 -0800 (PST) X-Google-Smtp-Source: AGHT+IFMMI/ZqHV08g6Xj3bg/DMDBpco9DrGgOvtZwlzS/ZPhKge0XtuZ9sxPb6lLq675v7KfgqsVg== X-Received: by 2002:adf:ec10:0:b0:336:7a5c:ffdf with SMTP id x16-20020adfec10000000b003367a5cffdfmr1696733wrn.13.1706369194253; Sat, 27 Jan 2024 07:26:34 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id bh2-20020a05600005c200b0033ae57a6092sm1082084wrb.0.2024.01.27.07.26.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 27 Jan 2024 07:26:33 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb/unwinders: better support for $pc not saved Date: Sat, 27 Jan 2024 15:26:32 +0000 Message-Id: <4d3be44a23e2e5853d966c081bccbeb751004310.1706366387.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, 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 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+public-inbox=simark.ca@sourceware.org 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. --- gdb/frame.c | 32 +++++++++++ gdb/testsuite/gdb.base/pc-not-saved.c | 48 +++++++++++++++++ gdb/testsuite/gdb.base/pc-not-saved.exp | 70 +++++++++++++++++++++++++ gdb/testsuite/gdb.base/pc-not-saved.py | 50 ++++++++++++++++++ 4 files changed, 200 insertions(+) create mode 100644 gdb/testsuite/gdb.base/pc-not-saved.c create mode 100644 gdb/testsuite/gdb.base/pc-not-saved.exp create mode 100644 gdb/testsuite/gdb.base/pc-not-saved.py diff --git a/gdb/frame.c b/gdb/frame.c index d95d63eb0f6..118d84b980e 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -2419,6 +2419,38 @@ get_prev_frame_always_1 (frame_info_ptr this_frame) } } + /* Ensure we can unwind the program counter of THIS_FRAME. */ + try + { + /* Calling frame_unwind_pc for the sentinel frame relies on the + current_frame being set, which at this point it might not be if we + are in the process of setting the current_frame after a stop (see + get_current_frame). + + The point of this check is to ensure that the unwinder for + THIS_FRAME can actually unwind the $pc, which we assume the + sentinel frame unwinder can always do (it's just a read from the + machine state), so we only call frame_unwind_pc for frames other + than the sentinel (level -1) frame. + + Additionally, we don't actually care about the value of the + unwound $pc, just that the call completed successfully. */ + if (this_frame->level >= 0) + frame_unwind_pc (this_frame); + } + catch (const gdb_exception_error &ex) + { + if (ex.error == NOT_AVAILABLE_ERROR || ex.error == OPTIMIZED_OUT_ERROR) + { + frame_debug_printf (" -> nullptr // no saved PC"); + this_frame->stop_reason = UNWIND_NO_SAVED_PC; + this_frame->prev = nullptr; + return nullptr; + } + + throw; + } + return get_prev_frame_maybe_check_cycle (this_frame); } diff --git a/gdb/testsuite/gdb.base/pc-not-saved.c b/gdb/testsuite/gdb.base/pc-not-saved.c new file mode 100644 index 00000000000..bc6632a97d7 --- /dev/null +++ b/gdb/testsuite/gdb.base/pc-not-saved.c @@ -0,0 +1,48 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +volatile int global_var = 0; + +void +other_func (void) +{ + /* Nothing. */ +} + +void +break_bt_here (void) +{ + /* This is all nonsense; just filler so this function has a body. */ + if (global_var != 99) + global_var++; + if (global_var != 98) + global_var++; + if (global_var != 97) + global_var++; + if (global_var != 96) + global_var++; + other_func (); + if (global_var != 95) + global_var++; +} + +int +main (void) +{ + break_bt_here (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/pc-not-saved.exp b/gdb/testsuite/gdb.base/pc-not-saved.exp new file mode 100644 index 00000000000..3a55a22c7c1 --- /dev/null +++ b/gdb/testsuite/gdb.base/pc-not-saved.exp @@ -0,0 +1,70 @@ +# Copyright 2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test how GDB handles a frame in which the previous-pc value is not +# available. Specifically, check that the backtrace correctly reports +# why the backtrace is truncated, and ensure that 'display' directives +# still work when 'stepi'-ing through the frame. + +require allow_python_tests + +standard_testfile + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { + return +} + +set remote_python_file \ + [gdb_remote_download host "${srcdir}/${subdir}/${testfile}.py"] +gdb_test_no_output "source ${remote_python_file}" "load python file" + +if { ![runto "break_bt_here"] } { + return +} + +gdb_test "bt" \ + [multi_line \ + "^#0 break_bt_here \\(\\) at \[^\r\n\]+" \ + "Backtrace stopped: frame did not save the PC"] \ + "backtrace from break_bt_here function" + +gdb_test "stepi" \ + [multi_line \ + "^(:?$hex in )?break_bt_here \\(\\) at \[^\r\n\]+" \ + "$decimal\\s+\[^\r\n\]+"] \ + "stepi without a display in place" + +gdb_test "display/i \$pc" \ + [multi_line \ + "^1: x/i \\\$pc" \ + "=> $hex :\\s+\[^\r\n\]+"] + +gdb_test "stepi" \ + [multi_line \ + "^(:?$hex in )?break_bt_here \\(\\) at \[^\r\n\]+" \ + "$decimal\\s+\[^\r\n\]+" \ + "1: x/i \\\$pc" \ + "=> $hex :\\s+\[^\r\n\]+"] \ + "stepi with a display in place" + +gdb_breakpoint other_func +gdb_continue_to_breakpoint "continue to other_func" + +gdb_test "bt" \ + [multi_line \ + "#0 other_func \\(\\) at \[^\r\n\]+" \ + "#1 (:?$hex in )?break_bt_here \\(\\) at \[^\r\n\]+" \ + "Backtrace stopped: frame did not save the PC"] \ + "backtrace from other_func function" diff --git a/gdb/testsuite/gdb.base/pc-not-saved.py b/gdb/testsuite/gdb.base/pc-not-saved.py new file mode 100644 index 00000000000..d2fb7b1ef1b --- /dev/null +++ b/gdb/testsuite/gdb.base/pc-not-saved.py @@ -0,0 +1,50 @@ +# Copyright (C) 2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import gdb +from gdb.unwinder import Unwinder, FrameId + +class break_unwinding(Unwinder): + """An unwinder for the function 'break_bt_here'. This unwinder will + claim any frame for the function in question, but doesn't provide + any unwound register values. Importantly, we don't provide a + previous $pc value, this means that if we are stopped in + 'break_bt_here' then we should fail to unwind beyond frame #0.""" + + def __init__(self): + Unwinder.__init__(self, "break unwinding") + + def __call__(self, pending_frame): + pc_desc = pending_frame.architecture ().registers ().find ("pc") + pc = pending_frame.read_register (pc_desc) + + sp_desc = pending_frame.architecture ().registers ().find ("sp") + sp = pending_frame.read_register (sp_desc) + + if pc.is_optimized_out or sp.is_optimized_out: + return None + + block = gdb.block_for_pc (pc) + if block == None: + return None + func = block.function + if func == None: + return None + if str (func) != "break_bt_here": + return None + fid = FrameId (sp, pc) + return pending_frame.create_unwind_info (fid) + +gdb.unwinder.register_unwinder(None, break_unwinding(), True) base-commit: 81b6f191f71fe0af5dd7b1c7c5b7737c3d249a66 -- 2.25.4