[PATCH 0/2] Add new gdb.Type.signedness attribute

[PATCH 0/2] Add new gdb.Type.signedness attribute

[Andrew Burgess]

While working on another patch I realised that the Pyton API doesn't
appear to have any mechanism to ask if a gdb.Type is signed or not.

This series addresses that oversight.

All feedback welcome,

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: use ARRAY_SIZE for iterating over an array
  gdb/python: add Type.signedness attribute

 gdb/NEWS                             |  4 ++
 gdb/doc/python.texi                  | 22 ++++++++++
 gdb/python/py-type.c                 | 66 +++++++++++++++++++++++++++-
 gdb/testsuite/gdb.python/py-arch.exp | 15 ++++++-
 gdb/testsuite/gdb.python/py-type.c   |  4 ++
 gdb/testsuite/gdb.python/py-type.exp | 21 +++++++++
 6 files changed, 129 insertions(+), 3 deletions(-)

-- 
2.25.4

[PATCH 1/2] gdb: use ARRAY_SIZE for iterating over an array

[Andrew Burgess]

Make use of ARRAY_SIZE to loop over a static global array, removes the
need to have a null entry at the end of the array.

There should be no user visible changes after this commit.
---
 gdb/python/py-type.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8b17b70fbe3..ab9447a3ac6 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -113,7 +113,6 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_NAMESPACE),
   ENTRY (TYPE_CODE_DECFLOAT),
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
-  { TYPE_CODE_UNDEF, NULL }
 };
 
 \f
@@ -1453,7 +1452,7 @@ gdbpy_initialize_types (void)
   if (PyType_Ready (&type_iterator_object_type) < 0)
     return -1;
 
-  for (i = 0; pyty_codes[i].name; ++i)
+  for (i = 0; i < ARRAY_SIZE (pyty_codes); ++i)
     {
       if (PyModule_AddIntConstant (gdb_module, pyty_codes[i].name,
 				   pyty_codes[i].code) < 0)
-- 
2.25.4

[PATCH 2/2] gdb/python: add Type.signedness attribute

[Andrew Burgess]

Add a new gdb.Type.signedness attribute that describes whether a type
is signed or not.

Unfortunately types can be signed, unsigned, or neither, so this
attribute is not a boolean, but is instead a integer that takes the
value of one of three new constants:

   gdb.TYPE_SIGNEDNESS_SIGNED
   gdb.TYPE_SIGNEDNESS_UNSIGNED
   gdb.TYPE_SIGNEDNESS_NONE

There's documentation, NEWS, and a few tests for the new
functionality.
---
 gdb/NEWS                             |  4 ++
 gdb/doc/python.texi                  | 22 ++++++++++
 gdb/python/py-type.c                 | 63 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-arch.exp | 15 ++++++-
 gdb/testsuite/gdb.python/py-type.c   |  4 ++
 gdb/testsuite/gdb.python/py-type.exp | 21 ++++++++++
 6 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 13b66286876..887530364c4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,6 +94,10 @@ maint packet
      is equivalent to the existing 'maint packet' CLI command; it
      allows a user specified packet to be sent to the remote target.
 
+  ** New read-only attribute gdb.Type.signedness, which contains one
+     of three new constants, gdb.TYPE_SIGNEDNESS_UNSIGNED,
+     gdb.TYPE_SIGNEDNESS_SIGNED, or gdb.TYPE_SIGNEDNESS_NONE.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4a66c11c19d..1d5bba03f5d 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1211,6 +1211,28 @@
 there is no associated objfile.
 @end defvar
 
+@defvar Type.signedness
+A constant describing the signedness of this type.  The valid values
+for this field are defined in the @code{gdb} module:
+
+@vtable @code
+@vindex TYPE_SIGNEDNESS_UNSIGNED
+@item gdb.TYPE_SIGNEDNESS_UNSIGNED
+This is an unsigned type.
+
+@vindex TYPE_SIGNEDNESS_SIGNED
+@item gdb.TYPE_SIGNEDNESS_SIGNED
+This is a signed type.
+
+@vindex TYPE_SIGNEDNESS_NONE
+@item gdb.TYPE_SIGNEDNESS_NONE
+This type is considered neither signed, or unsigned.  One example of
+where this can be seen is in C++, where @code{char}, @code{signed
+char}, and @code{unsigned char} are all considered distinct types.
+
+@end vtable
+@end defvar
+
 The following methods are provided:
 
 @defun Type.fields ()
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index ab9447a3ac6..cd3df7c0b66 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -115,6 +115,40 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
 };
 
+#undef ENTRY
+
+/* Constants representing the possible signedness states.  Every type is
+   in one of these states.  */
+
+enum py_type_signedness
+{
+  PY_TYPE_SIGNEDNESS_SIGNED,
+  PY_TYPE_SIGNEDNESS_UNSIGNED,
+  PY_TYPE_SIGNEDNESS_NONE,
+};
+
+/* This is used to initialize the gdb.TYPE_SIGNEDNESS_ constants.  */
+
+struct pyty_signedness
+{
+  /* The code.  */
+  enum py_type_signedness signedness;
+  /* The name.  */
+  const char *name;
+};
+
+/* A table mapping the PY_TYPE_SIGNEDNESS_ constants onto a string that
+   will be exported into Python as a constant in the gdb module.  */
+
+static struct pyty_signedness pyty_signedness_constants[] =
+{
+#define ENTRY(X) { PY_ ## X, #X }
+  ENTRY (TYPE_SIGNEDNESS_SIGNED),
+  ENTRY (TYPE_SIGNEDNESS_UNSIGNED),
+  ENTRY (TYPE_SIGNEDNESS_NONE),
+#undef ENTRY
+};
+
 \f
 
 static void
@@ -433,6 +467,24 @@ typy_get_objfile (PyObject *self, void *closure)
   return objfile_to_objfile_object (objfile).release ();
 }
 
+/* Return the signedness constant for this type.  */
+
+static PyObject *
+typy_get_signedness (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  enum py_type_signedness signedness;
+  if (type->is_unsigned ())
+    signedness = PY_TYPE_SIGNEDNESS_UNSIGNED;
+  else if (type->has_no_signedness ())
+    signedness = PY_TYPE_SIGNEDNESS_NONE;
+  else
+    signedness = PY_TYPE_SIGNEDNESS_SIGNED;
+
+  return gdb_py_object_from_longest (signedness).release ();
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1459,6 +1511,15 @@ gdbpy_initialize_types (void)
 	return -1;
     }
 
+  /* Place the gdb.TYPE_SIGNEDNESS_ constants into the gdb module.  */
+  for (i = 0; i < ARRAY_SIZE (pyty_signedness_constants); ++i)
+    {
+      if (PyModule_AddIntConstant (gdb_module,
+				   pyty_signedness_constants[i].name,
+				   pyty_signedness_constants[i].signedness) < 0)
+	return -1;
+    }
+
   if (gdb_pymodule_addobject (gdb_module, "Type",
 			      (PyObject *) &type_object_type) < 0)
     return -1;
@@ -1489,6 +1550,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The tag name for this type, or None.", NULL },
   { "objfile", typy_get_objfile, NULL,
     "The objfile this type was defined in, or None.", NULL },
+  { "signedness", typy_get_signedness, NULL,
+    "The signedness for this type.", NULL },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 14dc1bf85ee..a919544f064 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -64,12 +64,25 @@ if { ![is_address_zero_readable] } {
 }
 
 foreach size {0 1 2 3 4 8 16} {
-    foreach sign {"" ", True" ", False" ", None" ", \"blah\""} {
+    foreach sign_data {{"" TYPE_SIGNEDNESS_SIGNED} \
+			   {", True" TYPE_SIGNEDNESS_SIGNED} \
+			   {", False" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", None" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", \"blah\"" TYPE_SIGNEDNESS_SIGNED}} {
+	set sign [lindex $sign_data 0]
+	# GDB's 0 bit type is always signed.
+	if { $size == 0 } {
+	    set sign_result TYPE_SIGNEDNESS_SIGNED
+	} else {
+	    set sign_result [lindex $sign_data 1]
+	}
 	set fullsize [expr 8 * $size]
 	gdb_test_no_output "python t = arch.integer_type($fullsize$sign)" \
 	    "get integer type for $size$sign"
 	gdb_test "python print(t.sizeof)" "$size" \
 	    "print size of integer type for $size$sign"
+	gdb_test "python print(t.signedness == gdb.${sign_result})" "True" \
+	    "check signedness of type for $size$sign"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index a599d0c901e..c978e840287 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -70,6 +70,10 @@ struct Temargs
 
 Temargs<D, 23, &C::c> temvar;
 
+unsigned char global_unsigned_char;
+char global_char;
+signed char global_signed_char;
+
 #endif
 
 enum E
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 733b25d3210..0e7b4b64fa3 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,17 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Check the signedness of some types.
+proc test_signedness {lang} {
+    if {$lang == "c++"} {
+      gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+      gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+      gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    }
+
+    gdb_test "python print (gdb.parse_and_eval ('ss.x').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+}
+
 # Perform C Tests.
 if { [build_inferior "${binfile}" "c"] == 0 } {
   restart_gdb "${binfile}"
@@ -292,10 +303,19 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
   gdb_test "python print(gdb.parse_and_eval('aligncheck').type.alignof)" \
       $sint
 
+  # Ensure that all of the type signedness constants have different values.
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_UNSIGNED)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_UNSIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+
   with_test_prefix "lang_c" {
       runto_bp "break to inspect struct and array."
       test_fields "c"
       test_enums
+      test_signedness "c"
   }
 }
 
@@ -310,5 +330,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_range
       test_template
       test_enums
+      test_signedness "c++"
   }
 }
-- 
2.25.4

Re: [PATCH 1/2] gdb: use ARRAY_SIZE for iterating over an array

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Make use of ARRAY_SIZE to loop over a static global array, removes the
Andrew> need to have a null entry at the end of the array.

This looks fine.

Andrew> -  for (i = 0; pyty_codes[i].name; ++i)
Andrew> +  for (i = 0; i < ARRAY_SIZE (pyty_codes); ++i)

I think this can also be done like:

    for (const auto &item : pyty_codes)

which seems even less error-prone.

Somewhere I have patches to remove a lot of uses of ARRAY_SIZE...

Tom

Re: [PATCH 2/2] gdb/python: add Type.signedness attribute

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Add a new gdb.Type.signedness attribute that describes whether a type
Andrew> is signed or not.

Andrew> Unfortunately types can be signed, unsigned, or neither, so this
Andrew> attribute is not a boolean, but is instead a integer that takes the
Andrew> value of one of three new constants:

Andrew>    gdb.TYPE_SIGNEDNESS_SIGNED
Andrew>    gdb.TYPE_SIGNEDNESS_UNSIGNED
Andrew>    gdb.TYPE_SIGNEDNESS_NONE

It wasn't super clear to me which of these applies to non-scalar types,
like a union.  I'd guess 'NONE', but is that what really happens?

Other than that the patch looks reasonable to me.

Tom

[PATCHv2 0/2] Add new gdb.Type.signedness attribute

[Andrew Burgess]

Tom,

Thanks for the review feedback.  Here's an updated series.

Thanks,
Andrew

---

Changes since v1:

  - Use range based for loop in patches #1 and #2 instead of
    ARRAY_SIZE,

  - Force non-scalar types to return TYPE_SIGNEDNESS_NONE in patch #2,

  - Additional tests in patch #2 to cover more cases.

---

Andrew Burgess (2):
  gdb: use a range based for loop when iterating over an array
  gdb/python: add Type.signedness attribute

 gdb/NEWS                             |  4 ++
 gdb/doc/python.texi                  | 24 ++++++++++
 gdb/python/py-type.c                 | 69 +++++++++++++++++++++++++---
 gdb/testsuite/gdb.python/py-arch.exp | 15 +++++-
 gdb/testsuite/gdb.python/py-type.c   | 12 +++++
 gdb/testsuite/gdb.python/py-type.exp | 31 +++++++++++++
 6 files changed, 148 insertions(+), 7 deletions(-)

-- 
2.25.4

[PATCHv2 1/2] gdb: use a range based for loop when iterating over an array

[Andrew Burgess]

Make use of a range based for loop to iterate over a static global
array, removing the need to have a null entry at the end of the
array.

There should be no user visible changes after this commit.
---
 gdb/python/py-type.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8b17b70fbe3..15d73fe94e4 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -113,7 +113,6 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_NAMESPACE),
   ENTRY (TYPE_CODE_DECFLOAT),
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
-  { TYPE_CODE_UNDEF, NULL }
 };
 
 \f
@@ -1444,8 +1443,6 @@ _initialize_py_type ()
 int
 gdbpy_initialize_types (void)
 {
-  int i;
-
   if (PyType_Ready (&type_object_type) < 0)
     return -1;
   if (PyType_Ready (&field_object_type) < 0)
@@ -1453,10 +1450,9 @@ gdbpy_initialize_types (void)
   if (PyType_Ready (&type_iterator_object_type) < 0)
     return -1;
 
-  for (i = 0; pyty_codes[i].name; ++i)
+  for (const auto &item : pyty_codes)
     {
-      if (PyModule_AddIntConstant (gdb_module, pyty_codes[i].name,
-				   pyty_codes[i].code) < 0)
+      if (PyModule_AddIntConstant (gdb_module, item.name, item.code) < 0)
 	return -1;
     }
 
-- 
2.25.4

[PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Andrew Burgess]

Add a new gdb.Type.signedness attribute that describes whether a type
is signed or not.

Unfortunately types can be signed, unsigned, or neither, so this
attribute is not a boolean, but is instead a integer that takes the
value of one of three new constants:

   gdb.TYPE_SIGNEDNESS_SIGNED
   gdb.TYPE_SIGNEDNESS_UNSIGNED
   gdb.TYPE_SIGNEDNESS_NONE

There's documentation, NEWS, and a few tests for the new
functionality.
---
 gdb/NEWS                             |  4 ++
 gdb/doc/python.texi                  | 24 +++++++++++
 gdb/python/py-type.c                 | 61 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-arch.exp | 15 ++++++-
 gdb/testsuite/gdb.python/py-type.c   | 12 ++++++
 gdb/testsuite/gdb.python/py-type.exp | 31 ++++++++++++++
 6 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 13b66286876..887530364c4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,6 +94,10 @@ maint packet
      is equivalent to the existing 'maint packet' CLI command; it
      allows a user specified packet to be sent to the remote target.
 
+  ** New read-only attribute gdb.Type.signedness, which contains one
+     of three new constants, gdb.TYPE_SIGNEDNESS_UNSIGNED,
+     gdb.TYPE_SIGNEDNESS_SIGNED, or gdb.TYPE_SIGNEDNESS_NONE.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4a66c11c19d..d69b57f1ccf 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1211,6 +1211,30 @@
 there is no associated objfile.
 @end defvar
 
+@defvar Type.signedness
+A constant describing the signedness of this type.  The valid values
+for this field are defined in the @code{gdb} module:
+
+@vtable @code
+@vindex TYPE_SIGNEDNESS_UNSIGNED
+@item gdb.TYPE_SIGNEDNESS_UNSIGNED
+This is an unsigned type.
+
+@vindex TYPE_SIGNEDNESS_SIGNED
+@item gdb.TYPE_SIGNEDNESS_SIGNED
+This is a signed type.
+
+@vindex TYPE_SIGNEDNESS_NONE
+@item gdb.TYPE_SIGNEDNESS_NONE
+This type is considered neither signed, or unsigned.  This applies to
+all non-scalar types (e.g.@: c language structs), but can sometimes be
+the value return for scalar type, one example of where this can be
+seen is in C++, where @code{char}, @code{signed char}, and
+@code{unsigned char} are all considered distinct types.
+
+@end vtable
+@end defvar
+
 The following methods are provided:
 
 @defun Type.fields ()
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 15d73fe94e4..f33833bda58 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -115,6 +115,40 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
 };
 
+#undef ENTRY
+
+/* Constants representing the possible signedness states.  Every type is
+   in one of these states.  */
+
+enum py_type_signedness
+{
+  PY_TYPE_SIGNEDNESS_SIGNED,
+  PY_TYPE_SIGNEDNESS_UNSIGNED,
+  PY_TYPE_SIGNEDNESS_NONE,
+};
+
+/* This is used to initialize the gdb.TYPE_SIGNEDNESS_ constants.  */
+
+struct pyty_signedness
+{
+  /* The code.  */
+  enum py_type_signedness signedness;
+  /* The name.  */
+  const char *name;
+};
+
+/* A table mapping the PY_TYPE_SIGNEDNESS_ constants onto a string that
+   will be exported into Python as a constant in the gdb module.  */
+
+static struct pyty_signedness pyty_signedness_constants[] =
+{
+#define ENTRY(X) { PY_ ## X, #X }
+  ENTRY (TYPE_SIGNEDNESS_SIGNED),
+  ENTRY (TYPE_SIGNEDNESS_UNSIGNED),
+  ENTRY (TYPE_SIGNEDNESS_NONE),
+#undef ENTRY
+};
+
 \f
 
 static void
@@ -433,6 +467,24 @@ typy_get_objfile (PyObject *self, void *closure)
   return objfile_to_objfile_object (objfile).release ();
 }
 
+/* Return the signedness constant for this type.  */
+
+static PyObject *
+typy_get_signedness (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  enum py_type_signedness signedness;
+  if (!is_scalar_type (type) || type->has_no_signedness ())
+    signedness = PY_TYPE_SIGNEDNESS_NONE;
+  else if (type->is_unsigned ())
+    signedness = PY_TYPE_SIGNEDNESS_UNSIGNED;
+  else
+    signedness = PY_TYPE_SIGNEDNESS_SIGNED;
+
+  return gdb_py_object_from_longest (signedness).release ();
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1456,6 +1508,13 @@ gdbpy_initialize_types (void)
 	return -1;
     }
 
+  /* Place the gdb.TYPE_SIGNEDNESS_ constants into the gdb module.  */
+  for (const auto &item : pyty_signedness_constants)
+    {
+      if (PyModule_AddIntConstant (gdb_module, item.name, item.signedness) < 0)
+	return -1;
+    }
+
   if (gdb_pymodule_addobject (gdb_module, "Type",
 			      (PyObject *) &type_object_type) < 0)
     return -1;
@@ -1486,6 +1545,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The tag name for this type, or None.", NULL },
   { "objfile", typy_get_objfile, NULL,
     "The objfile this type was defined in, or None.", NULL },
+  { "signedness", typy_get_signedness, NULL,
+    "The signedness for this type.", NULL },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 14dc1bf85ee..a919544f064 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -64,12 +64,25 @@ if { ![is_address_zero_readable] } {
 }
 
 foreach size {0 1 2 3 4 8 16} {
-    foreach sign {"" ", True" ", False" ", None" ", \"blah\""} {
+    foreach sign_data {{"" TYPE_SIGNEDNESS_SIGNED} \
+			   {", True" TYPE_SIGNEDNESS_SIGNED} \
+			   {", False" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", None" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", \"blah\"" TYPE_SIGNEDNESS_SIGNED}} {
+	set sign [lindex $sign_data 0]
+	# GDB's 0 bit type is always signed.
+	if { $size == 0 } {
+	    set sign_result TYPE_SIGNEDNESS_SIGNED
+	} else {
+	    set sign_result [lindex $sign_data 1]
+	}
 	set fullsize [expr 8 * $size]
 	gdb_test_no_output "python t = arch.integer_type($fullsize$sign)" \
 	    "get integer type for $size$sign"
 	gdb_test "python print(t.sizeof)" "$size" \
 	    "print size of integer type for $size$sign"
+	gdb_test "python print(t.signedness == gdb.${sign_result})" "True" \
+	    "check signedness of type for $size$sign"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index a599d0c901e..7a05fd356cf 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -32,6 +32,13 @@ TS ts;
 
 int aligncheck;
 
+union UU
+{
+  int i;
+  float f;
+  int a[5];
+};
+
 #ifdef __cplusplus
 struct C
 {
@@ -70,6 +77,10 @@ struct Temargs
 
 Temargs<D, 23, &C::c> temvar;
 
+unsigned char global_unsigned_char;
+char global_char;
+signed char global_signed_char;
+
 #endif
 
 enum E
@@ -91,6 +102,7 @@ main ()
   int ar[2] = {1,2};
   struct s st;
   struct SS ss;
+  union UU uu;
 #ifdef __cplusplus
   C c;
   c.c = 1;
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 733b25d3210..982b0bbf261 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,27 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Check the signedness of some types.
+proc test_signedness {lang} {
+    if {$lang == "c++"} {
+	gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('c').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('&c').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    }
+
+    gdb_test "python print (gdb.parse_and_eval ('ss.x').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('ss').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.i').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.f').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.a').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+
+    gdb_test "python print (gdb.parse_and_eval ('&ss.x').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('&uu').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+}
+
 # Perform C Tests.
 if { [build_inferior "${binfile}" "c"] == 0 } {
   restart_gdb "${binfile}"
@@ -292,10 +313,19 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
   gdb_test "python print(gdb.parse_and_eval('aligncheck').type.alignof)" \
       $sint
 
+  # Ensure that all of the type signedness constants have different values.
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_UNSIGNED)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_UNSIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+
   with_test_prefix "lang_c" {
       runto_bp "break to inspect struct and array."
       test_fields "c"
       test_enums
+      test_signedness "c"
   }
 }
 
@@ -310,5 +340,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_range
       test_template
       test_enums
+      test_signedness "c++"
   }
 }
-- 
2.25.4

Re: [PATCHv2 0/2] Add new gdb.Type.signedness attribute

[Tom Tromey]

Andrew> Thanks for the review feedback.  Here's an updated series.

Looks good, thanks.

Tom

Re: [PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Simon Marchi]

> +@vindex TYPE_SIGNEDNESS_NONE
> +@item gdb.TYPE_SIGNEDNESS_NONE
> +This type is considered neither signed, or unsigned.  This applies to
> +all non-scalar types (e.g.@: c language structs), but can sometimes be
> +the value return for scalar type, one example of where this can be
> +seen is in C++, where @code{char}, @code{signed char}, and
> +@code{unsigned char} are all considered distinct types.

I'm curious, why would type `unsigned char` not return
gdb.TYPE_SIGNEDNESS_UNSIGNED?  I see the corresponding code in
gdbtypes.c, with the m_flag_nosign flag, so I understand that you would
implementation the Python binding based on that, but I am wondering
about the historical reason.

Simon

Re: [PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-12-07 22:13:40 -0500]:

> > +@vindex TYPE_SIGNEDNESS_NONE
> > +@item gdb.TYPE_SIGNEDNESS_NONE
> > +This type is considered neither signed, or unsigned.  This applies to
> > +all non-scalar types (e.g.@: c language structs), but can sometimes be
> > +the value return for scalar type, one example of where this can be
> > +seen is in C++, where @code{char}, @code{signed char}, and
> > +@code{unsigned char} are all considered distinct types.
> 
> I'm curious, why would type `unsigned char` not return
> gdb.TYPE_SIGNEDNESS_UNSIGNED?  I see the corresponding code in
> gdbtypes.c, with the m_flag_nosign flag, so I understand that you would
> implementation the Python binding based on that, but I am wondering
> about the historical reason.

Sorry, but I don't understand the question.  I'm assuming you actually
meant to ask a slightly different question, but, just for clarity,
I'll answer your actual question first.  I added these declarations to
gdb.python/py-type.c:

  unsigned char global_unsigned_char;
  char global_char;
  signed char global_signed_char;

And these tests to gdb.python/py-type.exp:

  gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
  gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
  gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"

So you can see that `unsigned char` does return
gdb.TYPE_SIGNEDNESS_UNSIGNED.

If your question is why would `char` not return UNSIGNED, then it
appears that plain char is not defined as unsigned in the standard.
This appears to be described in 6.2.5/15 of the C language draft that
I have to hand:

  "The three types char, signed char, and unsigned char are
   collectively called the character types. The implementation shall
   define char to have the same range, representation, and behavior as
   either signed char or unsigned char.)"

But I guess the problem is that whether char is signed or unsigned is
not encoded into the DWARF.

I'll update the Python documentation to talk about C/C++ rather than
just C++ w.r.t. this state.

Hope that helps,

Andrew

[PATCHv3 2/2] gdb/python: add Type.signedness attribute

[Andrew Burgess]

* Andrew Burgess <aburgess@redhat.com> [2021-12-08 09:56:57 +0000]:

> * Simon Marchi <simon.marchi@polymtl.ca> [2021-12-07 22:13:40 -0500]:
> 
> > > +@vindex TYPE_SIGNEDNESS_NONE
> > > +@item gdb.TYPE_SIGNEDNESS_NONE
> > > +This type is considered neither signed, or unsigned.  This applies to
> > > +all non-scalar types (e.g.@: c language structs), but can sometimes be
> > > +the value return for scalar type, one example of where this can be
> > > +seen is in C++, where @code{char}, @code{signed char}, and
> > > +@code{unsigned char} are all considered distinct types.
> > 
> > I'm curious, why would type `unsigned char` not return
> > gdb.TYPE_SIGNEDNESS_UNSIGNED?  I see the corresponding code in
> > gdbtypes.c, with the m_flag_nosign flag, so I understand that you would
> > implementation the Python binding based on that, but I am wondering
> > about the historical reason.
> 
> Sorry, but I don't understand the question.  I'm assuming you actually
> meant to ask a slightly different question, but, just for clarity,
> I'll answer your actual question first.  I added these declarations to
> gdb.python/py-type.c:
> 
>   unsigned char global_unsigned_char;
>   char global_char;
>   signed char global_signed_char;
> 
> And these tests to gdb.python/py-type.exp:
> 
>   gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
>   gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
>   gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
> 
> So you can see that `unsigned char` does return
> gdb.TYPE_SIGNEDNESS_UNSIGNED.
> 
> If your question is why would `char` not return UNSIGNED, then it
> appears that plain char is not defined as unsigned in the standard.
> This appears to be described in 6.2.5/15 of the C language draft that
> I have to hand:
> 
>   "The three types char, signed char, and unsigned char are
>    collectively called the character types. The implementation shall
>    define char to have the same range, representation, and behavior as
>    either signed char or unsigned char.)"
> 
> But I guess the problem is that whether char is signed or unsigned is
> not encoded into the DWARF.
> 
> I'll update the Python documentation to talk about C/C++ rather than
> just C++ w.r.t. this state.

Below is an updated version of this patch.  The changes from v2 are:

  - Updated the docs to talk about C and C++ now,

  - Run the char tests on both C and C++.

Given the changes are so minor, I'm considering this reviewed from the
engineering side.  I'll give Eli a few more days for a docs review,
then merge this.

Thanks,
Andrew

---

commit d492d88c50273b0af8f1d26d1706db5455bcfd8f
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Nov 30 14:35:44 2021 +0000

    gdb/python: add Type.signedness attribute
    
    Add a new gdb.Type.signedness attribute that describes whether a type
    is signed or not.
    
    Unfortunately types can be signed, unsigned, or neither, so this
    attribute is not a boolean, but is instead a integer that takes the
    value of one of three new constants:
    
       gdb.TYPE_SIGNEDNESS_SIGNED
       gdb.TYPE_SIGNEDNESS_UNSIGNED
       gdb.TYPE_SIGNEDNESS_NONE
    
    There's documentation, NEWS, and a few tests for the new
    functionality.

diff --git a/gdb/NEWS b/gdb/NEWS
index 13b66286876..887530364c4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,6 +94,10 @@ maint packet
      is equivalent to the existing 'maint packet' CLI command; it
      allows a user specified packet to be sent to the remote target.
 
+  ** New read-only attribute gdb.Type.signedness, which contains one
+     of three new constants, gdb.TYPE_SIGNEDNESS_UNSIGNED,
+     gdb.TYPE_SIGNEDNESS_SIGNED, or gdb.TYPE_SIGNEDNESS_NONE.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4a66c11c19d..423d0f8b23a 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1211,6 +1211,30 @@
 there is no associated objfile.
 @end defvar
 
+@defvar Type.signedness
+A constant describing the signedness of this type.  The valid values
+for this field are defined in the @code{gdb} module:
+
+@vtable @code
+@vindex TYPE_SIGNEDNESS_UNSIGNED
+@item gdb.TYPE_SIGNEDNESS_UNSIGNED
+This is an unsigned type.
+
+@vindex TYPE_SIGNEDNESS_SIGNED
+@item gdb.TYPE_SIGNEDNESS_SIGNED
+This is a signed type.
+
+@vindex TYPE_SIGNEDNESS_NONE
+@item gdb.TYPE_SIGNEDNESS_NONE
+This type is considered neither signed, or unsigned.  This applies to
+all non-scalar types (e.g.@: C language structs), but can sometimes be
+the value return for scalar type, one example of where this can be
+seen is in C and C++, where @code{char}, @code{signed char}, and
+@code{unsigned char} are all considered distinct types.
+
+@end vtable
+@end defvar
+
 The following methods are provided:
 
 @defun Type.fields ()
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 15d73fe94e4..f33833bda58 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -115,6 +115,40 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
 };
 
+#undef ENTRY
+
+/* Constants representing the possible signedness states.  Every type is
+   in one of these states.  */
+
+enum py_type_signedness
+{
+  PY_TYPE_SIGNEDNESS_SIGNED,
+  PY_TYPE_SIGNEDNESS_UNSIGNED,
+  PY_TYPE_SIGNEDNESS_NONE,
+};
+
+/* This is used to initialize the gdb.TYPE_SIGNEDNESS_ constants.  */
+
+struct pyty_signedness
+{
+  /* The code.  */
+  enum py_type_signedness signedness;
+  /* The name.  */
+  const char *name;
+};
+
+/* A table mapping the PY_TYPE_SIGNEDNESS_ constants onto a string that
+   will be exported into Python as a constant in the gdb module.  */
+
+static struct pyty_signedness pyty_signedness_constants[] =
+{
+#define ENTRY(X) { PY_ ## X, #X }
+  ENTRY (TYPE_SIGNEDNESS_SIGNED),
+  ENTRY (TYPE_SIGNEDNESS_UNSIGNED),
+  ENTRY (TYPE_SIGNEDNESS_NONE),
+#undef ENTRY
+};
+
 \f
 
 static void
@@ -433,6 +467,24 @@ typy_get_objfile (PyObject *self, void *closure)
   return objfile_to_objfile_object (objfile).release ();
 }
 
+/* Return the signedness constant for this type.  */
+
+static PyObject *
+typy_get_signedness (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  enum py_type_signedness signedness;
+  if (!is_scalar_type (type) || type->has_no_signedness ())
+    signedness = PY_TYPE_SIGNEDNESS_NONE;
+  else if (type->is_unsigned ())
+    signedness = PY_TYPE_SIGNEDNESS_UNSIGNED;
+  else
+    signedness = PY_TYPE_SIGNEDNESS_SIGNED;
+
+  return gdb_py_object_from_longest (signedness).release ();
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1456,6 +1508,13 @@ gdbpy_initialize_types (void)
 	return -1;
     }
 
+  /* Place the gdb.TYPE_SIGNEDNESS_ constants into the gdb module.  */
+  for (const auto &item : pyty_signedness_constants)
+    {
+      if (PyModule_AddIntConstant (gdb_module, item.name, item.signedness) < 0)
+	return -1;
+    }
+
   if (gdb_pymodule_addobject (gdb_module, "Type",
 			      (PyObject *) &type_object_type) < 0)
     return -1;
@@ -1486,6 +1545,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The tag name for this type, or None.", NULL },
   { "objfile", typy_get_objfile, NULL,
     "The objfile this type was defined in, or None.", NULL },
+  { "signedness", typy_get_signedness, NULL,
+    "The signedness for this type.", NULL },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 14dc1bf85ee..a919544f064 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -64,12 +64,25 @@ if { ![is_address_zero_readable] } {
 }
 
 foreach size {0 1 2 3 4 8 16} {
-    foreach sign {"" ", True" ", False" ", None" ", \"blah\""} {
+    foreach sign_data {{"" TYPE_SIGNEDNESS_SIGNED} \
+			   {", True" TYPE_SIGNEDNESS_SIGNED} \
+			   {", False" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", None" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", \"blah\"" TYPE_SIGNEDNESS_SIGNED}} {
+	set sign [lindex $sign_data 0]
+	# GDB's 0 bit type is always signed.
+	if { $size == 0 } {
+	    set sign_result TYPE_SIGNEDNESS_SIGNED
+	} else {
+	    set sign_result [lindex $sign_data 1]
+	}
 	set fullsize [expr 8 * $size]
 	gdb_test_no_output "python t = arch.integer_type($fullsize$sign)" \
 	    "get integer type for $size$sign"
 	gdb_test "python print(t.sizeof)" "$size" \
 	    "print size of integer type for $size$sign"
+	gdb_test "python print(t.signedness == gdb.${sign_result})" "True" \
+	    "check signedness of type for $size$sign"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index a599d0c901e..d73dad37e0e 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -32,6 +32,13 @@ TS ts;
 
 int aligncheck;
 
+union UU
+{
+  int i;
+  float f;
+  int a[5];
+};
+
 #ifdef __cplusplus
 struct C
 {
@@ -72,6 +79,10 @@ Temargs<D, 23, &C::c> temvar;
 
 #endif
 
+unsigned char global_unsigned_char;
+char global_char;
+signed char global_signed_char;
+
 enum E
 { v1, v2, v3
 };
@@ -91,6 +102,7 @@ main ()
   int ar[2] = {1,2};
   struct s st;
   struct SS ss;
+  union UU uu;
 #ifdef __cplusplus
   C c;
   c.c = 1;
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 733b25d3210..f0229f30a08 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,28 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Check the signedness of some types.
+proc test_signedness {lang} {
+    if {$lang == "c++"} {
+	gdb_test "python print (gdb.parse_and_eval ('c').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('&c').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    }
+
+    gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+
+    gdb_test "python print (gdb.parse_and_eval ('ss.x').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('ss').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.i').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.f').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.a').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+
+    gdb_test "python print (gdb.parse_and_eval ('&ss.x').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('&uu').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+}
+
 # Perform C Tests.
 if { [build_inferior "${binfile}" "c"] == 0 } {
   restart_gdb "${binfile}"
@@ -292,10 +314,19 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
   gdb_test "python print(gdb.parse_and_eval('aligncheck').type.alignof)" \
       $sint
 
+  # Ensure that all of the type signedness constants have different values.
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_UNSIGNED)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_UNSIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+
   with_test_prefix "lang_c" {
       runto_bp "break to inspect struct and array."
       test_fields "c"
       test_enums
+      test_signedness "c"
   }
 }
 
@@ -310,5 +341,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_range
       test_template
       test_enums
+      test_signedness "c++"
   }
 }

Re: [PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Simon Marchi]

On 2021-12-08 04:56, Andrew Burgess wrote:
> * Simon Marchi <simon.marchi@polymtl.ca> [2021-12-07 22:13:40 -0500]:
> 
>>> +@vindex TYPE_SIGNEDNESS_NONE
>>> +@item gdb.TYPE_SIGNEDNESS_NONE
>>> +This type is considered neither signed, or unsigned.  This applies to
>>> +all non-scalar types (e.g.@: c language structs), but can sometimes be
>>> +the value return for scalar type, one example of where this can be
>>> +seen is in C++, where @code{char}, @code{signed char}, and
>>> +@code{unsigned char} are all considered distinct types.
>>
>> I'm curious, why would type `unsigned char` not return
>> gdb.TYPE_SIGNEDNESS_UNSIGNED?  I see the corresponding code in
>> gdbtypes.c, with the m_flag_nosign flag, so I understand that you would
>> implementation the Python binding based on that, but I am wondering
>> about the historical reason.
> 
> Sorry, but I don't understand the question.  I'm assuming you actually
> meant to ask a slightly different question, but, just for clarity,
> I'll answer your actual question first.  I added these declarations to
> gdb.python/py-type.c:
> 
>   unsigned char global_unsigned_char;
>   char global_char;
>   signed char global_signed_char;
> 
> And these tests to gdb.python/py-type.exp:
> 
>   gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
>   gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness == gdb.TYPE_SIGNEDNESS_NONE)" "True"
>   gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
> 
> So you can see that `unsigned char` does return
> gdb.TYPE_SIGNEDNESS_UNSIGNED.

Ah, from the sentence in the doc, I assumed that all three would return
NONE.

> 
> If your question is why would `char` not return UNSIGNED, then it
> appears that plain char is not defined as unsigned in the standard.
> This appears to be described in 6.2.5/15 of the C language draft that
> I have to hand:
> 
>   "The three types char, signed char, and unsigned char are
>    collectively called the character types. The implementation shall
>    define char to have the same range, representation, and behavior as
>    either signed char or unsigned char.)"
> 
> But I guess the problem is that whether char is signed or unsigned is
> not encoded into the DWARF.

If I look at one of my executables, compiled with gcc 11:

0x00000069:   DW_TAG_base_type
                DW_AT_byte_size [DW_FORM_data1] (0x01)
                DW_AT_encoding [DW_FORM_data1]  (DW_ATE_signed_char)
                DW_AT_name [DW_FORM_strp]       ("char")

And if I build with -funsigned-char:

0x00000069:   DW_TAG_base_type
                DW_AT_byte_size [DW_FORM_data1] (0x01)
                DW_AT_encoding [DW_FORM_data1]  (DW_ATE_unsigned_char)
                DW_AT_name [DW_FORM_strp]       ("char")

Yet, the DWARF reader does:

  if (type->code () == TYPE_CODE_INT
      && name != nullptr
      && strcmp (name, "char") == 0)
    type->set_has_no_signedness (true);

I'd like to know the rationale for this, it doesn't make sense to me.
This is pretty much orthogonal to your patch though, your patch in
itself looks good to me.

Simon

Re: [PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Yet, the DWARF reader does:

Simon>   if (type->code () == TYPE_CODE_INT
Simon>       && name != nullptr
Simon>       && strcmp (name, "char") == 0)
Simon>     type->set_has_no_signedness (true);

Simon> I'd like to know the rationale for this, it doesn't make sense to me.

I think the intent is to mirror C semantics, where 'char' in one sense
is signed or unsigned -- it may or may not sign-extend during promotion
-- but is also officially different from both 'signed char' and
'unsigned char'.  This weird quirk normally doesn't matter, but does
affect overload resolution.  I looked for uses of this flag, and
overload (and the compile feature) are the spots where it seems to be
used.

Possible the Python API shouldn't bother with this, or there should be
some separate thing for the "is this the C char type" case, because
AFAIK that's the only such "signless" case anywhere.

Tom

Re: [PATCHv2 2/2] gdb/python: add Type.signedness attribute

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-12-08 09:56:51 -0700]:

> >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Yet, the DWARF reader does:
> 
> Simon>   if (type->code () == TYPE_CODE_INT
> Simon>       && name != nullptr
> Simon>       && strcmp (name, "char") == 0)
> Simon>     type->set_has_no_signedness (true);
> 
> Simon> I'd like to know the rationale for this, it doesn't make sense to me.
> 
> I think the intent is to mirror C semantics, where 'char' in one sense
> is signed or unsigned -- it may or may not sign-extend during promotion
> -- but is also officially different from both 'signed char' and
> 'unsigned char'.  This weird quirk normally doesn't matter, but does
> affect overload resolution.  I looked for uses of this flag, and
> overload (and the compile feature) are the spots where it seems to be
> used.

I was looking at this too, here's an example I had:

  #include <cstdio>

  void
  func (char arg)
  {
    printf ("In 'char' overload\n");
  }

  void
  func (unsigned char arg)
  {
    printf ("In 'unsigned char' overload\n");
  }

  void
  func (signed char arg)
  {
    printf ("In 'signed char' overload\n");
  }

  char c1;
  unsigned char c2;
  signed char c3;

  int
  main ()
  {
    func (c1);
    func (c2);
    func (c3);
  }

This compiles fine, and each version of `func` is called once.  If I
debug this in GDB and call `func` with each of c1, c2, c3, then GDB
correctly dispatches to each of the overloads.

If the above test is modified, replacing `char` with `int` then the
test no longer compiles as two of the overloads are aliases for each
other.

> Possible the Python API shouldn't bother with this, or there should be
> some separate thing for the "is this the C char type" case, because
> AFAIK that's the only such "signless" case anywhere.

I have an updated patch that hides the no-signedness on scalars by
default, but does allow the user to ask for this information if they
really want it.  Rather than add a separate method I've included the
functionality within a single signedness() method.  Here's the info
page description for the new method:

 -- Function: Type.signedness (reveal_non_signed_scalars='False')
     Return a constant describing the signedness of this type.  The
     REVEAL_NON_SIGNED_SCALARS parameter is 'False' by default, and is
     described below.  The possible return values for this function are
     defined in the 'gdb' module:

     'gdb.TYPE_SIGNEDNESS_UNSIGNED'
          This is an unsigned type.

     'gdb.TYPE_SIGNEDNESS_SIGNED'
          This is a signed type.

     'gdb.TYPE_SIGNEDNESS_NONE'
          This type is considered neither signed, or unsigned.  This is
          returned for all non-scalar types (e.g. C language structs).

          When REVEAL_NON_SIGNED_SCALARS is 'True', this constant can be
          returned for some scalar types.  For example, in C and C++
          'char', 'signed char', and 'unsigned char' are all considered
          distinct types.  Whether a plain 'char' is consigned signed or
          unsigned depends on the compile time environment, but in some
          situations it is important to be able to distinguish between
          the three different types of 'char'.  By passing
          REVEAL_NON_SIGNED_SCALARS as 'True', this function will return
          'TYPE_SIGNEDNESS_NONE' for the 'char' case.  When
          REVEAL_NON_SIGNED_SCALARS is 'False' (the default), this
          function will return either 'TYPE_SIGNEDNESS_SIGNED' or
          'TYPE_SIGNEDNESS_UNSIGNED' depending on what the compiler
          chose.

The complete new patch is below.

Thanks,
Andrew

---

commit e725a799d4ba1fbfd6da6a72b090e44a064ebb82
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Nov 30 14:35:44 2021 +0000

    gdb/python: add Type.signedness() method
    
    Add a new gdb.Type.signedness() method that returns a constant that
    describes whether a type is signed or not.
    
    Unfortunately types can be signed, unsigned, or neither, so this
    attribute is not a boolean, but is instead a integer that takes the
    value of one of three new constants:
    
       gdb.TYPE_SIGNEDNESS_SIGNED
       gdb.TYPE_SIGNEDNESS_UNSIGNED
       gdb.TYPE_SIGNEDNESS_NONE
    
    All non-scalar types will return TYPE_SIGNEDNESS_NONE.
    
    There's also a special case relating to C/C++ 'char', which can, in
    some cases, also return TYPE_SIGNEDNESS_NONE.  This is all described
    in the manual, but, in summary, the C/C++ languages consider 'char',
    'unsigned char', and 'signed char' to be different types.  The
    compiler picks if 'char' should be implemented as signed or unsigned,
    with different compilers picking different defaults, and (usually) the
    compiler user being able to change the default if they want.
    
    In some cases, e.g. function overload resolution, it is important for
    GDB to see the three different versions of 'char' as distinct.  But in
    many other cases, the user really only cares how the type was actually
    implemented (signed or unsigned).
    
    So, the new signedness method takes an optional boolean argument.  The
    argument is False by default.  When this argument is false, GDB will
    return TYPE_SIGNEDNESS_SIGNED or TYPE_SIGNEDNESS_UNSIGNED for the
    'char' type (depending on what the compiler generated), but, if the
    optional argument is True, GDB will return TYPE_SIGNEDNESS_NONE for a
    plain 'char' in C/C++.
    
    There's documentation, NEWS, and a few tests for the new
    functionality.

diff --git a/gdb/NEWS b/gdb/NEWS
index 13b66286876..887530364c4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,6 +94,10 @@ maint packet
      is equivalent to the existing 'maint packet' CLI command; it
      allows a user specified packet to be sent to the remote target.
 
+  ** New read-only attribute gdb.Type.signedness, which contains one
+     of three new constants, gdb.TYPE_SIGNEDNESS_UNSIGNED,
+     gdb.TYPE_SIGNEDNESS_SIGNED, or gdb.TYPE_SIGNEDNESS_NONE.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4a66c11c19d..bc2a63a823e 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1378,6 +1378,42 @@
 argument or a local variable is not known.
 @end defun
 
+@defun Type.signedness (reveal_non_signed_scalars=@code{False})
+Return a constant describing the signedness of this type.  The
+@var{reveal_non_signed_scalars} parameter is @code{False} by default,
+and is described below.  The possible return values for this function
+are defined in the @code{gdb} module:
+
+@vtable @code
+@vindex TYPE_SIGNEDNESS_UNSIGNED
+@item gdb.TYPE_SIGNEDNESS_UNSIGNED
+This is an unsigned type.
+
+@vindex TYPE_SIGNEDNESS_SIGNED
+@item gdb.TYPE_SIGNEDNESS_SIGNED
+This is a signed type.
+
+@vindex TYPE_SIGNEDNESS_NONE
+@item gdb.TYPE_SIGNEDNESS_NONE
+This type is considered neither signed, or unsigned.  This is returned
+for all non-scalar types (e.g.@: C language structs).
+
+When @var{reveal_non_signed_scalars} is @code{True}, this constant can
+be returned for some scalar types.  For example, in C and C++
+@code{char}, @code{signed char}, and @code{unsigned char} are all
+considered distinct types.  Whether a plain @code{char} is consigned
+signed or unsigned depends on the compile time environment, but in
+some situations it is important to be able to distinguish between the
+three different types of @code{char}.  By passing
+@var{reveal_non_signed_scalars} as @code{True}, this function will
+return @code{TYPE_SIGNEDNESS_NONE} for the @code{char} case.  When
+@var{reveal_non_signed_scalars} is @code{False} (the default), this
+function will return either @code{TYPE_SIGNEDNESS_SIGNED} or
+@code{TYPE_SIGNEDNESS_UNSIGNED} depending on what the compiler chose.
+
+@end vtable
+@end defun
+
 Each type has a code, which indicates what category this type falls
 into.  The available type categories are represented by constants
 defined in the @code{gdb} module:
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 15d73fe94e4..07cb66239c5 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -115,6 +115,40 @@ static struct pyty_code pyty_codes[] =
   ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
 };
 
+#undef ENTRY
+
+/* Constants representing the possible signedness states.  Every type is
+   in one of these states.  */
+
+enum py_type_signedness
+{
+  PY_TYPE_SIGNEDNESS_SIGNED,
+  PY_TYPE_SIGNEDNESS_UNSIGNED,
+  PY_TYPE_SIGNEDNESS_NONE,
+};
+
+/* This is used to initialize the gdb.TYPE_SIGNEDNESS_ constants.  */
+
+struct pyty_signedness
+{
+  /* The code.  */
+  enum py_type_signedness signedness;
+  /* The name.  */
+  const char *name;
+};
+
+/* A table mapping the PY_TYPE_SIGNEDNESS_ constants onto a string that
+   will be exported into Python as a constant in the gdb module.  */
+
+static struct pyty_signedness pyty_signedness_constants[] =
+{
+#define ENTRY(X) { PY_ ## X, #X }
+  ENTRY (TYPE_SIGNEDNESS_SIGNED),
+  ENTRY (TYPE_SIGNEDNESS_UNSIGNED),
+  ENTRY (TYPE_SIGNEDNESS_NONE),
+#undef ENTRY
+};
+
 \f
 
 static void
@@ -433,6 +467,36 @@ typy_get_objfile (PyObject *self, void *closure)
   return objfile_to_objfile_object (objfile).release ();
 }
 
+/* Return the signedness constant for this type.  */
+
+static PyObject *
+typy_get_signedness (PyObject *self, PyObject *args, PyObject *kw)
+{
+  static const char *keywords[] = { "reveal_non_signed_scalars", nullptr };
+  PyObject *reveal_non_signed_scalars_obj = nullptr;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "|O", keywords,
+					&reveal_non_signed_scalars_obj))
+    return nullptr;
+
+  bool reveal_non_signed_scalars
+    = (reveal_non_signed_scalars_obj != nullptr
+       && PyObject_IsTrue (reveal_non_signed_scalars_obj));
+
+  struct type *type = ((type_object *) self)->type;
+
+  enum py_type_signedness signedness;
+  if (!is_scalar_type (type)
+      || (reveal_non_signed_scalars && type->has_no_signedness ()))
+    signedness = PY_TYPE_SIGNEDNESS_NONE;
+  else if (type->is_unsigned ())
+    signedness = PY_TYPE_SIGNEDNESS_UNSIGNED;
+  else
+    signedness = PY_TYPE_SIGNEDNESS_SIGNED;
+
+  return gdb_py_object_from_longest (signedness).release ();
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1456,6 +1520,13 @@ gdbpy_initialize_types (void)
 	return -1;
     }
 
+  /* Place the gdb.TYPE_SIGNEDNESS_ constants into the gdb module.  */
+  for (const auto &item : pyty_signedness_constants)
+    {
+      if (PyModule_AddIntConstant (gdb_module, item.name, item.signedness) < 0)
+	return -1;
+    }
+
   if (gdb_pymodule_addobject (gdb_module, "Type",
 			      (PyObject *) &type_object_type) < 0)
     return -1;
@@ -1564,6 +1635,10 @@ Each field is a gdb.Field object." },
   { "volatile", typy_volatile, METH_NOARGS,
     "volatile () -> Type\n\
 Return a volatile variant of this type" },
+  { "signedness", (PyCFunction) typy_get_signedness,
+    METH_VARARGS | METH_KEYWORDS,
+    "signedness (reveal_non_signed_scalars) -> constant.\n\
+Return a signedness constant indicating the sign of this type." },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 14dc1bf85ee..2f98c61b0e8 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -64,12 +64,25 @@ if { ![is_address_zero_readable] } {
 }
 
 foreach size {0 1 2 3 4 8 16} {
-    foreach sign {"" ", True" ", False" ", None" ", \"blah\""} {
+    foreach sign_data {{"" TYPE_SIGNEDNESS_SIGNED} \
+			   {", True" TYPE_SIGNEDNESS_SIGNED} \
+			   {", False" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", None" TYPE_SIGNEDNESS_UNSIGNED} \
+			   {", \"blah\"" TYPE_SIGNEDNESS_SIGNED}} {
+	set sign [lindex $sign_data 0]
+	# GDB's 0 bit type is always signed.
+	if { $size == 0 } {
+	    set sign_result TYPE_SIGNEDNESS_SIGNED
+	} else {
+	    set sign_result [lindex $sign_data 1]
+	}
 	set fullsize [expr 8 * $size]
 	gdb_test_no_output "python t = arch.integer_type($fullsize$sign)" \
 	    "get integer type for $size$sign"
 	gdb_test "python print(t.sizeof)" "$size" \
 	    "print size of integer type for $size$sign"
+	gdb_test "python print(t.signedness() == gdb.${sign_result})" "True" \
+	    "check signedness of type for $size$sign"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index a599d0c901e..d73dad37e0e 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -32,6 +32,13 @@ TS ts;
 
 int aligncheck;
 
+union UU
+{
+  int i;
+  float f;
+  int a[5];
+};
+
 #ifdef __cplusplus
 struct C
 {
@@ -72,6 +79,10 @@ Temargs<D, 23, &C::c> temvar;
 
 #endif
 
+unsigned char global_unsigned_char;
+char global_char;
+signed char global_signed_char;
+
 enum E
 { v1, v2, v3
 };
@@ -91,6 +102,7 @@ main ()
   int ar[2] = {1,2};
   struct s st;
   struct SS ss;
+  union UU uu;
 #ifdef __cplusplus
   C c;
   c.c = 1;
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 733b25d3210..b1e7b183a00 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,28 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Check the signedness of some types.
+proc test_signedness {lang} {
+    if {$lang == "c++"} {
+	gdb_test "python print (gdb.parse_and_eval ('c').type.signedness() == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+	gdb_test "python print (gdb.parse_and_eval ('&c').type.signedness() == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    }
+
+    gdb_test "python print (gdb.parse_and_eval ('global_unsigned_char').type.signedness() == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('global_char').type.signedness() != gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('global_signed_char').type.signedness() == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+
+    gdb_test "python print (gdb.parse_and_eval ('ss.x').type.signedness() == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('ss').type.signedness() == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu').type.signedness() == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.i').type.signedness() == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.f').type.signedness() == gdb.TYPE_SIGNEDNESS_SIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('uu.a').type.signedness() == gdb.TYPE_SIGNEDNESS_NONE)" "True"
+
+    gdb_test "python print (gdb.parse_and_eval ('&ss.x').type.signedness() == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+    gdb_test "python print (gdb.parse_and_eval ('&uu').type.signedness() == gdb.TYPE_SIGNEDNESS_UNSIGNED)" "True"
+}
+
 # Perform C Tests.
 if { [build_inferior "${binfile}" "c"] == 0 } {
   restart_gdb "${binfile}"
@@ -292,10 +314,19 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
   gdb_test "python print(gdb.parse_and_eval('aligncheck').type.alignof)" \
       $sint
 
+  # Ensure that all of the type signedness constants have different values.
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_UNSIGNED)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_SIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+  gdb_test "python print (gdb.TYPE_SIGNEDNESS_UNSIGNED == gdb.TYPE_SIGNEDNESS_NONE)" \
+      "False"
+
   with_test_prefix "lang_c" {
       runto_bp "break to inspect struct and array."
       test_fields "c"
       test_enums
+      test_signedness "c"
   }
 }
 
@@ -310,5 +341,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_range
       test_template
       test_enums
+      test_signedness "c++"
   }
 }

Re: [PATCHv2 1/2] gdb: use a range based for loop when iterating over an array

[Andrew Burgess]

aburgess@redhat.com (Andrew Burgess) writes:

> Make use of a range based for loop to iterate over a static global
> array, removing the need to have a null entry at the end of the
> array.
>
> There should be no user visible changes after this commit.

I've gone ahead and pushed just this patch from this series.

Thanks,
Andrew



> ---
>  gdb/python/py-type.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 8b17b70fbe3..15d73fe94e4 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -113,7 +113,6 @@ static struct pyty_code pyty_codes[] =
>    ENTRY (TYPE_CODE_NAMESPACE),
>    ENTRY (TYPE_CODE_DECFLOAT),
>    ENTRY (TYPE_CODE_INTERNAL_FUNCTION),
> -  { TYPE_CODE_UNDEF, NULL }
>  };
>  
>  \f
> @@ -1444,8 +1443,6 @@ _initialize_py_type ()
>  int
>  gdbpy_initialize_types (void)
>  {
> -  int i;
> -
>    if (PyType_Ready (&type_object_type) < 0)
>      return -1;
>    if (PyType_Ready (&field_object_type) < 0)
> @@ -1453,10 +1450,9 @@ gdbpy_initialize_types (void)
>    if (PyType_Ready (&type_iterator_object_type) < 0)
>      return -1;
>  
> -  for (i = 0; pyty_codes[i].name; ++i)
> +  for (const auto &item : pyty_codes)
>      {
> -      if (PyModule_AddIntConstant (gdb_module, pyty_codes[i].name,
> -				   pyty_codes[i].code) < 0)
> +      if (PyModule_AddIntConstant (gdb_module, item.name, item.code) < 0)
>  	return -1;
>      }
>  
> -- 
> 2.25.4

[PATCHv3 0/3] Add Type.is_scalar and Type.is_signed properties

[Andrew Burgess]

Thanks for all the feedback on V1, and V2.  Additionally, Simon gave
some awesome suggestions on IRC

The new V3 series is a complete rewrite, main points are:

  - New Type.is_scalar property in patch #1,

  - Now have a Type.is_signed property (patch #2), which is either
    True or False.  Attempting to read this property on a non-scalar
    type will raise a ValueError.

  - Finally, patch #3 adds a test for to cover the 'char' is different
    to 'signed char' and 'unsigned char' issue.  This characteristic
    should be detected by simply comparing types.

Thoughts?

Thanks,
Andrew

---

Andrew Burgess (3):
  gdb/python: add Type.is_scalar property
  gdb/python: add Type.is_signed property
  gdb/testsuite: add new test for comparing char types in Python

 gdb/NEWS                             |  8 +++
 gdb/doc/python.texi                  | 16 +++++
 gdb/python/py-type.c                 | 38 ++++++++++++
 gdb/testsuite/gdb.python/py-arch.exp | 15 ++++-
 gdb/testsuite/gdb.python/py-type.c   | 16 +++++
 gdb/testsuite/gdb.python/py-type.exp | 88 +++++++++++++++++++++++++++-
 6 files changed, 179 insertions(+), 2 deletions(-)

-- 
2.25.4

[PATCHv3 1/3] gdb/python: add Type.is_scalar property

[Andrew Burgess]

Add a new read-only property which is True for scalar types,
otherwise, it's False.
---
 gdb/NEWS                             |  3 +++
 gdb/doc/python.texi                  |  6 ++++++
 gdb/python/py-type.c                 | 15 +++++++++++++++
 gdb/testsuite/gdb.python/py-type.c   | 16 ++++++++++++++++
 gdb/testsuite/gdb.python/py-type.exp | 26 +++++++++++++++++++++++++-
 5 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index fdd42049994..874ec94b8a2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -187,6 +187,9 @@ GNU/Linux/LoongArch    loongarch*-*-linux*
      set styling').  When false, which is the default if the argument
      is not given, then no styling is applied to the returned string.
 
+  ** New read-only attribute gdb.Type.is_scalar, which is True for
+     scalar types, and False for all other types.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index c1a3f5f2a7e..fa14dacf3ad 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1263,6 +1263,12 @@
 there is no associated objfile.
 @end defvar
 
+@defvar Type.is_scalar
+This property is @code{True} if the type is a scalar type, otherwise,
+this property is @code{False}.  Examples of non-scalar types include
+structures, unions, and classes.
+@end defvar
+
 The following methods are provided:
 
 @defun Type.fields ()
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 13dae1e2559..54761236d52 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -433,6 +433,19 @@ typy_get_objfile (PyObject *self, void *closure)
   return objfile_to_objfile_object (objfile).release ();
 }
 
+/* Return true if this is a scalar type, otherwise, returns false.  */
+
+static PyObject *
+typy_is_scalar (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  if (is_scalar_type (type))
+    Py_RETURN_TRUE;
+  else
+    Py_RETURN_FALSE;
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1487,6 +1500,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The tag name for this type, or None.", NULL },
   { "objfile", typy_get_objfile, NULL,
     "The objfile this type was defined in, or None.", NULL },
+  { "is_scalar", typy_is_scalar, nullptr,
+    "Is this a scalar type?", nullptr },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index 10cbd3b0875..92297d5ad4b 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -32,6 +32,13 @@ TS ts;
 
 int aligncheck;
 
+union UU
+{
+  int i;
+  float f;
+  int a[5];
+};
+
 #ifdef __cplusplus
 struct C
 {
@@ -72,6 +79,14 @@ Temargs<D, 23, &C::c> temvar;
 
 #endif
 
+unsigned char global_unsigned_char;
+char global_char;
+signed char global_signed_char;
+
+unsigned int global_unsigned_int;
+int global_int;
+signed int global_signed_int;
+
 enum E
 { v1, v2, v3
 };
@@ -91,6 +106,7 @@ main ()
   int ar[2] = {1,2};
   struct s st;
   struct SS ss;
+  union UU uu;
 #ifdef __cplusplus
   C c;
   c.c = 1;
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 4990eeb7ddb..2bb2bf67218 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,29 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Test the gdb.Type.is_scalar property.
+proc test_is_scalar { lang } {
+    if {$lang == "c++"} {
+	gdb_test "python print(gdb.parse_and_eval ('c').type.is_scalar)" "False"
+	gdb_test "python print(gdb.parse_and_eval ('&c').type.is_scalar)" "True"
+    }
+
+    foreach type { char int } {
+	gdb_test "python print(gdb.parse_and_eval('global_unsigned_${type}').type.is_scalar)" "True"
+	gdb_test "python print(gdb.parse_and_eval('global_${type}').type.is_scalar)" "True"
+	gdb_test "python print(gdb.parse_and_eval('global_signed_${type}').type.is_scalar)" "True"
+    }
+
+    gdb_test "python print(gdb.parse_and_eval ('ss.x').type.is_scalar)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('ss').type.is_scalar)" "False"
+    gdb_test "python print(gdb.parse_and_eval ('uu').type.is_scalar)" "False"
+
+    gdb_test "python print(gdb.parse_and_eval ('uu.i').type.is_scalar)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('uu.f').type.is_scalar)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('uu.a').type.is_scalar)"  "False"
+    gdb_test "python print(gdb.parse_and_eval ('&ss.x').type.is_scalar)" "True"
+}
+
 # Perform C Tests.
 if { [build_inferior "${binfile}" "c"] == 0 } {
   restart_gdb "${binfile}"
@@ -296,10 +319,10 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
       runto_bp "break to inspect struct and array."
       test_fields "c"
       test_enums
+      test_is_scalar "c"
   }
 }
 
-
 # Perform C++ Tests.
 if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
   restart_gdb "${binfile}-cxx"
@@ -310,5 +333,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_range
       test_template
       test_enums
+      test_is_scalar "c++"
   }
 }
-- 
2.25.4

[PATCHv3 2/3] gdb/python: add Type.is_signed property

[Andrew Burgess]

Add a new read-only property, Type.is_signed, which is True for signed
types, and False otherwise.

This property should only be read on types for which Type.is_scalar is
true, attempting to read this property for non-scalar types will raise
a ValueError.

I chose 'is_signed' rather than 'is_unsigned' in order to match the
existing Architecture.integer_type method, which takes a 'signed'
parameter.  As far as I could find, that was the only existing
signed/unsigned selector in the Python API, so it seemed reasonable to
stay consistent.
---
 gdb/NEWS                             |  5 +++++
 gdb/doc/python.texi                  | 10 ++++++++++
 gdb/python/py-type.c                 | 23 +++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-arch.exp | 15 ++++++++++++++-
 gdb/testsuite/gdb.python/py-type.exp | 28 ++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 874ec94b8a2..7c3cc2c8ef7 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -190,6 +190,11 @@ GNU/Linux/LoongArch    loongarch*-*-linux*
   ** New read-only attribute gdb.Type.is_scalar, which is True for
      scalar types, and False for all other types.
 
+  ** New read-only attribute gdb.Type.is_signed.  This attribute
+     should only be read when Type.is_scalar is True, and will be True
+     for signed types, and False for all other types.  Attempting to
+     read this attribute for non-scalar types will raise a ValueError.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on OpenRISC GNU/Linux.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index fa14dacf3ad..bcd7213c707 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1269,6 +1269,16 @@
 structures, unions, and classes.
 @end defvar
 
+@defvar Type.is_signed
+For scalar types (those for which @code{Type.is_scalar} is
+@code{True}), this property is @code{True} if the type is signed,
+otherwise this property is @code{False}.
+
+Attempting to read this property for a non-scalar type (a type for
+which @code{Type.is_scalar} is @code{False}), will raise a
+@code{ValueError}.
+@end defvar
+
 The following methods are provided:
 
 @defun Type.fields ()
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 54761236d52..7be3f3276c2 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -446,6 +446,27 @@ typy_is_scalar (PyObject *self, void *closure)
     Py_RETURN_FALSE;
 }
 
+/* Return true if this type is signed.  Raises a ValueError if this type
+   is not a scalar type.  */
+
+static PyObject *
+typy_is_signed (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  if (!is_scalar_type (type))
+    {
+      PyErr_SetString (PyExc_ValueError,
+		       _("Type must be a scalar type"));
+      return nullptr;
+    }
+
+  if (type->is_unsigned ())
+    Py_RETURN_FALSE;
+  else
+    Py_RETURN_TRUE;
+}
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1502,6 +1523,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The objfile this type was defined in, or None.", NULL },
   { "is_scalar", typy_is_scalar, nullptr,
     "Is this a scalar type?", nullptr },
+  { "is_signed", typy_is_signed, nullptr,
+    "Is this an signed type?", nullptr },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index b55778b0b72..58f6cb06b3e 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -64,12 +64,25 @@ if { ![is_address_zero_readable] } {
 }
 
 foreach size {0 1 2 3 4 8 16} {
-    foreach sign {"" ", True" ", False" ", None" ", \"blah\""} {
+    foreach sign_data {{"" True} \
+			   {", True" True} \
+			   {", False" False} \
+			   {", None" False} \
+			   {", \"blah\"" True}} {
+	set sign [lindex $sign_data 0]
+	# GDB's 0 bit type is always signed.
+	if { $size == 0 } {
+	    set sign_result True
+	} else {
+	    set sign_result [lindex $sign_data 1]
+	}
 	set fullsize [expr 8 * $size]
 	gdb_test_no_output "python t = arch.integer_type($fullsize$sign)" \
 	    "get integer type for $size$sign"
 	gdb_test "python print(t.sizeof)" "$size" \
 	    "print size of integer type for $size$sign"
+	gdb_test "python print(t.is_signed == ${sign_result})" "True" \
+	    "check signedness of type for $size$sign"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 2bb2bf67218..86cf8f3ff69 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,6 +270,32 @@ proc test_template {} {
     gdb_test "python print (ttype.template_argument(2))" "&C::c"
 }
 
+# Check the is_signed property of some types.
+proc test_is_signed {lang} {
+    if {$lang == "c++"} {
+	gdb_test "python print(gdb.parse_and_eval ('c').type.is_signed)"  \
+	"ValueError: Type must be a scalar type.*"
+	gdb_test "python print(gdb.parse_and_eval ('&c').type.is_signed == False)" "True"
+    }
+
+    gdb_test "python print(gdb.parse_and_eval('global_unsigned_char').type.is_signed == False)" "True"
+    gdb_test "python print(gdb.parse_and_eval('global_char').type.is_signed)" "True|False"
+    gdb_test "python print(gdb.parse_and_eval('global_signed_char').type.is_signed == True)" "True"
+
+    gdb_test "python print(gdb.parse_and_eval ('ss.x').type.is_signed == True)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('ss').type.is_signed)" \
+	"ValueError: Type must be a scalar type.*"
+    gdb_test "python print(gdb.parse_and_eval ('uu').type.is_signed)"  \
+	"ValueError: Type must be a scalar type.*"
+    gdb_test "python print(gdb.parse_and_eval ('uu.i').type.is_signed == True)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('uu.f').type.is_signed == True)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('uu.a').type.is_signed)"  \
+	"ValueError: Type must be a scalar type.*"
+
+    gdb_test "python print(gdb.parse_and_eval ('&ss.x').type.is_signed == False)" "True"
+    gdb_test "python print(gdb.parse_and_eval ('&uu').type.is_signed == False)" "True"
+}
+
 # Test the gdb.Type.is_scalar property.
 proc test_is_scalar { lang } {
     if {$lang == "c++"} {
@@ -320,6 +346,7 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
       test_fields "c"
       test_enums
       test_is_scalar "c"
+      test_is_signed "c"
   }
 }
 
@@ -334,5 +361,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_template
       test_enums
       test_is_scalar "c++"
+      test_is_signed "c++"
   }
 }
-- 
2.25.4

[PATCHv3 3/3] gdb/testsuite: add new test for comparing char types in Python

[Andrew Burgess]

There's an interesting property of the 'char' type in C and C++, the
three types 'char', 'unsigned char', and 'signed char', are all
considered distinct.

In contrast, and 'int' is signed by default, and so 'int' and 'signed
int' are considered the same type.

This commit adds a test to ensure that this edge case is visible to a
user from Python.

It is worth noting that for any particular compiler implementation (or
the flags a compiler was invoked with), a 'char' will be either signed
or unsigned; it has to be one or the other, and a user can access this
information by using the Type.is_signed property.  However, for
something like function overload resolution, the 'char' type is
considered distinct from the signed and unsigned variants.

There's no change to GDB with this commit, this is just adding a new
test to guard some existing functionality.
---
 gdb/testsuite/gdb.python/py-type.exp | 34 ++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 86cf8f3ff69..5613bc024f6 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -296,6 +296,38 @@ proc test_is_signed {lang} {
     gdb_test "python print(gdb.parse_and_eval ('&uu').type.is_signed == False)" "True"
 }
 
+# Compare the types of different symbols from the inferior, we're
+# checking that the types of different sybols of the same declared
+# type, are equal (in Python).
+proc test_type_equality {} {
+
+    foreach_with_prefix type { char int } {
+	gdb_test_no_output "python v1 = gdb.parse_and_eval('global_unsigned_${type}')"
+	gdb_test_no_output "python v2 = gdb.parse_and_eval('global_${type}')"
+	gdb_test_no_output "python v3 = gdb.parse_and_eval('global_signed_${type}')"
+
+	gdb_test_no_output "python t1 = v1.type"
+	gdb_test_no_output "python t2 = v2.type"
+	gdb_test_no_output "python t3 = v3.type"
+
+	if { $type == "char" } {
+	    # In C/C++ there's an interesting property of 'char' based types;
+	    # 'signed char', 'unsigned char', and 'char' are all distinct
+	    # types.  Weird, right?  Here we check that this property is
+	    # visible to Python code.
+	    gdb_test "python print(t1 != t2)" "True"
+	    gdb_test "python print(t1 != t3)" "True"
+	    gdb_test "python print(t2 != t3)" "True"
+	} else {
+	    # For 'int' type, when neither signed or unsigned is given
+	    # we expect the type to be signed by default.
+	    gdb_test "python print(t1 != t2)" "True"
+	    gdb_test "python print(t1 != t3)" "True"
+	    gdb_test "python print(t2 == t3)" "True"
+	}
+    }
+}
+
 # Test the gdb.Type.is_scalar property.
 proc test_is_scalar { lang } {
     if {$lang == "c++"} {
@@ -347,6 +379,7 @@ if { [build_inferior "${binfile}" "c"] == 0 } {
       test_enums
       test_is_scalar "c"
       test_is_signed "c"
+      test_type_equality
   }
 }
 
@@ -362,5 +395,6 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_enums
       test_is_scalar "c++"
       test_is_signed "c++"
+      test_type_equality
   }
 }
-- 
2.25.4

Re: [PATCHv3 1/3] gdb/python: add Type.is_scalar property

[Eli Zaretskii]

> Date: Fri, 25 Feb 2022 13:55:28 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> Add a new read-only property which is True for scalar types,
> otherwise, it's False.
> ---
>  gdb/NEWS                             |  3 +++
>  gdb/doc/python.texi                  |  6 ++++++
>  gdb/python/py-type.c                 | 15 +++++++++++++++
>  gdb/testsuite/gdb.python/py-type.c   | 16 ++++++++++++++++
>  gdb/testsuite/gdb.python/py-type.exp | 26 +++++++++++++++++++++++++-
>  5 files changed, 65 insertions(+), 1 deletion(-)

OK for the documentation parts.

Thanks.

Re: [PATCHv3 2/3] gdb/python: add Type.is_signed property

[Eli Zaretskii]

> Date: Fri, 25 Feb 2022 13:55:29 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
>  gdb/NEWS                             |  5 +++++
>  gdb/doc/python.texi                  | 10 ++++++++++
>  gdb/python/py-type.c                 | 23 +++++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-arch.exp | 15 ++++++++++++++-
>  gdb/testsuite/gdb.python/py-type.exp | 28 ++++++++++++++++++++++++++++
>  5 files changed, 80 insertions(+), 1 deletion(-)

Thanks, the documentation parts are OK.

Re: [PATCHv3 0/3] Add Type.is_scalar and Type.is_signed properties

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Thanks for all the feedback on V1, and V2.  Additionally, Simon gave
Andrew> some awesome suggestions on IRC

Andrew> The new V3 series is a complete rewrite, main points are:

Andrew>   - New Type.is_scalar property in patch #1,

Andrew>   - Now have a Type.is_signed property (patch #2), which is either
Andrew>     True or False.  Attempting to read this property on a non-scalar
Andrew>     type will raise a ValueError.

Andrew>   - Finally, patch #3 adds a test for to cover the 'char' is different
Andrew>     to 'signed char' and 'unsigned char' issue.  This characteristic
Andrew>     should be detected by simply comparing types.

Andrew> Thoughts?

It all looks reasonable to me.  Thank you.

Tom

Re: [PATCHv3 0/3] Add Type.is_scalar and Type.is_signed properties

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Thanks for all the feedback on V1, and V2.  Additionally, Simon gave
> Andrew> some awesome suggestions on IRC
>
> Andrew> The new V3 series is a complete rewrite, main points are:
>
> Andrew>   - New Type.is_scalar property in patch #1,
>
> Andrew>   - Now have a Type.is_signed property (patch #2), which is either
> Andrew>     True or False.  Attempting to read this property on a non-scalar
> Andrew>     type will raise a ValueError.
>
> Andrew>   - Finally, patch #3 adds a test for to cover the 'char' is different
> Andrew>     to 'signed char' and 'unsigned char' issue.  This characteristic
> Andrew>     should be detected by simply comparing types.
>
> Andrew> Thoughts?
>
> It all looks reasonable to me.  Thank you.

I pushed this series.

Thanks,
Andrew