gdb/python: missing PyObject_IsTrue error check in py-arch.c

Building on the previous two commits, I was auditing our uses of
PyObject_IsTrue looking for places where we were missing an error
check.

The gdb.Architecture.integer_type() function takes a 'signed' argument
which should be a 'bool', and the docs do say:

  If SIGNED is not specified, it defaults to 'True'.  If SIGNED is
  'False', the returned type will be unsigned.

Currently we use PyObject_IsTrue, but we are missing an error check.

To fix this I've tightened the code to enforce the bool requirement at
the point that the arguments are parsed.  With that done I can remove
the call to PyObject_IsTrue and instead compare to Py_True directly,
the object in question will always be a PyBool_Type.

However, we were testing that passing a non-bool argument for 'signed'
is treated as Py_False, this was added with this commit:

  commit 90fe61ced1c9aa4afb263326e336330d15603fbf
  Date:   Mon Nov 29 13:53:06 2021 +0000

      gdb/python: don't use the 'p' format for parsing args

which is when the PyObject_IsTrue call was added.  Given that the docs
do seem pretty clear that only True or False are suitable argument
values, my proposal is that we just remove these tests and instead
test that any non-bool argument value for 'signed' gives a TypeError.

This is a breaking change to the Python API, however, my hope is that
this is such a edge case that it will not cause too many problem.
I've added a NEWS entry to highlight this change though.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>

3 files changed, 22 insertions, 8 deletions

diff --git a/gdb/NEWS b/gdb/NEWS
index 20837bb..b96ac5e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -69,6 +69,9 @@
   ** New class gdb.missing_objfile.MissingObjfileHandler which can be
      sub-classed to create handlers for missing objfiles.
 
+  ** The 'signed' argument to gdb.Architecture.integer_type() will no
+     longer accept non-bool types.
+
 * Debugger Adapter Protocol changes
 
   ** The "scopes" request will now return a scope holding global
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index d73d7fc..f7e35a4 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -269,15 +269,16 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
 {
   static const char *keywords[] = { "size", "signed", NULL };
   int size;
-  PyObject *is_signed_obj = nullptr;
+  PyObject *is_signed_obj = Py_True;
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "i|O", keywords,
-					&size, &is_signed_obj))
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "i|O!", keywords,
+					&size,
+					&PyBool_Type, &is_signed_obj))
     return nullptr;
 
   /* Assume signed by default.  */
-  bool is_signed = (is_signed_obj == nullptr
-		    || PyObject_IsTrue (is_signed_obj));
+  gdb_assert (PyBool_Check (is_signed_obj));
+  bool is_signed = is_signed_obj == Py_True;
 
   struct gdbarch *gdbarch;
   ARCHPY_REQUIRE_VALID (self, gdbarch);
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index aef4186..14802ec 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -70,9 +70,7 @@ if { ![is_address_zero_readable] } {
 foreach size {0 1 2 3 4 8 16} {
     foreach sign_data {{"" True} \
 			   {", True" True} \
-			   {", False" False} \
-			   {", None" False} \
-			   {", \"blah\"" True}} {
+			   {", False" False}} {
 	set sign [lindex $sign_data 0]
 	# GDB's 0 bit type is always signed.
 	if { $size == 0 } {
@@ -94,6 +92,18 @@ gdb_test "python arch.integer_type(95)" \
     ".*ValueError.* no integer type of that size is available.*" \
     "call integer_type with invalid size"
 
+foreach_with_prefix test_data { {None None} \
+				    {"\"blah\"" str} \
+				    {1 int} } {
+    set bad_sign [lindex $test_data 0]
+    set bad_type [lindex $test_data 1]
+    gdb_test "python arch.integer_type(8, $bad_sign)" \
+	[multi_line \
+	     "Python Exception <class 'TypeError'>: argument 2 must be bool, not $bad_type" \
+	     "Error occurred in Python: argument 2 must be bool, not $bad_type"] \
+	"check 'signed' argument can handle non-bool type $bad_type"
+}
+
 # Test for gdb.architecture_names().  First we're going to grab the
 # complete list of architecture names using the 'complete' command.
 set arch_names []