public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* FYI: fix int/long/longest confusion in python
@ 2011-01-26 20:46 Tom Tromey
  0 siblings, 0 replies; only message in thread
From: Tom Tromey @ 2011-01-26 20:46 UTC (permalink / raw)
  To: gdb-patches

I am checking this in.

The python layer has some confusion about Python int and long objects.
It also freely uses Python's "long long", which isn't always available.
Finally, there were places where conversion from Python objects to C
values was not error-checked.

This patch fixes most of these problems.  I looked at all the uses of
the various int/long conversion functions and decided what to do in each
case.  I added wrapper macros in python-internal.h to try to help with
the "long long" problem -- but I did not solve it completely; there is
one remaining instance, and as all my platforms have long long support,
it was not pressing for me.

This fixes a pretty-printing bug I ran into with libstdc++ as a side
effect.

I think there is still a bit of signed/unsigned confusion around
addresses.  There is a PR for this.  I didn't try to normalize this.

Built and regtested on x86-64 (compile farm).

Tom

2011-01-24  Tom Tromey  <tromey@redhat.com>

	* python/python.c (gdbpy_solib_name): Use gdb_py_longest and
	GDB_PY_LL_ARG.
	* python/python-internal.h (GDB_PY_LL_ARG, GDB_PY_LLU_ARG): New
	macros.
	(gdb_py_longest, gdb_py_ulongest): New typedefs.
	(gdb_py_long_from_longest, gdb_py_long_from_ulongest)
	(gdb_py_long_as_ulongest): New defines.
	(gdb_py_object_from_longest, gdb_py_object_from_ulongest)
	(gdb_py_int_as_long): Declare.
	* python/py-value.c (valpy_lazy_string): Use gdb_py_longest,
	GDB_PY_LL_ARG, gdb_py_object_from_longest.
	(valpy_long): Add comment.
	* python/py-utils.c (get_addr_from_python): Use
	gdb_py_long_as_ulongest.  Handle overflow properly.
	(gdb_py_object_from_longest): New function.
	(gdb_py_object_from_ulongest): Likewise.
	(gdb_py_int_as_long): Likewise.
	* python/py-type.c (typy_array): Use gdb_py_int_as_long.
	* python/py-symtab.c (salpy_get_pc): Use
	gdb_py_long_from_ulongest.
	(salpy_get_line): Use PyInt_FromLong.
	* python/py-param.c (set_parameter_value): Use
	gdb_py_int_as_long.
	* python/py-lazy-string.c (stpy_get_address): Use
	gdb_py_long_from_ulongest.
	* python/py-frame.c (frapy_pc): Use gdb_py_long_from_ulongest.
	* python/py-cmd.c (cmdpy_completer): Use gdb_py_int_as_long.
	* python/py-breakpoint.c (bppy_set_thread): Use
	gdb_py_int_as_long.
	(bppy_set_task): Likewise.
	(bppy_set_ignore_count): Likewise.
	(bppy_set_hit_count): Likewise.
	* python/py-block.c (blpy_get_start): Use
	gdb_py_object_from_ulongest.
	(blpy_get_end): Likewise.
	(gdbpy_block_for_pc): Use gdb_py_ulongest and GDB_PY_LLU_ARG.

diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index b6b9904..faa4c46 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -109,7 +109,7 @@ blpy_get_start (PyObject *self, void *closure)
 
   BLPY_REQUIRE_VALID (self, block);
 
-  return PyLong_FromUnsignedLongLong (BLOCK_START (block));
+  return gdb_py_object_from_ulongest (BLOCK_START (block));
 }
 
 static PyObject *
@@ -119,7 +119,7 @@ blpy_get_end (PyObject *self, void *closure)
 
   BLPY_REQUIRE_VALID (self, block);
 
-  return PyLong_FromUnsignedLongLong (BLOCK_END (block));
+  return gdb_py_object_from_ulongest (BLOCK_END (block));
 }
 
 static PyObject *
@@ -268,12 +268,12 @@ blpy_block_syms_dealloc (PyObject *obj)
 PyObject *
 gdbpy_block_for_pc (PyObject *self, PyObject *args)
 {
-  unsigned PY_LONG_LONG pc;
+  gdb_py_ulongest pc;
   struct block *block;
   struct obj_section *section;
   struct symtab *symtab;
 
-  if (!PyArg_ParseTuple (args, "K", &pc))
+  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
     return NULL;
 
   section = find_pc_mapped_section (pc);
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index b6d0088..52ea2c4 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -211,7 +211,7 @@ static int
 bppy_set_thread (PyObject *self, PyObject *newvalue, void *closure)
 {
   breakpoint_object *self_bp = (breakpoint_object *) self;
-  int id;
+  long id;
 
   BPPY_SET_REQUIRE_VALID (self_bp);
 
@@ -223,7 +223,9 @@ bppy_set_thread (PyObject *self, PyObject *newvalue, void *closure)
     }
   else if (PyInt_Check (newvalue))
     {
-      id = (int) PyInt_AsLong (newvalue);
+      if (! gdb_py_int_as_long (newvalue, &id))
+	return -1;
+
       if (! valid_thread_id (id))
 	{
 	  PyErr_SetString (PyExc_RuntimeError, 
@@ -250,7 +252,7 @@ static int
 bppy_set_task (PyObject *self, PyObject *newvalue, void *closure)
 {
   breakpoint_object *self_bp = (breakpoint_object *) self;
-  int id;
+  long id;
 
   BPPY_SET_REQUIRE_VALID (self_bp);
 
@@ -262,7 +264,9 @@ bppy_set_task (PyObject *self, PyObject *newvalue, void *closure)
     }
   else if (PyInt_Check (newvalue))
     {
-      id = (int) PyInt_AsLong (newvalue);
+      if (! gdb_py_int_as_long (newvalue, &id))
+	return -1;
+
       if (! valid_task_id (id))
 	{
 	  PyErr_SetString (PyExc_RuntimeError, 
@@ -324,7 +328,9 @@ bppy_set_ignore_count (PyObject *self, PyObject *newvalue, void *closure)
       return -1;
     }
 
-  value = PyInt_AsLong (newvalue);
+  if (! gdb_py_int_as_long (newvalue, &value))
+    return -1;
+
   if (value < 0)
     value = 0;
   set_ignore_count (self_bp->number, (int) value, 0);
@@ -346,11 +352,19 @@ bppy_set_hit_count (PyObject *self, PyObject *newvalue, void *closure)
 		       _("Cannot delete `hit_count' attribute."));
       return -1;
     }
-  else if (! PyInt_Check (newvalue) || PyInt_AsLong (newvalue) != 0)
+  else
     {
-      PyErr_SetString (PyExc_AttributeError,
-		       _("The value of `hit_count' must be zero."));
-      return -1;
+      long value;
+
+      if (! gdb_py_int_as_long (newvalue, &value))
+	return -1;
+
+      if (value != 0)
+	{
+	  PyErr_SetString (PyExc_AttributeError,
+			   _("The value of `hit_count' must be zero."));
+	  return -1;
+	}
     }
 
   self_bp->bp->hit_count = 0;
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 3de3af2..245d66a 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -279,9 +279,14 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
     {
       /* User code may also return one of the completion constants,
 	 thus requesting that sort of completion.  */
-      long value = PyInt_AsLong (resultobj);
+      long value;
 
-      if (value >= 0 && value < (long) N_COMPLETERS)
+      if (! gdb_py_int_as_long (resultobj, &value))
+	{
+	  /* Ignore.  */
+	  PyErr_Clear ();
+	}
+      else if (value >= 0 && value < (long) N_COMPLETERS)
 	result = completers[value].completer (command, text, word);
     }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index a52c8c7..a4ff66f 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -201,7 +201,7 @@ frapy_pc (PyObject *self, PyObject *args)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-  return PyLong_FromUnsignedLongLong (pc);
+  return gdb_py_long_from_ulongest (pc);
 }
 
 /* Implementation of gdb.Frame.block (self) -> gdb.Block.
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index 3863bb1..940ce88 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -54,7 +54,7 @@ stpy_get_address (PyObject *self, void *closure)
 {
   lazy_string_object *self_string = (lazy_string_object *) self;
 
-  return PyLong_FromUnsignedLongLong (self_string->address);
+  return gdb_py_long_from_ulongest (self_string->address);
 }
 
 static PyObject *
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index efef0d0..22aa91d 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -232,7 +232,9 @@ set_parameter_value (parmpy_object *self, PyObject *value)
 	    return -1;
 	  }
 
-	l = PyInt_AsLong (value);
+	if (! gdb_py_int_as_long (value, &l))
+	  return -1;
+
 	if (self->type == var_uinteger)
 	  {
 	    ok = (l >= 0 && l <= UINT_MAX);
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 85a30a6..95dda5d 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -186,7 +186,7 @@ salpy_get_pc (PyObject *self, void *closure)
 
   SALPY_REQUIRE_VALID (self, sal);
 
-  return PyLong_FromUnsignedLongLong (sal->pc);
+  return gdb_py_long_from_ulongest (sal->pc);
 }
 
 static PyObject *
@@ -196,7 +196,7 @@ salpy_get_line (PyObject *self, void *closure)
 
   SALPY_REQUIRE_VALID (self, sal);
 
-  return PyLong_FromUnsignedLongLong (sal->line);
+  return PyInt_FromLong (sal->line);
 }
 
 static PyObject *
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 9a6b077..c010420 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -276,13 +276,13 @@ typy_strip_typedefs (PyObject *self, PyObject *args)
 static PyObject *
 typy_array (PyObject *self, PyObject *args)
 {
-  int n1, n2;
+  long n1, n2;
   PyObject *n2_obj = NULL;
   struct type *array = NULL;
   struct type *type = ((type_object *) self)->type;
   volatile struct gdb_exception except;
 
-  if (! PyArg_ParseTuple (args, "i|O", &n1, &n2_obj))
+  if (! PyArg_ParseTuple (args, "l|O", &n1, &n2_obj))
     return NULL;
 
   if (n2_obj)
@@ -293,8 +293,8 @@ typy_array (PyObject *self, PyObject *args)
 			   _("Array bound must be an integer"));
 	  return NULL;
 	}
-      n2 = (int) PyInt_AsLong (n2_obj);
-      if (PyErr_Occurred ())
+
+      if (! gdb_py_int_as_long (n2_obj, &n2))
 	return NULL;
     }
   else
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 8a9176ac..79e17c4 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -304,39 +304,73 @@ get_addr_from_python (PyObject *obj, CORE_ADDR *addr)
 {
   if (gdbpy_is_value_object (obj))
     *addr = value_as_address (value_object_to_value (obj));
-  else if (PyLong_Check (obj))
+  else
     {
-      /* Assume CORE_ADDR corresponds to unsigned long.  */
-      *addr = PyLong_AsUnsignedLong (obj);
-      if (PyErr_Occurred () != NULL)
+      PyObject *num = PyNumber_Long (obj);
+      gdb_py_ulongest val;
+
+      if (num == NULL)
 	return 0;
-    }
-  else if (PyInt_Check (obj))
-    {
-      long val;
 
-      /* Assume CORE_ADDR corresponds to unsigned long.  */
-      val = PyInt_AsLong (obj);
+      val = gdb_py_long_as_ulongest (num);
+      Py_XDECREF (num);
+      if (PyErr_Occurred ())
+	return 0;
 
-      if (val >= 0)
-	*addr = val;
-      else
-      {
-	/* If no error ocurred, VAL is indeed negative.  */
-	if (PyErr_Occurred () != NULL)
+      if (sizeof (val) > sizeof (CORE_ADDR) && ((CORE_ADDR) val) != val)
+	{
+	  PyErr_SetString (PyExc_ValueError,
+			   _("Overflow converting to address."));
 	  return 0;
+	}
 
-	PyErr_SetString (PyExc_ValueError,
-			 _("Supplied address is negative."));
-	return 0;
-      }
-    }
-  else
-    {
-      PyErr_SetString (PyExc_TypeError,
-		       _("Invalid type for address."));
-      return 0;
+      *addr = val;
     }
 
   return 1;
 }
+
+/* Convert a LONGEST to the appropriate Python object -- either an
+   integer object or a long object, depending on its value.  */
+
+PyObject *
+gdb_py_object_from_longest (LONGEST l)
+{
+#ifdef HAVE_LONG_LONG		/* Defined by Python.  */
+  /* If we have 'long long', and the value overflows a 'long', use a
+     Python Long; otherwise use a Python Int.  */
+  if (sizeof (l) > sizeof (long)
+      && (l > PyInt_GetMax () || l < (- (LONGEST) PyInt_GetMax ()) - 1))
+    return PyLong_FromLongLong (l);
+#endif
+  return PyInt_FromLong (l);
+}
+
+/* Convert a ULONGEST to the appropriate Python object -- either an
+   integer object or a long object, depending on its value.  */
+
+PyObject *
+gdb_py_object_from_ulongest (ULONGEST l)
+{
+#ifdef HAVE_LONG_LONG		/* Defined by Python.  */
+  /* If we have 'long long', and the value overflows a 'long', use a
+     Python Long; otherwise use a Python Int.  */
+  if (sizeof (l) > sizeof (unsigned long) && l > PyInt_GetMax ())
+    return PyLong_FromUnsignedLongLong (l);
+#endif
+
+  if (l > PyInt_GetMax ())
+    return PyLong_FromUnsignedLong (l);
+
+  return PyInt_FromLong (l);
+}
+
+/* Like PyInt_AsLong, but returns 0 on failure, 1 on success, and puts
+   the value into an out parameter.  */
+
+int
+gdb_py_int_as_long (PyObject *obj, long *result)
+{
+  *result = PyInt_AsLong (obj);
+  return ! (*result == -1 && PyErr_Occurred ());
+}
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 1aa9443..27bf101 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -307,13 +307,13 @@ valpy_get_dynamic_type (PyObject *self, void *closure)
 static PyObject *
 valpy_lazy_string (PyObject *self, PyObject *args, PyObject *kw)
 {
-  int length = -1;
+  gdb_py_longest length = -1;
   struct value *value = ((value_object *) self)->value;
   const char *user_encoding = NULL;
   static char *keywords[] = { "encoding", "length", NULL };
   PyObject *str_obj;
 
-  if (!PyArg_ParseTupleAndKeywords (args, kw, "|si", keywords,
+  if (!PyArg_ParseTupleAndKeywords (args, kw, "|s" GDB_PY_LL_ARG, keywords,
 				    &user_encoding, &length))
     return NULL;
 
@@ -987,14 +987,7 @@ valpy_int (PyObject *self)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-#ifdef HAVE_LONG_LONG		/* Defined by Python.  */
-  /* If we have 'long long', and the value overflows a 'long', use a
-     Python Long; otherwise use a Python Int.  */
-  if (sizeof (l) > sizeof (long) && (l > PyInt_GetMax ()
-				     || l < (- (LONGEST) PyInt_GetMax ()) - 1))
-    return PyLong_FromLongLong (l);
-#endif
-  return PyInt_FromLong (l);
+  return gdb_py_object_from_longest (l);
 }
 
 /* Implements conversion to long.  */
@@ -1019,11 +1012,7 @@ valpy_long (PyObject *self)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-#ifdef HAVE_LONG_LONG		/* Defined by Python.  */
-  return PyLong_FromLongLong (l);
-#else
-  return PyLong_FromLong (l);
-#endif
+  return gdb_py_long_from_longest (l);
 }
 
 /* Implements conversion to float.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 80d0763..9dac04d 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -73,6 +73,32 @@ typedef int Py_ssize_t;
 #define PyEval_ReleaseLock()
 #endif
 
+/* Python supplies HAVE_LONG_LONG and some `long long' support when it
+   is available.  These defines let us handle the differences more
+   cleanly.  */
+#ifdef HAVE_LONG_LONG
+
+#define GDB_PY_LL_ARG "L"
+#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
+
+#else /* HAVE_LONG_LONG */
+
+#define GDB_PY_LL_ARG "L"
+#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
+
+#endif /* HAVE_LONG_LONG */
+
+
 /* In order to be able to parse symtab_and_line_to_sal_object function 
    a real symtab_and_line structure is needed.  */
 #include "symtab.h"
@@ -243,4 +269,8 @@ extern PyObject *gdbpy_convert_exception (struct gdb_exception);
 
 int get_addr_from_python (PyObject *obj, CORE_ADDR *addr);
 
+PyObject *gdb_py_object_from_longest (LONGEST l);
+PyObject *gdb_py_object_from_ulongest (ULONGEST l);
+int gdb_py_int_as_long (PyObject *, long *);
+
 #endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 375042c..134e730 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -407,16 +407,9 @@ gdbpy_solib_name (PyObject *self, PyObject *args)
 {
   char *soname;
   PyObject *str_obj;
-#ifdef PY_LONG_LONG
-  unsigned PY_LONG_LONG pc;
-  /* To be compatible with Python 2.4 the format strings are not const.  */
-  char *format = "K";
-#else
-  unsigned long pc;
-  char *format = "k";
-#endif
+  gdb_py_longest pc;
 
-  if (!PyArg_ParseTuple (args, format, &pc))
+  if (!PyArg_ParseTuple (args, GDB_PY_LL_ARG, &pc))
     return NULL;
 
   soname = solib_name_from_address (current_program_space, pc);
-- 
1.7.3.4

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2011-01-26 19:48 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-26 20:46 FYI: fix int/long/longest confusion in python Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).