Re: [patch] [python] PR python/15461 (gate architecture calls)

[Phil Muldoon <pmuldoon@redhat.com>]

On 28/08/13 16:54, Tom Tromey wrote:
> Phil> I don't disagree on the efficiency argument, but my goal here was to
> Phil> follow the pattern that other objects use to determine validity of the
> Phil> underlying GDB data. To bring a sense of uniformity to how we do
> Phil> things in Python. So how we check a gdb.Frame's, et al, validity, the
> Phil> pattern will be the same, as far as possible, for other objects.
> 
> Ok.
> 
> If you look at other ones, they set the Python exception.  At least that
> is true for py-block.c (twice), py-inferior.c, py-inthread.c,
> py-symbol.c, py-symtab.c (twice), etc.
> 
> FWIW I don't mind inconsistency in these little details.  What matters
> is the context in which the macro is most useful.

For some reason I totally misread your original comment.  I re-read
the thread this morning, and found my mistake.  Mea Cupla.  I thought
you did not want the macro, but that was not the case.  Your objection
was to the error() call, and was totally correct.  Not sure why I read
that email so incorrectly.  Apologies.

So just ignore my previous comments ;)

Updated the patch.  ChangeLog remains the same.

Cheers,

Phil

--

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 7098a8a..a31ffdd 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -30,6 +30,18 @@ typedef struct arch_object_type_object {
 
 static struct gdbarch_data *arch_object_data = NULL;
 
+/* Require a valid Architecture.  */
+#define ARCHPY_REQUIRE_VALID(arch_obj, arch)			\
+  do {								\
+    arch = arch_object_to_gdbarch (arch_obj);			\
+    if (arch == NULL)						\
+      {							\
+	PyErr_SetString (PyExc_RuntimeError,			\
+			 _("Architecture is invalid."));	\
+	return NULL;						\
+      }							\
+  } while (0)
+
 static PyTypeObject arch_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("arch_object");
 
@@ -82,9 +94,14 @@ gdbarch_to_arch_object (struct gdbarch *gdbarch)
 static PyObject *
 archpy_name (PyObject *self, PyObject *args)
 {
-  struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
-  const char *name = (gdbarch_bfd_arch_info (gdbarch))->printable_name;
-  PyObject *py_name = PyString_FromString (name);
+  struct gdbarch *gdbarch = NULL;
+  const char *name;
+  PyObject *py_name;
+
+  ARCHPY_REQUIRE_VALID (self, gdbarch);
+
+  name = (gdbarch_bfd_arch_info (gdbarch))->printable_name;
+  py_name = PyString_FromString (name);
 
   return py_name;
 }
@@ -104,7 +121,9 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
   gdb_py_ulongest start_temp;
   long count = 0, i;
   PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
-  struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+  struct gdbarch *gdbarch = NULL;
+
+  ARCHPY_REQUIRE_VALID (self, gdbarch);
 
   if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
                                     &start_temp, &end_obj, &count_obj))
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4e736b8..6fff256 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -26,6 +26,14 @@ if ![runto_main] {
    return -1
 }
 
+# Test python/15461.  Invalid architectures should not trigger an
+# internal GDB assert.
+gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
+gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
+    "Test empty architecture.name does not trigger an assert"
+gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
+    "Test empty architecture.disassemble does not trigger an assert"
+
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
 gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
 gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0