[PATCH 0/7] Standarize int-wrapping approach in Python code

[PATCH 0/7] Standarize int-wrapping approach in Python code

[Tom Tromey]

A while back I fixed a bug where a CORE_ADDR was wrapped for Python
using PyLong_FromLong, rather than using an unsigned conversion.

After doing this, I thought it would be good to audit the code for
other oddities.  Mostly what I found were possible overflows from code
that converted specifically to "int" (an issue only for Python 2 I
suppose).

This series changes the Python code in gdb to prefer two of its own
functions -- one signed and one unsigned -- which should avoid all
issues both now and in the future.  The idea here is ban PyInt_From*
and PyLong_From*, and simply always use our own wrappers.

Let me know what you think.

Tom

[PATCH 1/7] Don't use PyInt_FromSsize_t

[Tom Tromey]

Change the Python layer to avoid PyInt_FromSsize_t, and remove the
compatibility define.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python-internal.h (PyInt_FromSsize_t): Remove define.
	* python/py-record.c (recpy_element_number): Use
	gdb_py_object_from_longest.
	(recpy_gap_number): Likewise.
---
 gdb/ChangeLog                | 7 +++++++
 gdb/python/py-record.c       | 4 ++--
 gdb/python/python-internal.h | 1 -
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index a081ca942f3..a6b08dcdd83 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -374,7 +374,7 @@ recpy_element_number (PyObject *self, void* closure)
 {
   const recpy_element_object * const obj = (recpy_element_object *) self;
 
-  return PyInt_FromSsize_t (obj->number);
+  return gdb_py_object_from_longest (obj->number).release ();
 }
 
 /* Implementation of RecordInstruction.__hash__ [int] and
@@ -454,7 +454,7 @@ recpy_gap_number (PyObject *self, void *closure)
 {
   const recpy_gap_object * const obj = (const recpy_gap_object *) self;
 
-  return PyInt_FromSsize_t (obj->number);
+  return gdb_py_object_from_longest (obj->number).release ();
 }
 
 /* Implementation of RecordGap.error_code [int].  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 6874543441b..e406f37533e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -97,7 +97,6 @@
 
 #define PyInt_Check PyLong_Check
 #define PyInt_FromLong PyLong_FromLong
-#define PyInt_FromSsize_t PyLong_FromSsize_t
 #define PyInt_AsLong PyLong_AsLong
 #define PyInt_AsSsize_t PyLong_AsSsize_t
 
-- 
2.26.2

[PATCH 2/7] Don't use gdb_py_long_from_longest

[Tom Tromey]

Change the Python layer to avoid gdb_py_long_from_longest, and remove
the defines.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python-internal.h (gdb_py_long_from_longest): Remove
	defines.
	* python/py-value.c (valpy_long): Use gdb_py_object_from_longest.
	* python/py-type.c (convert_field, typy_get_sizeof): Use
	gdb_py_object_from_longest.
	* python/py-record-btrace.c (btpy_list_index): Use
	gdb_py_object_from_longest.
---
 gdb/ChangeLog                 | 10 ++++++++++
 gdb/python/py-record-btrace.c |  2 +-
 gdb/python/py-type.c          |  8 +++-----
 gdb/python/py-value.c         |  2 +-
 gdb/python/python-internal.h  |  2 --
 5 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 08613a856d0..84a3d9eae73 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -556,7 +556,7 @@ btpy_list_index (PyObject *self, PyObject *value)
   if (index < 0)
     return PyErr_Format (PyExc_ValueError, _("Not in list."));
 
-  return gdb_py_long_from_longest (index);
+  return gdb_py_object_from_longest (index).release ();
 }
 
 /* Implementation of BtraceList.count (self, value) -> int.  */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index d0dfb52811b..c5a071fee21 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -183,8 +183,7 @@ convert_field (struct type *type, int field)
 
       if (type->code () == TYPE_CODE_ENUM)
 	{
-	  arg.reset (gdb_py_long_from_longest (TYPE_FIELD_ENUMVAL (type,
-								   field)));
+	  arg = gdb_py_object_from_longest (TYPE_FIELD_ENUMVAL (type, field));
 	  attrstring = "enumval";
 	}
       else
@@ -192,8 +191,7 @@ convert_field (struct type *type, int field)
 	  if (TYPE_FIELD_LOC_KIND (type, field) == FIELD_LOC_KIND_DWARF_BLOCK)
 	    arg = gdbpy_ref<>::new_reference (Py_None);
 	  else
-	    arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
-								    field)));
+	    arg = gdb_py_object_from_longest (TYPE_FIELD_BITPOS (type, field));
 	  attrstring = "bitpos";
 	}
 
@@ -725,7 +723,7 @@ typy_get_sizeof (PyObject *self, void *closure)
 
   if (size_varies)
     Py_RETURN_NONE;
-  return gdb_py_long_from_longest (TYPE_LENGTH (type));
+  return gdb_py_object_from_longest (TYPE_LENGTH (type)).release ();
 }
 
 /* Return the alignment of the type represented by SELF, in bytes.  */
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 2ebbe0a3569..0e3b7025fae 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1733,7 +1733,7 @@ valpy_long (PyObject *self)
   if (TYPE_UNSIGNED (type))
     return gdb_py_long_from_ulongest (l);
   else
-    return gdb_py_long_from_longest (l);
+    return gdb_py_object_from_longest (l).release ();
 }
 
 /* Implements conversion to float.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index e406f37533e..b93c78fd6e1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -126,7 +126,6 @@
 #define GDB_PY_LLU_ARG "K"
 typedef PY_LONG_LONG gdb_py_longest;
 typedef unsigned PY_LONG_LONG gdb_py_ulongest;
-#define gdb_py_long_from_longest PyLong_FromLongLong
 #define gdb_py_long_from_ulongest PyLong_FromUnsignedLongLong
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLongLong
 
@@ -136,7 +135,6 @@ typedef unsigned PY_LONG_LONG gdb_py_ulongest;
 #define GDB_PY_LLU_ARG "K"
 typedef long gdb_py_longest;
 typedef unsigned long gdb_py_ulongest;
-#define gdb_py_long_from_longest PyLong_FromLong
 #define gdb_py_long_from_ulongest PyLong_FromUnsignedLong
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLong
 
-- 
2.26.2

[PATCH 3/7] Don't use gdb_py_long_from_ulongest

[Tom Tromey]

Remove the gdb_py_long_from_ulongest defines and change the Python
layer to prefer gdb_py_object_from_ulongest.  While writing this I
noticed that the error handling in archpy_disassemble was incorrect --
it could call PyDict_SetItemString with a NULL value.  This patch also
fixes this bug.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python-internal.h (gdb_py_long_from_ulongest): Remove
	defines.
	* python/py-value.c (valpy_long): Use
	gdb_py_object_from_ulongest.
	* python/py-symtab.c (salpy_get_pc): Use
	gdb_py_object_from_ulongest.
	(salpy_get_last): Likewise.
	* python/py-record-btrace.c (recpy_bt_insn_pc): Use
	gdb_py_object_from_ulongest.
	* python/py-lazy-string.c (stpy_get_address): Use
	gdb_py_object_from_ulongest.
	* python/py-frame.c (frapy_pc): Use gdb_py_object_from_ulongest.
	* python/py-arch.c (archpy_disassemble): Use
	gdb_py_object_from_ulongest and gdb_py_object_from_longest.  Fix
	error handling.
---
 gdb/ChangeLog                 | 18 ++++++++++++++++++
 gdb/python/py-arch.c          | 25 +++++++++++++++++--------
 gdb/python/py-frame.c         |  2 +-
 gdb/python/py-lazy-string.c   |  2 +-
 gdb/python/py-record-btrace.c |  2 +-
 gdb/python/py-symtab.c        |  4 ++--
 gdb/python/py-value.c         |  2 +-
 gdb/python/python-internal.h  |  2 --
 8 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index d9eaf81a30a..3f8e769ff59 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -209,14 +209,23 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
 	  return NULL;
         }
 
-      if (PyDict_SetItemString (insn_dict.get (), "addr",
-                                gdb_py_long_from_ulongest (pc))
-          || PyDict_SetItemString (insn_dict.get (), "asm",
-                                   PyString_FromString (!stb.empty ()
-							? stb.c_str ()
-							: "<unknown>"))
-          || PyDict_SetItemString (insn_dict.get (), "length",
-                                   PyInt_FromLong (insn_len)))
+      gdbpy_ref<> pc_obj = gdb_py_object_from_ulongest (pc);
+      if (pc_obj == nullptr)
+	return nullptr;
+
+      gdbpy_ref<> asm_obj (PyString_FromString (!stb.empty ()
+						? stb.c_str ()
+						: "<unknown>"));
+      if (asm_obj == nullptr)
+	return nullptr;
+
+      gdbpy_ref<> len_obj = gdb_py_object_from_longest (insn_len);
+      if (len_obj == nullptr)
+	return nullptr;
+
+      if (PyDict_SetItemString (insn_dict.get (), "addr", pc_obj.get ())
+          || PyDict_SetItemString (insn_dict.get (), "asm", asm_obj.get ())
+          || PyDict_SetItemString (insn_dict.get (), "length", len_obj.get ()))
 	return NULL;
 
       pc += insn_len;
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index e121afb222d..090705507e6 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -232,7 +232,7 @@ frapy_pc (PyObject *self, PyObject *args)
       GDB_PY_HANDLE_EXCEPTION (except);
     }
 
-  return gdb_py_long_from_ulongest (pc);
+  return gdb_py_object_from_ulongest (pc).release ();
 }
 
 /* Implementation of gdb.Frame.read_register (self, register) -> gdb.Value.
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index f71bb1d613d..401c0a6fdf5 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -61,7 +61,7 @@ stpy_get_address (PyObject *self, void *closure)
 {
   lazy_string_object *self_string = (lazy_string_object *) self;
 
-  return gdb_py_long_from_ulongest (self_string->address);
+  return gdb_py_object_from_ulongest (self_string->address).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 84a3d9eae73..762c0bcbcc0 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -232,7 +232,7 @@ recpy_bt_insn_pc (PyObject *self, void *closure)
   if (insn == NULL)
     return NULL;
 
-  return gdb_py_long_from_ulongest (insn->pc);
+  return gdb_py_object_from_ulongest (insn->pc).release ();
 }
 
 /* Implementation of RecordInstruction.size [int] for btrace.
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 6229bc5123b..b0e7618af7e 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -264,7 +264,7 @@ salpy_get_pc (PyObject *self, void *closure)
 
   SALPY_REQUIRE_VALID (self, sal);
 
-  return gdb_py_long_from_ulongest (sal->pc);
+  return gdb_py_object_from_ulongest (sal->pc).release ();
 }
 
 /* Implementation of the get method for the 'last' attribute of
@@ -278,7 +278,7 @@ salpy_get_last (PyObject *self, void *closure)
   SALPY_REQUIRE_VALID (self, sal);
 
   if (sal->end > 0)
-    return gdb_py_long_from_ulongest (sal->end - 1);
+    return gdb_py_object_from_ulongest (sal->end - 1).release ();
   else
     Py_RETURN_NONE;
 }
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 0e3b7025fae..a884d6b70b2 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1731,7 +1731,7 @@ valpy_long (PyObject *self)
     }
 
   if (TYPE_UNSIGNED (type))
-    return gdb_py_long_from_ulongest (l);
+    return gdb_py_object_from_ulongest (l).release ();
   else
     return gdb_py_object_from_longest (l).release ();
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index b93c78fd6e1..2c4195f3d03 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -126,7 +126,6 @@
 #define GDB_PY_LLU_ARG "K"
 typedef PY_LONG_LONG gdb_py_longest;
 typedef unsigned PY_LONG_LONG gdb_py_ulongest;
-#define gdb_py_long_from_ulongest PyLong_FromUnsignedLongLong
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLongLong
 
 #else /* HAVE_LONG_LONG */
@@ -135,7 +134,6 @@ typedef unsigned PY_LONG_LONG gdb_py_ulongest;
 #define GDB_PY_LLU_ARG "K"
 typedef long gdb_py_longest;
 typedef unsigned long gdb_py_ulongest;
-#define gdb_py_long_from_ulongest PyLong_FromUnsignedLong
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLong
 
 #endif /* HAVE_LONG_LONG */
-- 
2.26.2

[PATCH 4/7] Don't use PyLong_FromLong

[Tom Tromey]

This changes gdb to avoid PyLong_FromLong, preferring to
gdb_py_object_from_longest instead.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python.c (gdbpy_parameter_value): Use
	gdb_py_object_from_longest.
	* python/py-type.c (convert_field, typy_range): Use
	gdb_py_object_from_longest.
	* python/py-tui.c (gdbpy_tui_width, gdbpy_tui_height): Use
	gdb_py_object_from_longest.
	* python/py-lazy-string.c (stpy_get_length): Use
	gdb_py_object_from_longest.
	* python/py-infthread.c (thpy_get_num, thpy_get_global_num): Use
	gdb_py_object_from_longest.
	* python/py-infevents.c (create_memory_changed_event_object): Use
	gdb_py_object_from_longest.
	* python/py-inferior.c (infpy_get_num): Use
	gdb_py_object_from_longest.
	(infpy_get_pid): Likewise.
---
 gdb/ChangeLog               | 18 ++++++++++++++++++
 gdb/python/py-inferior.c    |  4 ++--
 gdb/python/py-infevents.c   |  2 +-
 gdb/python/py-infthread.c   |  8 ++++++--
 gdb/python/py-lazy-string.c |  2 +-
 gdb/python/py-tui.c         |  8 ++++++--
 gdb/python/py-type.c        |  6 +++---
 gdb/python/python.c         |  2 +-
 8 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index a7699510db7..7204356ac73 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -423,7 +423,7 @@ infpy_get_num (PyObject *self, void *closure)
 
   INFPY_REQUIRE_VALID (inf);
 
-  return PyLong_FromLong (inf->inferior->num);
+  return gdb_py_object_from_longest (inf->inferior->num).release ();
 }
 
 static PyObject *
@@ -433,7 +433,7 @@ infpy_get_pid (PyObject *self, void *closure)
 
   INFPY_REQUIRE_VALID (inf);
 
-  return PyLong_FromLong (inf->inferior->pid);
+  return gdb_py_object_from_longest (inf->inferior->pid).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 3cf746103c7..f303c395c13 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -104,7 +104,7 @@ create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
   if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
     return NULL;
 
-  gdbpy_ref<> len_obj (PyLong_FromLong (len));
+  gdbpy_ref<> len_obj = gdb_py_object_from_longest (len);
   if (len_obj == NULL)
     return NULL;
 
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index f350e367d5e..669b6d8b074 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -130,7 +130,9 @@ thpy_get_num (PyObject *self, void *closure)
 
   THPY_REQUIRE_VALID (thread_obj);
 
-  return PyLong_FromLong (thread_obj->thread->per_inf_num);
+  gdbpy_ref<> result
+    = gdb_py_object_from_longest (thread_obj->thread->per_inf_num);
+  return result.release ();
 }
 
 /* Getter for InferiorThread.global_num.  */
@@ -142,7 +144,9 @@ thpy_get_global_num (PyObject *self, void *closure)
 
   THPY_REQUIRE_VALID (thread_obj);
 
-  return PyLong_FromLong (thread_obj->thread->global_num);
+  gdbpy_ref<> result
+    = gdb_py_object_from_longest (thread_obj->thread->global_num);
+  return result.release ();
 }
 
 /* Getter for InferiorThread.ptid  -> (pid, lwp, tid).
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index 401c0a6fdf5..aaee94f5e1f 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -88,7 +88,7 @@ stpy_get_length (PyObject *self, void *closure)
 {
   lazy_string_object *self_string = (lazy_string_object *) self;
 
-  return PyLong_FromLong (self_string->length);
+  return gdb_py_object_from_longest (self_string->length).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 95c71f1d2dd..8a24f2f3af4 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -392,7 +392,9 @@ gdbpy_tui_width (PyObject *self, void *closure)
 {
   gdbpy_tui_window *win = (gdbpy_tui_window *) self;
   REQUIRE_WINDOW (win);
-  return PyLong_FromLong (win->window->viewport_width ());
+  gdbpy_ref<> result
+    = gdb_py_object_from_longest (win->window->viewport_width ());
+  return result.release ();
 }
 
 /* Return the height of the TUI window.  */
@@ -401,7 +403,9 @@ gdbpy_tui_height (PyObject *self, void *closure)
 {
   gdbpy_tui_window *win = (gdbpy_tui_window *) self;
   REQUIRE_WINDOW (win);
-  return PyLong_FromLong (win->window->viewport_height ());
+  gdbpy_ref<> result
+    = gdb_py_object_from_longest (win->window->viewport_height ());
+  return result.release ();
 }
 
 /* Return the title of the TUI window.  */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index c5a071fee21..8825b0749ce 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -233,7 +233,7 @@ convert_field (struct type *type, int field)
   if (PyObject_SetAttrString (result.get (), "is_base_class", arg.get ()) < 0)
     return NULL;
 
-  arg.reset (PyLong_FromLong (TYPE_FIELD_BITSIZE (type, field)));
+  arg = gdb_py_object_from_longest (TYPE_FIELD_BITSIZE (type, field));
   if (arg == NULL)
     return NULL;
   if (PyObject_SetAttrString (result.get (), "bitsize", arg.get ()) < 0)
@@ -596,11 +596,11 @@ typy_range (PyObject *self, PyObject *args)
       break;
     }
 
-  gdbpy_ref<> low_bound (PyLong_FromLong (low));
+  gdbpy_ref<> low_bound = gdb_py_object_from_longest (low);
   if (low_bound == NULL)
     return NULL;
 
-  gdbpy_ref<> high_bound (PyLong_FromLong (high));
+  gdbpy_ref<> high_bound = gdb_py_object_from_longest (high);
   if (high_bound == NULL)
     return NULL;
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7787dce4b4c..9cc8af63338 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -496,7 +496,7 @@ gdbpy_parameter_value (enum var_types type, void *var)
       /* Fall through.  */
     case var_zinteger:
     case var_zuinteger_unlimited:
-      return PyLong_FromLong (* (int *) var);
+      return gdb_py_object_from_longest (* (int *) var).release ();
 
     case var_uinteger:
       {
-- 
2.26.2

[PATCH 5/7] Don't use PyLong_FromLongLong

[Tom Tromey]

This changes gdb to avoid PyLong_FromLongLong, preferring to use
gdb_py_object_from_longest instead.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/py-infevents.c (create_register_changed_event_object):
	Use gdb_py_object_from_longest.
	* python/py-exitedevent.c (create_exited_event_object): Use
	gdb_py_object_from_longest.
---
 gdb/ChangeLog               | 7 +++++++
 gdb/python/py-exitedevent.c | 2 +-
 gdb/python/py-infevents.c   | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 9912b26e909..b19cc959884 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -30,7 +30,7 @@ create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
 
   if (exit_code)
     {
-      gdbpy_ref<> exit_code_obj (PyLong_FromLongLong (*exit_code));
+      gdbpy_ref<> exit_code_obj = gdb_py_object_from_longest (*exit_code);
 
       if (exit_code_obj == NULL)
 	return NULL;
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index f303c395c13..b653e4a3229 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -76,7 +76,7 @@ create_register_changed_event_object (struct frame_info *frame,
   if (evpy_add_attribute (event.get (), "frame", frame_obj.get ()) < 0)
     return NULL;
 
-  gdbpy_ref<> regnum_obj (PyLong_FromLongLong (regnum));
+  gdbpy_ref<> regnum_obj = gdb_py_object_from_longest (regnum);
   if (regnum_obj == NULL)
     return NULL;
 
-- 
2.26.2

[PATCH 6/7] Don't use PyLong_FromUnsignedLong

[Tom Tromey]

This changes gdb to avoid PyLong_FromUnsignedLong, preferring
gdb_py_object_from_ulongest instead.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python.c (gdbpy_parameter_value): Use
	gdb_py_object_from_ulongest.
---
 gdb/ChangeLog       | 5 +++++
 gdb/python/python.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9cc8af63338..878602712d8 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -504,13 +504,13 @@ gdbpy_parameter_value (enum var_types type, void *var)
 
 	if (val == UINT_MAX)
 	  Py_RETURN_NONE;
-	return PyLong_FromUnsignedLong (val);
+	return gdb_py_object_from_ulongest (val).release ();
       }
 
     case var_zuinteger:
       {
 	unsigned int val = * (unsigned int *) var;
-	return PyLong_FromUnsignedLong (val);
+	return gdb_py_object_from_ulongest (val).release ();
       }
     }
 
-- 
2.26.2

[PATCH 7/7] Don't use PyInt_FromLong

[Tom Tromey]

Avoid the use of PyInt_FromLong, preferring gdb_py_object_from_longest
instead.  I found found another spot that was incorrectly handling
errors (see gdbpy_create_ptid_object) while writing this patch; it is
fixed here.

2020-09-11  Tom Tromey  <tromey@adacore.com>

	* python/python-internal.h (PyInt_FromLong): Remove define.
	* python/py-value.c (convert_value_from_python): Use
	gdb_py_object_from_longest.
	* python/py-type.c (typy_get_code): Use
	gdb_py_object_from_longest.
	* python/py-symtab.c (salpy_get_line): Use
	gdb_py_object_from_longest.
	* python/py-symbol.c (sympy_get_addr_class, sympy_line): Use
	gdb_py_object_from_longest.
	* python/py-record.c (recpy_gap_reason_code): Use
	gdb_py_object_from_longest.
	* python/py-record-btrace.c (recpy_bt_insn_size)
	(recpy_bt_func_level, btpy_list_count): Use
	gdb_py_object_from_longest.
	* python/py-infthread.c (gdbpy_create_ptid_object): Use
	gdb_py_object_from_longest.  Fix error handling.
	* python/py-framefilter.c (bootstrap_python_frame_filters): Use
	gdb_py_object_from_longest.
	* python/py-frame.c (frapy_type, frapy_unwind_stop_reason): Use
	gdb_py_object_from_longest.
	* python/py-breakpoint.c (bppy_get_type, bppy_get_number)
	(bppy_get_thread, bppy_get_task, bppy_get_hit_count)
	(bppy_get_ignore_count): Use gdb_py_object_from_longest.
---
 gdb/ChangeLog                 | 26 ++++++++++++++++++++++++++
 gdb/python/py-breakpoint.c    | 12 ++++++------
 gdb/python/py-frame.c         |  4 ++--
 gdb/python/py-framefilter.c   |  4 ++--
 gdb/python/py-infthread.c     | 19 +++++++++++++++----
 gdb/python/py-record-btrace.c |  8 +++++---
 gdb/python/py-record.c        |  2 +-
 gdb/python/py-symbol.c        |  4 ++--
 gdb/python/py-symtab.c        |  2 +-
 gdb/python/py-type.c          |  2 +-
 gdb/python/py-value.c         |  2 +-
 gdb/python/python-internal.h  |  1 -
 12 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 11345ad3052..7369c91ad90 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -552,7 +552,7 @@ bppy_get_type (PyObject *self, void *closure)
 
   BPPY_REQUIRE_VALID (self_bp);
 
-  return PyInt_FromLong (self_bp->bp->type);
+  return gdb_py_object_from_longest (self_bp->bp->type).release ();
 }
 
 /* Python function to get the visibility of the breakpoint.  */
@@ -613,7 +613,7 @@ bppy_get_number (PyObject *self, void *closure)
 
   BPPY_REQUIRE_VALID (self_bp);
 
-  return PyInt_FromLong (self_bp->number);
+  return gdb_py_object_from_longest (self_bp->number).release ();
 }
 
 /* Python function to get the breakpoint's thread ID.  */
@@ -627,7 +627,7 @@ bppy_get_thread (PyObject *self, void *closure)
   if (self_bp->bp->thread == -1)
     Py_RETURN_NONE;
 
-  return PyInt_FromLong (self_bp->bp->thread);
+  return gdb_py_object_from_longest (self_bp->bp->thread).release ();
 }
 
 /* Python function to get the breakpoint's task ID (in Ada).  */
@@ -641,7 +641,7 @@ bppy_get_task (PyObject *self, void *closure)
   if (self_bp->bp->task == 0)
     Py_RETURN_NONE;
 
-  return PyInt_FromLong (self_bp->bp->task);
+  return gdb_py_object_from_longest (self_bp->bp->task).release ();
 }
 
 /* Python function to get the breakpoint's hit count.  */
@@ -652,7 +652,7 @@ bppy_get_hit_count (PyObject *self, void *closure)
 
   BPPY_REQUIRE_VALID (self_bp);
 
-  return PyInt_FromLong (self_bp->bp->hit_count);
+  return gdb_py_object_from_longest (self_bp->bp->hit_count).release ();
 }
 
 /* Python function to get the breakpoint's ignore count.  */
@@ -663,7 +663,7 @@ bppy_get_ignore_count (PyObject *self, void *closure)
 
   BPPY_REQUIRE_VALID (self_bp);
 
-  return PyInt_FromLong (self_bp->bp->ignore_count);
+  return gdb_py_object_from_longest (self_bp->bp->ignore_count).release ();
 }
 
 /* Internal function to validate the Python parameters/keywords
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 090705507e6..24d4fae6242 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -165,7 +165,7 @@ frapy_type (PyObject *self, PyObject *args)
       GDB_PY_HANDLE_EXCEPTION (except);
     }
 
-  return PyInt_FromLong (type);
+  return gdb_py_object_from_longest (type).release ();
 }
 
 /* Implementation of gdb.Frame.architecture (self) -> gdb.Architecture.
@@ -209,7 +209,7 @@ frapy_unwind_stop_reason (PyObject *self, PyObject *args)
 
   stop_reason = get_frame_unwind_stop_reason (frame);
 
-  return PyInt_FromLong (stop_reason);
+  return gdb_py_object_from_longest (stop_reason).release ();
 }
 
 /* Implementation of gdb.Frame.pc (self) -> Long.
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 45bf51b935c..d0348b5aeff 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1098,11 +1098,11 @@ bootstrap_python_frame_filters (struct frame_info *frame,
   if (sort_func == NULL)
     return NULL;
 
-  gdbpy_ref<> py_frame_low (PyInt_FromLong (frame_low));
+  gdbpy_ref<> py_frame_low = gdb_py_object_from_longest (frame_low);
   if (py_frame_low == NULL)
     return NULL;
 
-  gdbpy_ref<> py_frame_high (PyInt_FromLong (frame_high));
+  gdbpy_ref<> py_frame_high = gdb_py_object_from_longest (frame_high);
   if (py_frame_high == NULL)
     return NULL;
 
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 669b6d8b074..fec7bcad30f 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -307,10 +307,21 @@ gdbpy_create_ptid_object (ptid_t ptid)
   lwp = ptid.lwp ();
   tid = ptid.tid ();
 
-  PyTuple_SET_ITEM (ret, 0, PyInt_FromLong (pid));
-  PyTuple_SET_ITEM (ret, 1, PyInt_FromLong (lwp));
-  PyTuple_SET_ITEM (ret, 2, PyInt_FromLong (tid));
- 
+  gdbpy_ref<> pid_obj = gdb_py_object_from_longest (pid);
+  if (pid_obj == nullptr)
+    return nullptr;
+  gdbpy_ref<> lwp_obj = gdb_py_object_from_longest (lwp);
+  if (lwp_obj == nullptr)
+    return nullptr;
+  gdbpy_ref<> tid_obj = gdb_py_object_from_longest (tid);
+  if (tid_obj == nullptr)
+    return nullptr;
+
+  /* Note that these steal references, hence the use of 'release'.  */
+  PyTuple_SET_ITEM (ret, 0, pid_obj.release ());
+  PyTuple_SET_ITEM (ret, 1, lwp_obj.release ());
+  PyTuple_SET_ITEM (ret, 2, tid_obj.release ());
+
   return ret;
 }
 
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 762c0bcbcc0..15cd15bb0dc 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -246,7 +246,7 @@ recpy_bt_insn_size (PyObject *self, void *closure)
   if (insn == NULL)
     return NULL;
 
-  return PyInt_FromLong (insn->size);
+  return gdb_py_object_from_longest (insn->size).release ();
 }
 
 /* Implementation of RecordInstruction.is_speculative [bool] for btrace.
@@ -342,7 +342,8 @@ recpy_bt_func_level (PyObject *self, void *closure)
     return NULL;
 
   tinfo = ((recpy_element_object *) self)->thread;
-  return PyInt_FromLong (tinfo->btrace.level + func->level);
+  return gdb_py_object_from_longest (tinfo->btrace.level
+				     + func->level).release ();
 }
 
 /* Implementation of RecordFunctionSegment.symbol [gdb.Symbol] for btrace.
@@ -566,7 +567,8 @@ btpy_list_count (PyObject *self, PyObject *value)
 {
   /* We know that if an element is in the list, it is so exactly one time,
      enabling us to reuse the "is element of" check.  */
-  return PyInt_FromLong (btpy_list_contains (self, value));
+  return gdb_py_object_from_longest (btpy_list_contains (self,
+							 value)).release ();
 }
 
 /* Python rich compare function to allow for equality and inequality checks
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index a6b08dcdd83..38634081224 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -464,7 +464,7 @@ recpy_gap_reason_code (PyObject *self, void *closure)
 {
   const recpy_gap_object * const obj = (const recpy_gap_object *) self;
 
-  return PyInt_FromLong (obj->reason_code);
+  return gdb_py_object_from_longest (obj->reason_code).release ();
 }
 
 /* Implementation of RecordGap.error_string [str].  */
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index d683505c8e5..0adcb0bc9ad 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -131,7 +131,7 @@ sympy_get_addr_class (PyObject *self, void *closure)
 
   SYMPY_REQUIRE_VALID (self, symbol);
 
-  return PyInt_FromLong (SYMBOL_CLASS (symbol));
+  return gdb_py_object_from_longest (SYMBOL_CLASS (symbol)).release ();
 }
 
 static PyObject *
@@ -221,7 +221,7 @@ sympy_line (PyObject *self, void *closure)
 
   SYMPY_REQUIRE_VALID (self, symbol);
 
-  return PyInt_FromLong (SYMBOL_LINE (symbol));
+  return gdb_py_object_from_longest (SYMBOL_LINE (symbol)).release ();
 }
 
 /* Implementation of gdb.Symbol.is_valid (self) -> Boolean.
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index b0e7618af7e..579662f92d2 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -290,7 +290,7 @@ salpy_get_line (PyObject *self, void *closure)
 
   SALPY_REQUIRE_VALID (self, sal);
 
-  return PyInt_FromLong (sal->line);
+  return gdb_py_object_from_longest (sal->line).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8825b0749ce..5f09795fd52 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -157,7 +157,7 @@ typy_get_code (PyObject *self, void *closure)
 {
   struct type *type = ((type_object *) self)->type;
 
-  return PyInt_FromLong (type->code ());
+  return gdb_py_object_from_longest (type->code ()).release ();
 }
 
 /* Helper function for typy_fields which converts a single field to a
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index a884d6b70b2..c8e51c14b41 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1860,7 +1860,7 @@ convert_value_from_python (PyObject *obj)
 	      if (PyErr_ExceptionMatches (PyExc_OverflowError))
 		{
 		  gdbpy_err_fetch fetched_error;
-		  gdbpy_ref<> zero (PyInt_FromLong (0));
+		  gdbpy_ref<> zero = gdb_py_object_from_longest (0);
 
 		  /* Check whether obj is positive.  */
 		  if (PyObject_RichCompareBool (obj, zero.get (), Py_GT) > 0)
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 2c4195f3d03..36c1ab281f8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -96,7 +96,6 @@
 #define Py_TPFLAGS_CHECKTYPES 0
 
 #define PyInt_Check PyLong_Check
-#define PyInt_FromLong PyLong_FromLong
 #define PyInt_AsLong PyLong_AsLong
 #define PyInt_AsSsize_t PyLong_AsSsize_t
 
-- 
2.26.2

Re: [PATCH 0/7] Standarize int-wrapping approach in Python code

[Joel Brobecker]

Hi Tom,

On Fri, Sep 11, 2020 at 08:33:02AM -0600, Tom Tromey wrote:
> A while back I fixed a bug where a CORE_ADDR was wrapped for Python
> using PyLong_FromLong, rather than using an unsigned conversion.
> 
> After doing this, I thought it would be good to audit the code for
> other oddities.  Mostly what I found were possible overflows from code
> that converted specifically to "int" (an issue only for Python 2 I
> suppose).
> 
> This series changes the Python code in gdb to prefer two of its own
> functions -- one signed and one unsigned -- which should avoid all
> issues both now and in the future.  The idea here is ban PyInt_From*
> and PyLong_From*, and simply always use our own wrappers.
> 
> Let me know what you think.

Makes sense to me. I scanned the patch series, and it looked good.

-- 
Joel

Re: [PATCH 0/7] Standarize int-wrapping approach in Python code

[Tom Tromey]

>> This series changes the Python code in gdb to prefer two of its own
>> functions -- one signed and one unsigned -- which should avoid all
>> issues both now and in the future.  The idea here is ban PyInt_From*
>> and PyLong_From*, and simply always use our own wrappers.
>> 
>> Let me know what you think.

Joel> Makes sense to me. I scanned the patch series, and it looked good.

Thanks.  I'm going to check this in.

Tom