From patchwork Fri Mar 10 14:55:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 66217 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 EFB4D385B524 for ; Fri, 10 Mar 2023 14:56:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EFB4D385B524 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678460199; bh=oLkhZKxTnU3zAmvVM8Rh9QQsmpp/Z0REKN6rd5dRBmw=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=YCbnqlC92/YIxBzSjSfO0yrc37BTiUaej2imiDVOJn4ENAa4Il9A4yRvdhKWJFpK5 oA+enJzZ/pWMjco+pjx7BdxB+v4Djac+jB2CFEhW+7iiVeTHWoUxkbm0Hd6Dj91GG6 OaBfPomSFjTahBFtDxMlm0UnSGnQGAgjr9lnxEYk= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org 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 A73D13858D3C for ; Fri, 10 Mar 2023 14:55:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A73D13858D3C Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-573--4IYlmicOLGvXXESTqBFTA-1; Fri, 10 Mar 2023 09:55:55 -0500 X-MC-Unique: -4IYlmicOLGvXXESTqBFTA-1 Received: by mail-wm1-f69.google.com with SMTP id k20-20020a05600c1c9400b003e2249bd2b4so1917687wms.5 for ; Fri, 10 Mar 2023 06:55:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678460154; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oLkhZKxTnU3zAmvVM8Rh9QQsmpp/Z0REKN6rd5dRBmw=; b=wZ4kf/BSxg980UgPAZ66hj/rfjXITnIO/sCB5SEg2smw88noUhoiQ21KcvLaZiARyh QggDrPfUiTB8TeI/k4Y7iXkZGuV8Jadauewyk2Lp3ErYl4gRfb5s5rOnyxoqF3Csfu4q Eo0TzNwMF6x8/PbIHz+EqOqA387VpBwjKD+cmLMdHLY1Y9CfCZFoe0it2bOZIfP2KxvK qwqxDIPJwW9IOMCfnEcKjdOJIuJw7aEYTM/ojxD/nuHIvRCfqfv32IAayDku7YfIev0t sfvJw5cBw6Zk7b3CIVW8qQTZF3xR1AXpvTxfoOsIuej2QGxUWsMAFzvEEDg9wO0fzGoW T99w== X-Gm-Message-State: AO0yUKVOsjh6G39WoOOyPkSaEKSQgTlv+CZJQFvcovab4456Xvv1j7dC tDlfUCXi/1Mi6Mh/LqjgLRGad8szEid80JfpuQZw7X+rS+pIhrQtXC9GE37Qzv2lNUpx8r03Ucw ZkQ8BYWFZ9opJJAv6/02Wq0hzAtxD8llAbaE8vXm+UD4QqFzO4MIdppwcGrec0zVvCfER58dtzi nXNORbFA== X-Received: by 2002:a5d:664e:0:b0:2c5:3ccf:e99a with SMTP id f14-20020a5d664e000000b002c53ccfe99amr17257934wrw.6.1678460153720; Fri, 10 Mar 2023 06:55:53 -0800 (PST) X-Google-Smtp-Source: AK7set+/YnvAlx/1fcnd/P2R0WrQA3Jby9Td9LZox2+sn7meuscn1LLnTFPJ/djCnjnmE5vvQ8HUCA== X-Received: by 2002:a5d:664e:0:b0:2c5:3ccf:e99a with SMTP id f14-20020a5d664e000000b002c53ccfe99amr17257919wrw.6.1678460153393; Fri, 10 Mar 2023 06:55:53 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id w1-20020a5d6081000000b002c6e8cb612fsm2236191wrt.92.2023.03.10.06.55.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Mar 2023 06:55:53 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 02/10] gdb/python: make the gdb.unwinder.Unwinder class more robust Date: Fri, 10 Mar 2023 14:55:19 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, URI_LONG_REPEAT 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" This commit makes a few related changes to the gdb.unwinder.Unwinder class attributes: 1. The 'name' attribute is now a read-only attribute. This prevents user code from changing the name after registering the unwinder. It seems very unlikely that any user is actually trying to do this in the wild, so I'm not very worried that this will upset anyone, 2. We now validate that the name is a string in the Unwinder.__init__ method, and throw an error if this is not the case. Hopefully nobody was doing this in the wild. This should make it easier to ensure the 'info unwinder' command shows sane output (how to display a non-string name for an unwinder?), 3. The 'enabled' attribute is now implemented with a getter and setter. In the setter we ensure that the new value is a boolean, but the real important change is that we call 'gdb.invalidate_cached_frames()'. This means that the backtrace will be updated if a user manually disables an unwinder (rather than calling the 'disable unwinder' command). It is not unreasonable to think that a user might register multiple unwinders (relating to some project) and have one command that disables/enables all the related unwinders. This command might operate by poking the enabled attribute of each unwinder object directly, after this commit, this would now work correctly. There's tests for all the changes, and lots of documentation updates that both cover the new changes, but also further improve (I think) the general documentation for GDB's Unwinder API. Reviewed-By: Eli Zaretskii --- gdb/NEWS | 14 ++++ gdb/doc/python.texi | 60 +++++++++++++++- gdb/python/lib/gdb/unwinder.py | 23 ++++++- gdb/testsuite/gdb.python/py-unwind.exp | 94 ++++++++++++++++++++++++-- gdb/testsuite/gdb.python/py-unwind.py | 13 +++- 5 files changed, 191 insertions(+), 13 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index c32ff92c98a..ed0f86e79ec 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -96,6 +96,20 @@ show always-read-ctf without a thread restriction. The same is also true for the 'task' field of an Ada task-specific breakpoint. +* Python API + + ** The gdb.unwinder.Unwinder.name attribute is now read-only. + + ** The name argument passed to gdb.unwinder.Unwinder.__init__ must + now be of type 'str' otherwise a TypeError will be raised. + + ** The gdb.unwinder.Unwinder.enabled attribute can now only accept + values of type 'bool'. Changing this attribute will now + invalidate GDB's frame-cache, which means GDB will need to + rebuild its frame-cache when next required - either with, or + without the particular unwinder, depending on how 'enabled' was + changed. + *** Changes in GDB 13 * MI version 1 is deprecated, and will be removed in GDB 14. diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 9ab0aca7d57..c36130210ba 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2842,6 +2842,33 @@ @var{value} is a register value (a @code{gdb.Value} object). @end defun +@subheading The @code{gdb.unwinder} Module + +@value{GDBN} comes with a @code{gdb.unwinder} module which contains +the following class: + +@deftp {class} gdb.unwinder.Unwinder +The @code{Unwinder} class is a base class from which user created +unwinders can derive, though it is not required that unwinders derive +from this class, so long any user created unwinder has the required +@code{name} and @code{enabled} attributes. + +@defun gdb.unwinder.Unwinder.__init__(@var{name}) +The @var{name} is a string used to identify this unwinder within some +@value{GDBN} commands. +@end defun + +@defvar gdb.unwinder.name +A read-only attribute which is a string, the name of this unwinder. +@end defvar + +@defvar gdb.unwinder.enabled +A modifiable attribute containing a boolean, when @code{True} the +unwinder is enabled, and will be used by @value{GDBN}. When +@code{False} the unwinder has been disabled, and will not be used. +@end defvar +@end deftp + @subheading Registering an Unwinder Object files and program spaces can have unwinders registered with @@ -2872,9 +2899,7 @@ @subheading Unwinder Skeleton Code -@value{GDBN} comes with the module containing the base @code{Unwinder} -class. Derive your unwinder class from it and structure the code as -follows: +Here is an example of how and structure a user created unwinder: @smallexample from gdb.unwinder import Unwinder @@ -2910,6 +2935,35 @@ gdb.unwinder.register_unwinder(, MyUnwinder(), ) @end smallexample +@subheading Managing Registered Unwinders +@value{GDBN} defines 3 new commands to manage registered unwinders. +These are: + +@table @code +@item info unwinder @r{[} @var{locus} @r{[} @var{name-regexp} @r{]} @r{]} +Lists all registered unwinders. Arguments @var{locus} and +@var{name-regexp} are both optional and can be used to filter which +unwinders are listed. + +The @var{locus} argument should be either @kbd{global}, +@kbd{progspace}, or the name of an object file. Only unwinders +registered for the specified locus will be listed. + +The @var{name-regexp} is a regular expression used to match against +unwinder names. When trying to match against unwinder names that +include a string enclose @var{name-regexp} in quotes. +@item disable unwinder @r{[} @var{locus} @r{[} @var{name-regexp} @r{]} @r{]} +The @var{locus} and @var{name-regexp} are interpreted as in @kbd{info +unwinder} above, but instead of listing the matching unwinders, all of +the matching unwinders are disabled. The @code{enabled} field of each +matching unwinder is set to @code{False}. +@item enable unwinder @r{[} @var{locus} @r{[} @var{name-regexp} @r{]} @r{]} +The @var{locus} and @var{name-regexp} are interpreted as in @kbd{info +unwinder} above, but instead of listing the matching unwinders, all of +the matching unwinders are enabled. The @code{enabled} field of each +matching unwinder is set to @code{True}. +@end table + @node Xmethods In Python @subsubsection Xmethods In Python @cindex xmethods in Python diff --git a/gdb/python/lib/gdb/unwinder.py b/gdb/python/lib/gdb/unwinder.py index a854d8dbdaa..1303245c054 100644 --- a/gdb/python/lib/gdb/unwinder.py +++ b/gdb/python/lib/gdb/unwinder.py @@ -35,8 +35,27 @@ class Unwinder(object): Args: name: An identifying name for the unwinder. """ - self.name = name - self.enabled = True + + if not isinstance(name, str): + raise TypeError("incorrect type for name: %s" % type(name)) + + self._name = name + self._enabled = True + + @property + def name(self): + return self._name + + @property + def enabled(self): + return self._enabled + + @enabled.setter + def enabled(self, value): + if not isinstance(value, bool): + raise TypeError("incorrect type for enabled attribute: %s" % type(value)) + self._enabled = value + gdb.invalidate_cached_frames() def __call__(self, pending_frame): """GDB calls this method to unwind a frame. diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp index 5bf9ae129ac..337e5dc2504 100644 --- a/gdb/testsuite/gdb.python/py-unwind.exp +++ b/gdb/testsuite/gdb.python/py-unwind.exp @@ -40,25 +40,105 @@ if {![runto_main]} { return 0 } +# Check for the corrupt backtrace. +proc check_for_broken_backtrace {testname} { + gdb_test_sequence "where" $testname { + "\\r\\n#0 .* corrupt_frame_inner \\(\\) at " + "\\r\\n#1 .* corrupt_frame_outer \\(\\) at " + "Backtrace stopped: frame did not save the PC" + } +} + +# Check for the correct backtrace. +proc check_for_fixed_backtrace {testname} { + gdb_test_sequence "where" $testname { + "\\r\\n#0 .* corrupt_frame_inner \\(\\) at " + "\\r\\n#1 .* corrupt_frame_outer \\(\\) at " + "\\r\\n#2 .* main \\(.*\\) at" + } +} + +# Check the 'info unwinder' output. +proc check_info_unwinder {testname enabled} { + if {$enabled} { + set suffix "" + } else { + set suffix " \\\[disabled\\\]" + } + + gdb_test_sequence "info unwinder" $testname \ + [list \ + "Global:" \ + " test unwinder${suffix}"] +} + set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] gdb_breakpoint [gdb_get_line_number "break backtrace-broken"] +gdb_continue_to_breakpoint "break backtrace-broken" + +check_for_broken_backtrace "Backtrace is initially broken" + gdb_test "source ${pyfile}" "Python script imported" \ - "import python scripts" + "import python scripts" -gdb_continue_to_breakpoint "break backtrace-broken" -gdb_test_sequence "where" "Backtrace restored by unwinder" { - "\\r\\n#0 .* corrupt_frame_inner \\(\\) at " - "\\r\\n#1 .* corrupt_frame_outer \\(\\) at " - "\\r\\n#2 .* main \\(.*\\) at" -} +check_info_unwinder "info unwinder after loading script" on + +check_for_fixed_backtrace "check backtrace after loading unwinder" # Check that the Python unwinder frames can be flushed / released. gdb_test "maint flush register-cache" "Register cache flushed\\." "flush frames" +check_for_fixed_backtrace "check backtrace after flush" + +# Try to disable the unwinder but instead set the enabled field to a +# non boolean value. This should fail. Check the 'info unwinder' +# output to be sure. +gdb_test "python global_test_unwinder.enabled = \"off\"" \ + [multi_line \ + "TypeError: incorrect type for enabled attribute: " \ + "Error while executing Python code\\."] +check_info_unwinder "info unwinder after failed disable" on + +# While we're doing silly stuff, lets try to change the name of this +# unwider. Doing this is bad as the new name might clash with an +# already registered name, which violates the promises made during +# 'register_unwinder'. +gdb_test "python global_test_unwinder.name = \"foo\"" \ + [multi_line \ + "AttributeError: can't set attribute" \ + "Error while executing Python code\\."] +check_info_unwinder "info unwinder after failed name change" on + +# Now actually disable the unwinder by manually adjusting the +# 'enabled' attribute. Check that the stack is once again broken, and +# that the unwinder shows as disabled in the 'info unwinder' output. +gdb_test_no_output "python global_test_unwinder.enabled = False" +check_for_broken_backtrace "stack is broken after disabling" +check_info_unwinder "info unwinder after manually disabling" off + +# Now enable the unwinder using the 'enable unwinder' command. +gdb_test "enable unwinder global \"test unwinder\"" \ + "1 unwinder enabled" +check_for_fixed_backtrace "check backtrace after enabling with command" +check_info_unwinder "info unwinder after command enabled" on + +# And now disable using the command and check the stack is once again +# broken, and that the 'info unwinder' output updates correctly. +gdb_test "disable unwinder global \"test unwinder\"" \ + "1 unwinder disabled" +check_for_broken_backtrace "stack is broken after command disabling" +check_info_unwinder "info unwinder after command disabling" off + # Check that invalid register names cause errors. gdb_test "python print(add_saved_register_error)" "True" \ "add_saved_register error" gdb_test "python print(read_register_error)" "True" \ "read_register error" + +# Try to create an unwinder object with a non-string name. +gdb_test "python obj = simple_unwinder(True)" \ + [multi_line \ + "TypeError: incorrect type for name: " \ + "Error while executing Python code\\."] diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py index f7ad8e45a35..edd2e30eb9b 100644 --- a/gdb/testsuite/gdb.python/py-unwind.py +++ b/gdb/testsuite/gdb.python/py-unwind.py @@ -130,5 +130,16 @@ class TestUnwinder(Unwinder): return None -gdb.unwinder.register_unwinder(None, TestUnwinder(), True) +global_test_unwinder = TestUnwinder() +gdb.unwinder.register_unwinder(None, global_test_unwinder, True) + + +class simple_unwinder(Unwinder): + def __init__(self, name): + super().__init__(name) + + def __call__(self, pending_frame): + return None + + print("Python script imported")