[PATCH v2 1/3] [gdb/python] Use gdbpy_err_fetch::{type,value} as getters

[PATCH v2 1/3] [gdb/python] Use gdbpy_err_fetch::{type,value} as getters

[Tom de Vries]

Similar to gdbpy_err_fetch::value, add a getter gdbpy_err_fetch::type, and use
both consistently to get gdbpy_err_fetch members m_error_value and
m_error_type.

Tested on aarch64-linux.
---
 gdb/python/py-utils.c        |  8 ++++----
 gdb/python/python-internal.h | 12 ++++++++++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 9382eb62a5f..4fbdb695a50 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -195,10 +195,10 @@ gdbpy_err_fetch::to_string () const
      Using str (aka PyObject_Str) will fetch the error message from
      gdb.GdbError ("message").  */
 
-  if (m_error_value.get () != nullptr && m_error_value.get () != Py_None)
-    return gdbpy_obj_to_string (m_error_value.get ());
+  if (this->value ().get () != nullptr && this->value ().get () != Py_None)
+    return gdbpy_obj_to_string (this->value ().get ());
   else
-    return gdbpy_obj_to_string (m_error_type.get ());
+    return gdbpy_obj_to_string (this->type ().get ());
 }
 
 /* See python-internal.h.  */
@@ -206,7 +206,7 @@ gdbpy_err_fetch::to_string () const
 gdb::unique_xmalloc_ptr<char>
 gdbpy_err_fetch::type_to_string () const
 {
-  return gdbpy_obj_to_string (m_error_type.get ());
+  return gdbpy_obj_to_string (this->type ().get ());
 }
 
 /* Convert a GDB exception to the appropriate Python exception.
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c68aff5340e..9ceb4aa7ed4 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -675,16 +675,24 @@ class gdbpy_err_fetch
 
   bool type_matches (PyObject *type) const
   {
-    return PyErr_GivenExceptionMatches (m_error_type.get (), type);
+    gdbpy_ref<> err_type = this->type ();
+    return PyErr_GivenExceptionMatches (err_type.get (), type);
   }
 
   /* Return a new reference to the exception value object.  */
 
-  gdbpy_ref<> value ()
+  gdbpy_ref<> value () const
   {
     return m_error_value;
   }
 
+  /* Return a new reference to the exception type object.  */
+
+  gdbpy_ref<> type () const
+  {
+    return m_error_type;
+  }
+
 private:
 
   gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;

base-commit: cdabd12b186e8e794045372b753416a18c387d7b
-- 
2.35.3

[PATCH v2 2/3] [gdb/python] Normalize exceptions in gdbpy_err_fetch

[Tom de Vries]

With python 3.12, I run into:
...
(gdb) PASS: gdb.python/py-block.exp: check variable access
python print (block['nonexistent'])^M
Python Exception <class 'KeyError'>: 'nonexistent'^M
Error occurred in Python: 'nonexistent'^M
(gdb) FAIL: gdb.python/py-block.exp: check nonexistent variable
...

The problem is that that PyErr_Fetch returns a normalized exception, while the
test-case matches the output for an unnormalized exception.

With python 3.6, PyErr_Fetch returns an unnormalized exception, and the
test passes.

Fix this by:
- updating the test-case to match the output for a normalized exception, and
- lazily forcing normalized exceptions using PyErr_NormalizeException.

Tested on aarch64-linux.
---
 gdb/python/python-internal.h          | 15 ++++++++++++++-
 gdb/testsuite/gdb.python/py-block.exp |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 9ceb4aa7ed4..30802ae2480 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -683,6 +683,18 @@ class gdbpy_err_fetch
 
   gdbpy_ref<> value () const
   {
+    if (!m_normalized)
+      {
+	PyObject *error_type, *error_value, *error_traceback;
+	error_type = m_error_type.release ();
+	error_value = m_error_value.release ();
+	error_traceback = m_error_traceback.release ();
+	PyErr_NormalizeException (&error_type, &error_value, &error_traceback);
+	m_error_type.reset (error_type);
+	m_error_value.reset (error_value);
+	m_error_traceback.reset (error_traceback);
+	m_normalized = true;
+      }
     return m_error_value;
   }
 
@@ -695,7 +707,8 @@ class gdbpy_err_fetch
 
 private:
 
-  gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
+  mutable gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
+  mutable bool m_normalized = false;
 };
 
 /* Called before entering the Python interpreter to install the
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 942611af305..99642a546a7 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -44,7 +44,7 @@ gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
 gdb_test "python print (block\['f'\].name == 'f')" "True" "check variable access"
-gdb_test "python print (block\['nonexistent'\])" ".*KeyError.*: nonexistent.*" \
+gdb_test "python print (block\['nonexistent'\])" ".*KeyError.*: 'nonexistent'.*" \
          "check nonexistent variable"
 gdb_test "python print (block\[42\])" ".*TypeError.*: Expected a string.*" \
          "check non-string key"
-- 
2.35.3

Re: [PATCH v2 2/3] [gdb/python] Normalize exceptions in gdbpy_err_fetch

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> With python 3.12, I run into:
Tom> ...
Tom> (gdb) PASS: gdb.python/py-block.exp: check variable access
Tom> python print (block['nonexistent'])^M
Tom> Python Exception <class 'KeyError'>: 'nonexistent'^M
Tom> Error occurred in Python: 'nonexistent'^M
Tom> (gdb) FAIL: gdb.python/py-block.exp: check nonexistent variable
Tom> ...

Tom> The problem is that that PyErr_Fetch returns a normalized exception, while the
Tom> test-case matches the output for an unnormalized exception.

Tom> With python 3.6, PyErr_Fetch returns an unnormalized exception, and the
Tom> test passes.

Tom> Fix this by:
Tom> - updating the test-case to match the output for a normalized exception, and
Tom> - lazily forcing normalized exceptions using PyErr_NormalizeException.

Tom> Tested on aarch64-linux.

Thanks.

Tom> -  gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
Tom> +  mutable gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
Tom> +  mutable bool m_normalized = false;

Maybe it's better not to add 'const'.
Not sure though.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH v2 2/3] [gdb/python] Normalize exceptions in gdbpy_err_fetch

[Tom de Vries]

On 3/8/24 21:21, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> With python 3.12, I run into:
> Tom> ...
> Tom> (gdb) PASS: gdb.python/py-block.exp: check variable access
> Tom> python print (block['nonexistent'])^M
> Tom> Python Exception <class 'KeyError'>: 'nonexistent'^M
> Tom> Error occurred in Python: 'nonexistent'^M
> Tom> (gdb) FAIL: gdb.python/py-block.exp: check nonexistent variable
> Tom> ...
> 
> Tom> The problem is that that PyErr_Fetch returns a normalized exception, while the
> Tom> test-case matches the output for an unnormalized exception.
> 
> Tom> With python 3.6, PyErr_Fetch returns an unnormalized exception, and the
> Tom> test passes.
> 
> Tom> Fix this by:
> Tom> - updating the test-case to match the output for a normalized exception, and
> Tom> - lazily forcing normalized exceptions using PyErr_NormalizeException.
> 
> Tom> Tested on aarch64-linux.
> 
> Thanks.
> 
> Tom> -  gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
> Tom> +  mutable gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
> Tom> +  mutable bool m_normalized = false;
> 
> Maybe it's better not to add 'const'.
> Not sure though.
> 

It's either use mutable, or drop the const on to_string.  I decided the 
former is less weird, but agreed, it's debatable.

Thanks,
- Tom

> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

[PATCH v2 3/3] [gdb/python] Handle deprecation of PyErr_{Fetch,Restore} in 3.12

[Tom de Vries]

Starting python version 3.12, PyErr_Fetch and PyErr_Restore are deprecated.

Use PyErr_GetRaisedException and PyErr_SetRaisedException instead, for
python >= 3.12.

Tested on aarch64-linux.
---
 gdb/python/python-internal.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 30802ae2480..d603b3a1b85 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -640,12 +640,18 @@ class gdbpy_err_fetch
 
   gdbpy_err_fetch ()
   {
+#if PY_VERSION_HEX < 0x030c0000
     PyObject *error_type, *error_value, *error_traceback;
 
     PyErr_Fetch (&error_type, &error_value, &error_traceback);
     m_error_type.reset (error_type);
     m_error_value.reset (error_value);
     m_error_traceback.reset (error_traceback);
+#else
+    /* PyErr_Fetch is deprecated in python 3.12, use PyErr_GetRaisedException
+       instead.  */
+    m_exc.reset (PyErr_GetRaisedException ());
+#endif
   }
 
   /* Call PyErr_Restore using the values stashed in this object.
@@ -654,9 +660,15 @@ class gdbpy_err_fetch
 
   void restore ()
   {
+#if PY_VERSION_HEX < 0x030c0000
     PyErr_Restore (m_error_type.release (),
 		   m_error_value.release (),
 		   m_error_traceback.release ());
+#else
+    /* PyErr_Restore is deprecated in python 3.12, use PyErr_SetRaisedException
+       instead.  */
+    PyErr_SetRaisedException (m_exc.release ());
+#endif
   }
 
   /* Return the string representation of the exception represented by
@@ -683,6 +695,7 @@ class gdbpy_err_fetch
 
   gdbpy_ref<> value () const
   {
+#if PY_VERSION_HEX < 0x030c0000
     if (!m_normalized)
       {
 	PyObject *error_type, *error_value, *error_traceback;
@@ -696,19 +709,32 @@ class gdbpy_err_fetch
 	m_normalized = true;
       }
     return m_error_value;
+#else
+    return m_exc;
+#endif
   }
 
   /* Return a new reference to the exception type object.  */
 
   gdbpy_ref<> type () const
   {
+#if PY_VERSION_HEX < 0x030c0000
     return m_error_type;
+#else
+    if (m_exc.get() == nullptr)
+      return nullptr;
+    return gdbpy_ref<>::new_reference ((PyObject *)Py_TYPE (m_exc.get ()));
+#endif
   }
 
 private:
 
+#if PY_VERSION_HEX < 0x030c0000
   mutable gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;
   mutable bool m_normalized = false;
+#else
+  gdbpy_ref<> m_exc;
+#endif
 };
 
 /* Called before entering the Python interpreter to install the
-- 
2.35.3

Re: [PATCH v2 3/3] [gdb/python] Handle deprecation of PyErr_{Fetch, Restore} in 3.12

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Starting python version 3.12, PyErr_Fetch and PyErr_Restore are deprecated.
Tom> Use PyErr_GetRaisedException and PyErr_SetRaisedException instead, for
Tom> python >= 3.12.

Tom> Tested on aarch64-linux.

Looks good.  Thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH v2 1/3] [gdb/python] Use gdbpy_err_fetch::{type, value} as getters

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> -  if (m_error_value.get () != nullptr && m_error_value.get () != Py_None)
Tom> -    return gdbpy_obj_to_string (m_error_value.get ());
Tom> +  if (this->value ().get () != nullptr && this->value ().get () != Py_None)
Tom> +    return gdbpy_obj_to_string (this->value ().get ());
Tom>    else
Tom> -    return gdbpy_obj_to_string (m_error_type.get ());
Tom> +    return gdbpy_obj_to_string (this->type ().get ());

I think it'd be better to introduce a local variable for this->value() here.

Tom

Re: [PATCH v2 1/3] [gdb/python] Use gdbpy_err_fetch::{type, value} as getters

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On 3/8/24 21:20, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> -  if (m_error_value.get () != nullptr && m_error_value.get () != Py_None)
> Tom> -    return gdbpy_obj_to_string (m_error_value.get ());
> Tom> +  if (this->value ().get () != nullptr && this->value ().get () != Py_None)
> Tom> +    return gdbpy_obj_to_string (this->value ().get ());
> Tom>    else
> Tom> -    return gdbpy_obj_to_string (m_error_type.get ());
> Tom> +    return gdbpy_obj_to_string (this->type ().get ());
> 
> I think it'd be better to introduce a local variable for this->value() here.


Done in attached patch.

I'll commit shortly.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-python-Use-gdbpy_err_fetch-type-value-as-getters.patch --]
[-- Type: text/x-patch, Size: 2513 bytes --]

From 097d7bb58fa3c459531ecd37a693049f2bc41488 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Fri, 8 Mar 2024 13:54:25 +0100
Subject: [PATCH 1/3] [gdb/python] Use gdbpy_err_fetch::{type,value} as getters

Similar to gdbpy_err_fetch::value, add a getter gdbpy_err_fetch::type, and use
both consistently to get gdbpy_err_fetch members m_error_value and
m_error_type.

Tested on aarch64-linux.
---
 gdb/python/py-utils.c        |  9 +++++----
 gdb/python/python-internal.h | 12 ++++++++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 9382eb62a5f..3fcf48f7998 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -195,10 +195,11 @@ gdbpy_err_fetch::to_string () const
      Using str (aka PyObject_Str) will fetch the error message from
      gdb.GdbError ("message").  */
 
-  if (m_error_value.get () != nullptr && m_error_value.get () != Py_None)
-    return gdbpy_obj_to_string (m_error_value.get ());
+  gdbpy_ref<> value = this->value ();
+  if (value.get () != nullptr && value.get () != Py_None)
+    return gdbpy_obj_to_string (value.get ());
   else
-    return gdbpy_obj_to_string (m_error_type.get ());
+    return gdbpy_obj_to_string (this->type ().get ());
 }
 
 /* See python-internal.h.  */
@@ -206,7 +207,7 @@ gdbpy_err_fetch::to_string () const
 gdb::unique_xmalloc_ptr<char>
 gdbpy_err_fetch::type_to_string () const
 {
-  return gdbpy_obj_to_string (m_error_type.get ());
+  return gdbpy_obj_to_string (this->type ().get ());
 }
 
 /* Convert a GDB exception to the appropriate Python exception.
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c68aff5340e..9ceb4aa7ed4 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -675,16 +675,24 @@ class gdbpy_err_fetch
 
   bool type_matches (PyObject *type) const
   {
-    return PyErr_GivenExceptionMatches (m_error_type.get (), type);
+    gdbpy_ref<> err_type = this->type ();
+    return PyErr_GivenExceptionMatches (err_type.get (), type);
   }
 
   /* Return a new reference to the exception value object.  */
 
-  gdbpy_ref<> value ()
+  gdbpy_ref<> value () const
   {
     return m_error_value;
   }
 
+  /* Return a new reference to the exception type object.  */
+
+  gdbpy_ref<> type () const
+  {
+    return m_error_type;
+  }
+
 private:
 
   gdbpy_ref<> m_error_type, m_error_value, m_error_traceback;

base-commit: 5cd52661808b9fdc2df56dd4c20a9a8ece72dbc1
-- 
2.35.3