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.129.124]) by sourceware.org (Postfix) with ESMTPS id A095F385800A for ; Fri, 10 Mar 2023 14:56:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A095F385800A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1678460165; 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=xs2IAq7wpz2A7jTeBoMWo/+TZScZ3AaLR+NAb5DPP2c=; b=jQ3oezRg/gpv5qDzkSJZTWtlmUBzxBOFpsUdJTOA42HBukf7ndBYDWucBW2bPV4XJ7wvdm d43ifKLhCjVrrslcwXOOCcv8pi0+iPAIIRiDWhXrJIQuucWjxbT/7WVHDdBQ9GJbXrOjJa Qx6vpKobGnuvc93EoetJkl+w4Hqqi+w= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-41-n5fePA-vNEyOFtwAinAa6w-1; Fri, 10 Mar 2023 09:56:03 -0500 X-MC-Unique: n5fePA-vNEyOFtwAinAa6w-1 Received: by mail-wm1-f71.google.com with SMTP id e17-20020a05600c219100b003e21fa60ec1so2076485wme.2 for ; Fri, 10 Mar 2023 06:56:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678460162; 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=xs2IAq7wpz2A7jTeBoMWo/+TZScZ3AaLR+NAb5DPP2c=; b=dl16E9U4NOCrtogMi1rmpup7paEd70yPIOF4t0d9zKfLTLuZtG26j8Ax2b0pubw5dm X6OIRZbTxwCE8cAV/RnwQ5Z1L+Oahpwna4UQ74zD7enl+yygUbymEtqXkJb+bCo4GvW/ F8DHbbZ1q8X/Ynrtpsp3/OyeJdsnAecwXt9e6ADY8mTuXtFHBElk69EhAhY8xqXq12/0 oAAK22+dpR0VERxE7xZ6NreW36M1stkWSWdxXYFgvPgHkX2LMghLbmGvxv6ubIZFNN8A trr+Ll7gPgS6M0iNnPlLnPkn1TxFY2y75UW4SYblzWmmxFStDMlqdQ+BSh8qukE5/xDe /3kA== X-Gm-Message-State: AO0yUKXYDbJUIlxlKGvyHDY2pXB2Le21wcwVXitZX1sZmsYy/JB3mBFp MFsdRF+npsIsVp13ZHWnaLd5Rx6n1Rf/908vrNsIcQV5DGMIofwZ83TvgY38AEGTckZfGN58Vbj ytxrFwLBOJStul2TUqeIFQ72RoA+svG9vW/wEUG/kR1q5BBxiUZWjaJ5fyb+WKkCiRxvoxSjq4c +gjDoiSA== X-Received: by 2002:a05:600c:3106:b0:3dc:18de:b20d with SMTP id g6-20020a05600c310600b003dc18deb20dmr3129716wmo.33.1678460161971; Fri, 10 Mar 2023 06:56:01 -0800 (PST) X-Google-Smtp-Source: AK7set/KiLGMPpEMs11D32WV2PWEuy/S3T0eOjP40Hy5VwpELinHOMINc3QYuoSHZkoKltcqp6Tjqg== X-Received: by 2002:a05:600c:3106:b0:3dc:18de:b20d with SMTP id g6-20020a05600c310600b003dc18deb20dmr3129698wmo.33.1678460161536; Fri, 10 Mar 2023 06:56:01 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id j26-20020a05600c1c1a00b003e8f0334db8sm189338wms.5.2023.03.10.06.56.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Mar 2023 06:56:01 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 09/10] gdb/python: Allow gdb.UnwindInfo to be created with non gdb.Value args Date: Fri, 10 Mar 2023 14:55:26 +0000 Message-Id: <9da6361cc56f184b6a44b1086d9fbeae6d30eab9.1678460067.git.aburgess@redhat.com> 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 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.7 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 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: Currently when creating a gdb.UnwindInfo object a user must call gdb.PendingFrame.create_unwind_info and pass a frame-id object. The frame-id object should have at least a 'sp' attribute, and probably a 'pc' attribute too (it can also, in some cases have a 'special' attribute). Currently all of these frame-id attributes need to be gdb.Value objects, but the only reason for that requirement is that we have some code in py-unwind.c that only handles gdb.Value objects. If instead we switch to using get_addr_from_python in py-utils.c then we will support both gdb.Value objects and also raw numbers, which might make things simpler in some cases. So, I started rewriting pyuw_object_attribute_to_pointer (in py-unwind.c) to use get_addr_from_python. However, while looking at the code I noticed a problem. The pyuw_object_attribute_to_pointer function returns a boolean flag, if everything goes OK we return true, but we return false in two cases, (1) when the attribute is not present, which might be acceptable, or might be an error, and (2) when we get an error trying to extract the attribute value, in which case a Python error will have been set. Now in pending_framepy_create_unwind_info we have this code: if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp)) { PyErr_SetString (PyExc_ValueError, _("frame_id should have 'sp' attribute.")); return NULL; } Notice how we always set an error. This will override any error that is already set. So, if you create a frame-id object that has an 'sp' attribute, but the attribute is not a gdb.Value, then currently we fail to extract the attribute value (it's not a gdb.Value) and set this error in pyuw_object_attribute_to_pointer: rc = pyuw_value_obj_to_pointer (pyo_value.get (), addr); if (!rc) PyErr_Format ( PyExc_ValueError, _("The value of the '%s' attribute is not a pointer."), attr_name); Then we return to pending_framepy_create_unwind_info and immediately override this error with the error about 'sp' being missing. This all feels very confused. Here's my proposed solution: pyuw_object_attribute_to_pointer will now return a tri-state enum, with states OK, MISSING, or ERROR. The meanings of these states are: OK - Attribute exists and was extracted fine, MISSING - Attribute doesn't exist, no Python error was set. ERROR - Attribute does exist, but there was an error while extracting it, a Python error was set. We need to update pending_framepy_create_unwind_info, the only user of pyuw_object_attribute_to_pointer, but now I think things are much clearer. Errors from lower levels are not blindly overridden with the generic meaningless error message, but we still get the "missing 'sp' attribute" error when appropriate. This change also includes the switch to get_addr_from_python which was what started this whole journey. For well behaving user code there should be no visible changes after this commit. For user code that hits an error, hopefully the new errors should be more helpful in figuring out what's gone wrong. Additionally, users can now use integers for the 'sp' and 'pc' attributes in their frame-id objects if that is useful. --- gdb/NEWS | 4 + gdb/doc/python.texi | 3 +- gdb/python/py-unwind.c | 113 ++++++++++++++----------- gdb/testsuite/gdb.python/py-unwind.exp | 36 ++++++++ gdb/testsuite/gdb.python/py-unwind.py | 6 +- 5 files changed, 111 insertions(+), 51 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 0d9049ff134..c4f7de11c6e 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -130,6 +130,10 @@ show always-read-ctf - gdb.PendingFrame.function(): Return a gdb.Symbol for the current pending frame, or None. + ** The frame-id passed to gdb.PendingFrame.create_unwind_info can + now use either an integer or a gdb.Value object for each of its + 'sp', 'pc', and 'special' attributes. + *** 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 1c4239841af..3074b5250a3 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2815,7 +2815,8 @@ this. @end table -Each attribute value should be an instance of @code{gdb.Value}. +Each attribute value should either be an instance of @code{gdb.Value} +or an integer. @end defun diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index 432a26a760a..409dbd3a470 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -132,58 +132,63 @@ extern PyTypeObject pending_frame_object_type extern PyTypeObject unwind_info_object_type CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("unwind_info_object"); -/* Convert gdb.Value instance to inferior's pointer. Return 1 on success, - 0 on failure. */ +/* An enum returned by pyuw_object_attribute_to_pointer, a function which + is used to extract an attribute from a Python object. */ -static int -pyuw_value_obj_to_pointer (PyObject *pyo_value, CORE_ADDR *addr) +enum class pyuw_get_attr_code { - int rc = 0; - struct value *value; + /* The attribute was present, and its value was successfully extracted. */ + ATTR_OK, - try - { - if ((value = value_object_to_value (pyo_value)) != NULL) - { - *addr = unpack_pointer (value->type (), - value->contents ().data ()); - rc = 1; - } - } - catch (const gdb_exception &except) - { - gdbpy_convert_exception (except); - } - return rc; -} + /* The attribute was not present, or was present and its value was None. + No Python error has been set. */ + ATTR_MISSING, -/* Get attribute from an object and convert it to the inferior's - pointer value. Return 1 if attribute exists and its value can be - converted. Otherwise, if attribute does not exist or its value is - None, return 0. In all other cases set Python error and return - 0. */ + /* The attribute was present, but there was some error while trying to + get the value from the attribute. A Python error will be set when + this is returned. */ + ATTR_ERROR, +}; -static int +/* Get the attribute named ATTR_NAME from the object PYO and convert it to + an inferior pointer value, placing the pointer in *ADDR. + + Return pyuw_get_attr_code::ATTR_OK if the attribute was present and its + value was successfully written into *ADDR. For any other return value + the contents of *ADDR are undefined. + + Return pyuw_get_attr_code::ATTR_MISSING if the attribute was not + present, or it was present but its value was None. The contents of + *ADDR are undefined in this case. No Python error will be set in this + case. + + Return pyuw_get_attr_code::ATTR_ERROR if the attribute was present, but + there was some error while extracting the attribute's value. A Python + error will be set in this case. The contents of *ADDR are undefined. */ + +static pyuw_get_attr_code pyuw_object_attribute_to_pointer (PyObject *pyo, const char *attr_name, CORE_ADDR *addr) { - int rc = 0; + if (!PyObject_HasAttrString (pyo, attr_name)) + return pyuw_get_attr_code::ATTR_MISSING; - if (PyObject_HasAttrString (pyo, attr_name)) + gdbpy_ref<> pyo_value (PyObject_GetAttrString (pyo, attr_name)); + if (pyo_value == nullptr) { - gdbpy_ref<> pyo_value (PyObject_GetAttrString (pyo, attr_name)); + gdb_assert (PyErr_Occurred ()); + return pyuw_get_attr_code::ATTR_ERROR; + } + if (pyo_value == Py_None) + return pyuw_get_attr_code::ATTR_MISSING; - if (pyo_value != NULL && pyo_value != Py_None) - { - rc = pyuw_value_obj_to_pointer (pyo_value.get (), addr); - if (!rc) - PyErr_Format ( - PyExc_ValueError, - _("The value of the '%s' attribute is not a pointer."), - attr_name); - } + if (get_addr_from_python (pyo_value.get (), addr) < 0) + { + gdb_assert (PyErr_Occurred ()); + return pyuw_get_attr_code::ATTR_ERROR; } - return rc; + + return pyuw_get_attr_code::ATTR_OK; } /* Called by the Python interpreter to obtain string representation @@ -687,13 +692,18 @@ pending_framepy_create_unwind_info (PyObject *self, PyObject *args) PENDING_FRAMEPY_REQUIRE_VALID ((pending_frame_object *) self); if (!PyArg_ParseTuple (args, "O:create_unwind_info", &pyo_frame_id)) - return NULL; - if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp)) + return nullptr; + + pyuw_get_attr_code code + = pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp); + if (code == pyuw_get_attr_code::ATTR_MISSING) { PyErr_SetString (PyExc_ValueError, _("frame_id should have 'sp' attribute.")); - return NULL; + return nullptr; } + else if (code == pyuw_get_attr_code::ATTR_ERROR) + return nullptr; /* The logic of building frame_id depending on the attributes of the frame_id object: @@ -704,13 +714,20 @@ pending_framepy_create_unwind_info (PyObject *self, PyObject *args) Y Y N frame_id_build (sp, pc) Y Y Y frame_id_build_special (sp, pc, special) */ - if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "pc", &pc)) + code = pyuw_object_attribute_to_pointer (pyo_frame_id, "pc", &pc); + if (code == pyuw_get_attr_code::ATTR_ERROR) + return nullptr; + else if (code == pyuw_get_attr_code::ATTR_MISSING) return pyuw_create_unwind_info (self, frame_id_build_wild (sp)); - if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "special", &special)) + + code = pyuw_object_attribute_to_pointer (pyo_frame_id, "special", &special); + if (code == pyuw_get_attr_code::ATTR_ERROR) + return nullptr; + else if (code == pyuw_get_attr_code::ATTR_MISSING) return pyuw_create_unwind_info (self, frame_id_build (sp, pc)); - else - return pyuw_create_unwind_info (self, - frame_id_build_special (sp, pc, special)); + + return pyuw_create_unwind_info (self, + frame_id_build_special (sp, pc, special)); } /* Implementation of PendingFrame.architecture (self) -> gdb.Architecture. */ diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp index d65b8447525..fddf4f15393 100644 --- a/gdb/testsuite/gdb.python/py-unwind.exp +++ b/gdb/testsuite/gdb.python/py-unwind.exp @@ -200,6 +200,42 @@ gdb_test "disable unwinder global \"simple\"" \ check_for_fixed_backtrace \ "check backtrace before testing PendingFrame methods" +# Turn the 'simple' unwinder back on. +gdb_test "enable unwinder global \"simple\"" \ + "1 unwinder enabled" + +# Replace the "simple" unwinder with a new version that doesn't set +# the 'sp' attribute. Also the 'pc' attribute is invalid, but we'll +# hit the missing 'sp' error first. +with_test_prefix "frame-id 'sp' is None" { + gdb_test_no_output "python obj = simple_unwinder(\"simple\", None, \"xyz\")" + gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)" + gdb_test_no_output "python captured_pending_frame = None" + gdb_test "backtrace" \ + "Python Exception : frame_id should have 'sp' attribute\\.\r\n.*" +} + +# Replace the "simple" unwinder with a new version that sets the 'sp' +# attribute to an invalid value. Also the 'pc' attribute is invalid, but we'll +# hit the invalid 'sp' error first. +with_test_prefix "frame-id 'sp' is invalid" { + gdb_test_no_output "python obj = simple_unwinder(\"simple\", \"jkl\", \"xyz\")" + gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)" + gdb_test_no_output "python captured_pending_frame = None" + gdb_test "backtrace" \ + "Python Exception : invalid literal for int\\(\\) with base 10: 'jkl'\r\n.*" +} + +# Replace the "simple" unwinder with a new version that sets the 'sp' +# to a valid value, but set the 'pc' attribute to an invalid value. +with_test_prefix "frame-id 'pc' is invalid" { + gdb_test_no_output "python obj = simple_unwinder(\"simple\", 0x123, \"xyz\")" + gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)" + gdb_test_no_output "python captured_pending_frame = None" + gdb_test "backtrace" \ + "Python Exception : invalid literal for int\\(\\) with base 10: 'xyz'\r\n.*" +} + # Gather information about every frame. gdb_test_no_output "python capture_all_frame_information()" gdb_test_no_output "python gdb.newest_frame().select()" diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py index f8f04b7f514..dbabb006e4b 100644 --- a/gdb/testsuite/gdb.python/py-unwind.py +++ b/gdb/testsuite/gdb.python/py-unwind.py @@ -141,8 +141,10 @@ captured_unwind_info_repr = None class simple_unwinder(Unwinder): - def __init__(self, name): + def __init__(self, name, sp=0x123, pc=0x456): super().__init__(name) + self._sp = sp + self._pc = pc def __call__(self, pending_frame): global captured_pending_frame @@ -155,7 +157,7 @@ class simple_unwinder(Unwinder): if captured_pending_frame is None: captured_pending_frame = pending_frame captured_pending_frame_repr = repr(pending_frame) - fid = FrameId(gdb.Value(0x123), gdb.Value(0x456)) + fid = FrameId(self._sp, self._pc) uw = pending_frame.create_unwind_info(fid) uw.add_saved_register("rip", gdb.Value(0x123)) uw.add_saved_register("rbp", gdb.Value(0x456)) -- 2.25.4