gdb/python: don't use PyObject_IsInstance in gdbpy_is_color

The gdbpy_is_color function uses PyObject_IsInstance, and converts the
return from PyObject_IsInstance to a bool.

Unfortunately, PyObject_IsInstance can return -1, 0, or 1, for error,
failure, or success respectively.  When converting to a bool both -1
and 1 will convert to true.

Additionally, when PyObject_IsInstance returns -1 an error will be
set.

What this means is that, if gdbpy_is_color is called with a non
gdb.Color object, and the PyObject_IsInstance check raises an error,
then (a) GDB will continue as if the object is a gdb.Color object,
which is likely going to invoke undefined behaviour, see
gdbpy_get_color for example, and (b) when GDB eventually returns to
the Python interpreter, due to an error being set, we'll see:

  Python Exception <class 'SystemError'>: PyEval_EvalFrameEx returned a result with an error set
  Error occurred in Python: PyEval_EvalFrameEx returned a result with an error set

However, after the previous commit, gdb.Color can no longer be
sub-classed, this means that fixing the above problems is easy, we can
replace the PyObject_IsInstance check with a PyObject_TypeCheck, the
PyObject_TypeCheck function only returns 0 or 1, there's no -1 error
case.

It's also worth noting that PyObject_TypeCheck is the function that is
more commonly used within GDB's Python API implementation, include the
py-color.c use there were only 4 PyObject_IsInstance uses.  Of the
remaining 3, 2 are fine, and one other (in py-disasm.c) is also
wrong.  I'll address that in a separate patch.

There's also a new test included which exposes the above issue.

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

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index fb4b80e41cc8de0e419795ff545a37634931c5ba..c48d14e1418ee2202b0c2d0539f4dc26ddf3ac56 100644 (file)
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -64,7 +64,8 @@ create_color_object (const ui_file_style::color &color)
 bool
 gdbpy_is_color (PyObject *obj)
 {
-  return PyObject_IsInstance (obj, (PyObject *) &colorpy_object_type);
+  gdb_assert (obj != nullptr);
+  return PyObject_TypeCheck (obj, &colorpy_object_type) != 0;
 }
 
 /* See py-color.h.  */

diff --git a/gdb/testsuite/gdb.python/py-color.exp b/gdb/testsuite/gdb.python/py-color.exp
index 88967d4d43ea5f5708f3253c99e17a366b649609..99b4689028990df098ca2e2279541bf5e64d2854 100644 (file)
--- a/gdb/testsuite/gdb.python/py-color.exp
+++ b/gdb/testsuite/gdb.python/py-color.exp
@@ -107,3 +107,26 @@ gdb_test_multiline "Try to sub-class gdb.Color" \
         "Python Exception <class 'TypeError'>: type 'gdb\\.Color' is not an acceptable base type" \
         "Error occurred in Python: type 'gdb\\.Color' is not an acceptable base type"]
 
+gdb_test_multiline "Setup a color parameter and non gdb.Color object" \
+    "python" "" \
+    "class my_param(gdb.Parameter):" "" \
+    "  def __init__(self):" "" \
+    "    super().__init__('color-param', gdb.COMMAND_NONE, gdb.PARAM_COLOR)" "" \
+    "    self.value = gdb.Color('red')" "" \
+    "color_param = my_param()" "" \
+    " " "" \
+    "class bad_type:" "" \
+    "  @property" "" \
+    "  def __class__(self):" "" \
+    "    raise RuntimeError('__class__ error for bad_type')" "" \
+    "bad_obj = bad_type()" "" \
+    "end" ""
+
+gdb_test_no_output "python color_param.value = gdb.Color('blue')" \
+    "set color parameter to blue"
+
+gdb_test "python color_param.value = bad_obj" \
+    [multi_line \
+        "Python Exception <class 'RuntimeError'>: color argument must be a gdb\\.Color object\\." \
+        "Error occurred in Python: color argument must be a gdb\\.Color object\\."] \
+    "set color parameter to a non-color type"