From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id RNgLAm3r0GDhRAAAWB0awg (envelope-from ) for ; Mon, 21 Jun 2021 15:41:33 -0400 Received: by simark.ca (Postfix, from userid 112) id ECCE11F1F2; Mon, 21 Jun 2021 15:41:32 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 30E591E54D for ; Mon, 21 Jun 2021 15:41:31 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 216FB393AC3D for ; Mon, 21 Jun 2021 19:41:30 +0000 (GMT) Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by sourceware.org (Postfix) with ESMTPS id ADE2B385701F for ; Mon, 21 Jun 2021 19:41:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ADE2B385701F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x442.google.com with SMTP id b3so10694625wrm.6 for ; Mon, 21 Jun 2021 12:41:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=O1itONJha7eeCh/b5oPw3tiUzBdadgoXZawmoYmiSEc=; b=HT3FcdCYXBr3oJCpREEbZ4zS8IuBu1EzGY/RMKopW1ICv/Ncw8vtKR20jwJJ61E8zD TGE8Aj6CUX2gtB43hN6P414/7GTVuzezpwz6RjF8MxPNAvBE60o9xqOnUr0Orf9/rcyT Lmk8qjdHK/iuhmUjWBGXoF+SPA9Q1ZcYbfP32RQ8R7Xc1RoSJPbB8ELNsp7lST3XaE3j Ua0pi0v7dV4pV9aHCdbO5lSWUG8FClrTXImB/bLO/gp+DFrcSxzqWTYgv5KAulR1qS/g mhTcwklW3o69+lwkxCWkkjb6WHufSjWmB1k4g2Csaqr85L5CWqZD2649Wl6nggaSFX0x jAOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=O1itONJha7eeCh/b5oPw3tiUzBdadgoXZawmoYmiSEc=; b=HbOUaRknpv0notMSGNzQZ6b2FEeTdTnMWYFnC7uVn9QjSjgt1D+Ochd6gf8nckbyxA 6t9Psh3CKejVEpa5RbnO/W+FIlWwgvciu9PLuFHhIM4doxbBIyKZG9pcCebr1xHwRlUv Ea+H2x/rwR45ocgfmabYhEUKSidTq8UQlMH5y44Y7JvlDLK9A3sFU4ly6axJ2lSQCYHT bnI52EDSCVqRhR1oUZe5Nw89KAiWrxKjPEWl+hcxAqUHLGq5xT5rONBKP3kM9dyAP/b1 rkCmyMC1WfJK+3+lfh2yzS2JbMvIYgMZ8L+InSfb3lKdYVYPG7AfDSm1CTP2L2VWWLOL QpUA== X-Gm-Message-State: AOAM53174xHTkYsNDxQ1IFXQmtu/NA3fFNtrq8rS9NmxJKeaQ0udfJWH MVTXiP5HhdnymIIQ7VNQPlL9prgQIY3jxhUz X-Google-Smtp-Source: ABdhPJwV3EJIUrkPuk2MsE2d4X4C8NHJShXfNdWzte9dyNWX6FuTuoolyoqqxM6cHPLzA73dLKbglg== X-Received: by 2002:adf:a284:: with SMTP id s4mr12776wra.397.1624304476619; Mon, 21 Jun 2021 12:41:16 -0700 (PDT) Received: from localhost (host86-140-92-85.range86-140.btcentralplus.com. [86.140.92.85]) by smtp.gmail.com with ESMTPSA id o7sm19803240wro.76.2021.06.21.12.41.15 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 12:41:15 -0700 (PDT) Date: Mon, 21 Jun 2021 20:41:14 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCH 1/5] gdb/python: handle saving user registers in a frame unwinder Message-ID: <20210621194114.GF2568@embecosm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 20:40:55 up 5 days, 3:32, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" I've pushed this patch. Thanks, Andrew * Andrew Burgess [2021-05-29 21:57:10 +0100]: > This patch came about because I wanted to write a frame unwinder that > would corrupt the backtrace in a particular way. In order to achieve > what I wanted I ended up trying to write an unwinder like this: > > class FrameId(object): > .... snip class definition .... > > class TestUnwinder(Unwinder): > def __init__(self): > Unwinder.__init__(self, "some name") > > 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) > > # ... snip code to decide if this unwinder applies or not. > > fid = FrameId(pc, sp) > unwinder = pending_frame.create_unwind_info(fid) > unwinder.add_saved_register(pc_desc, pc) > unwinder.add_saved_register(sp_desc, sp) > return unwinder > > The important things here are the two calls: > > unwinder.add_saved_register(pc_desc, pc) > unwinder.add_saved_register(sp_desc, sp) > > On x86-64 these would fail with an assertion error: > > gdb/regcache.c:168: internal-error: int register_size(gdbarch*, int): Assertion `regnum >= 0 && regnum < gdbarch_num_cooked_regs (gdbarch)' failed. > > What happens is that in unwind_infopy_add_saved_register (py-unwind.c) > we call register_size, as register_size should only be called on > cooked (real or pseudo) registers, and 'pc' and 'sp' are implemented > as user registers (at least on x86-64), we trigger the assertion. > > A simple fix would be to check in unwind_infopy_add_saved_register if > the register number we are handling is a cooked register or not, if > not we can throw a 'Bad register' error back to the Python code. > > However, I think we can do better. > > Consider that at the CLI we can do this: > > (gdb) set $pc=0x1234 > > This works because GDB first evaluates '$pc' to get a register value, > then evaluates '0x1234' to create a value encapsulating the > immediate. The contents of the immediate value are then copied back > to the location of the register value representing '$pc'. > > The value location for a user-register will (usually) be the location > of the real register that was accessed, so on x86-64 we'd expect this > to be $rip. > > So, in this patch I propose that in the unwinder code, when > add_saved_register is called, if it is passed a > user-register (i.e. non-cooked) then we first fetch the register, > extract the real register number from the value's location, and use > that new register number when handling the add_saved_register call. > > If either the value location that we get for the user-register is not > a cooked register then we can throw a 'Bad register' error back to the > Python code, but in most cases this will not happen. > > gdb/ChangeLog: > > * python/py-unwind.c (unwind_infopy_add_saved_register): Handle > saving user registers. > > gdb/testsuite/ChangeLog: > > * gdb.python/py-unwind-user-regs.c: New file. > * gdb.python/py-unwind-user-regs.exp: New file. > * gdb.python/py-unwind-user-regs.py: New file. > --- > gdb/ChangeLog | 5 + > gdb/python/py-unwind.c | 21 ++++ > gdb/testsuite/ChangeLog | 6 ++ > .../gdb.python/py-unwind-user-regs.c | 37 +++++++ > .../gdb.python/py-unwind-user-regs.exp | 98 +++++++++++++++++++ > .../gdb.python/py-unwind-user-regs.py | 72 ++++++++++++++ > 6 files changed, 239 insertions(+) > create mode 100644 gdb/testsuite/gdb.python/py-unwind-user-regs.c > create mode 100644 gdb/testsuite/gdb.python/py-unwind-user-regs.exp > create mode 100644 gdb/testsuite/gdb.python/py-unwind-user-regs.py > > diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c > index 7c195eb539d..d6e2f85dbc1 100644 > --- a/gdb/python/py-unwind.c > +++ b/gdb/python/py-unwind.c > @@ -27,6 +27,7 @@ > #include "python-internal.h" > #include "regcache.h" > #include "valprint.h" > +#include "user-regs.h" > > /* Debugging of Python unwinders. */ > > @@ -265,6 +266,26 @@ unwind_infopy_add_saved_register (PyObject *self, PyObject *args) > PyErr_SetString (PyExc_ValueError, "Bad register"); > return NULL; > } > + > + /* If REGNUM identifies a user register then *maybe* we can convert this > + to a real (i.e. non-user) register. The maybe qualifier is because we > + don't know what user registers each target might add, however, the > + following logic should work for the usual style of user registers, > + where the read function just forwards the register read on to some > + other register with no adjusting the value. */ > + if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch)) > + { > + struct value *user_reg_value > + = value_of_user_reg (regnum, pending_frame->frame_info); > + if (VALUE_LVAL (user_reg_value) == lval_register) > + regnum = VALUE_REGNUM (user_reg_value); > + if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch)) > + { > + PyErr_SetString (PyExc_ValueError, "Bad register"); > + return NULL; > + } > + } > + > { > struct value *value; > size_t data_size; > diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.c b/gdb/testsuite/gdb.python/py-unwind-user-regs.c > new file mode 100644 > index 00000000000..8d1efd1a85d > --- /dev/null > +++ b/gdb/testsuite/gdb.python/py-unwind-user-regs.c > @@ -0,0 +1,37 @@ > +/* This test program is part of GDB, the GNU debugger. > + > + Copyright 2021 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; > + > +void __attribute__ ((noinline)) > +foo (void) > +{ > + ++global_var; /* Break here. */ > +} > + > +void __attribute__ ((noinline)) > +bar (void) > +{ > + foo (); > +} > + > +int > +main (void) > +{ > + bar (); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.exp b/gdb/testsuite/gdb.python/py-unwind-user-regs.exp > new file mode 100644 > index 00000000000..7ae3a5bb19f > --- /dev/null > +++ b/gdb/testsuite/gdb.python/py-unwind-user-regs.exp > @@ -0,0 +1,98 @@ > +# Copyright (C) 2021 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 . > + > +# Setup an unwinder that uses gdb.UnwindInfo.add_saved_register with > +# the register's 'pc' and 'sp'. On some (all?) targets, these > +# registers are implemented as user-registers, and so can't normally > +# be written to directly. > +# > +# The Python unwinder now includes code similar to how the expression > +# evaluator would handle something like 'set $pc=0x1234', we fetch the > +# value of '$pc', and then use the value's location to tell us which > +# register to write to. > +# > +# The unwinder defined here deliberately breaks the unwind by setting > +# the unwound $pc and $sp to be equal to the current frame's $pc and > +# $sp. GDB will spot this as a loop in the backtrace and terminate > +# the unwind. > +# > +# However, by the time the unwind terminates we have already shown > +# that it is possible to call add_saved_register with a user-register, > +# so the test is considered passed. > +# > +# For completeness this test checks two cases, calling > +# add_saved_register with a gdb.RegisterDescriptor and calling > +# add_saved_register with a string containing the register name. > + > +load_lib gdb-python.exp > + > +standard_testfile > + > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > + return -1 > +} > + > +# Skip all tests if Python scripting is not enabled. > +if { [skip_python_tests] } { continue } > + > +if ![runto_main] then { > + fail "can't run to main" > + return 0 > +} > + > +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] > + > +gdb_breakpoint [gdb_get_line_number "Break here"] > +gdb_continue_to_breakpoint "stop at test breakpoint" > + > +# Load the script containing the unwinders. There are actually two > +# unwinders defined here that will catch the same function, so we > +# immediately disable one of the unwinders. > +gdb_test_no_output "source ${pyfile}"\ > + "import python scripts" > +gdb_test "disable unwinder global \"break unwinding using strings\"" \ > + "1 unwinder disabled" "disable the unwinder that uses strings" > + > +# At this point we are using the unwinder that passes a > +# gdb.RegisterDescriptor to add_saved_register. > +gdb_test_sequence "bt" "Backtrace corrupted by descriptor based unwinder" { > + "\\r\\n#0 \[^\r\n\]* foo \\(\\) at " > + "\\r\\n#1 \[^\r\n\]* bar \\(\\) at " > + "Backtrace stopped: previous frame inner to this frame \\(corrupt stack\\?\\)" > +} > + > +# Disable the unwinder that calls add_saved_register with a > +# gdb.RegisterDescriptor, and enable the unwinder that calls > +# add_saved_register with a string (containing the register name). > +gdb_test "disable unwinder global \"break unwinding using descriptors\"" \ > + "1 unwinder disabled" "disable the unwinder that uses descriptors" > +gdb_test "enable unwinder global \"break unwinding using strings\"" \ > + "1 unwinder enabled" "enable the unwinder that uses strings" > +gdb_test_sequence "bt" "Backtrace corrupted by string based unwinder" { > + "\\r\\n#0 \[^\r\n\]* foo \\(\\) at " > + "\\r\\n#1 \[^\r\n\]* bar \\(\\) at " > + "Backtrace stopped: previous frame inner to this frame \\(corrupt stack\\?\\)" > +} > + > +# Just for completeness, disable the string unwinder again (neither of > +# our special unwinders are now enabled), and check the backtrace. We > +# now get the complete stack back to main. > +gdb_test "disable unwinder global \"break unwinding using strings\"" \ > + "1 unwinder disabled" "disable the unwinder that uses strings again" > +gdb_test_sequence "bt" "Backtrace not corrupted when using no unwinder" { > + "\\r\\n#0 \[^\r\n\]* foo \\(\\) at " > + "\\r\\n#1 \[^\r\n\]* bar \\(\\) at " > + "\\r\\n#2 \[^\r\n\]* main \\(\\) at " > +} > diff --git a/gdb/testsuite/gdb.python/py-unwind-user-regs.py b/gdb/testsuite/gdb.python/py-unwind-user-regs.py > new file mode 100644 > index 00000000000..e5edd7cbd9c > --- /dev/null > +++ b/gdb/testsuite/gdb.python/py-unwind-user-regs.py > @@ -0,0 +1,72 @@ > +# Copyright (C) 2021 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 > + > + > +class FrameId(object): > + def __init__(self, sp, pc): > + self._sp = sp > + self._pc = pc > + > + @property > + def sp(self): > + return self._sp > + > + @property > + def pc(self): > + return self._pc > + > + > +class TestUnwinder(Unwinder): > + def __init__(self, use_descriptors): > + if use_descriptors: > + tag = "using descriptors" > + else: > + tag = "using strings" > + > + Unwinder.__init__(self, "break unwinding %s" % tag) > + self._use_descriptors = use_descriptors > + > + 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) > + > + block = gdb.block_for_pc(int(pc)) > + if block == None: > + return None > + func = block.function > + if func == None: > + return None > + if str(func) != "bar": > + return None > + > + fid = FrameId(pc, sp) > + unwinder = pending_frame.create_unwind_info(fid) > + if self._use_descriptors: > + unwinder.add_saved_register(pc_desc, pc) > + unwinder.add_saved_register(sp_desc, sp) > + else: > + unwinder.add_saved_register("pc", pc) > + unwinder.add_saved_register("sp", sp) > + return unwinder > + > + > +gdb.unwinder.register_unwinder(None, TestUnwinder(True), True) > +gdb.unwinder.register_unwinder(None, TestUnwinder(False), True) > -- > 2.25.4 >