public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 4/7] Report Python errors coming from gdb.post_event
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
  2018-09-15  7:25 ` [PATCH 2/7] Preserve sign when converting gdb.Value to Python int Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-15  7:25 ` [PATCH 3/7] Allow conversion of pointers to Python int Tom Tromey
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR python/14062 points out that errors coming from the gdb.post_event
callback are not reported.  This can make it hard to understand why
your Python code in gdb isn't working.

Because users have control over whether exceptions are printed at all,
it seems good to simply have post_event report errors in the usual
way.

gdb/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/14062:
	* python/python.c (gdbpy_run_events): Do not ignore exceptions.

gdb/testsuite/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/14062:
	* gdb.python/python.exp: Add test for post_event error.
---
 gdb/ChangeLog                       |  5 +++++
 gdb/python/python.c                 |  3 +--
 gdb/testsuite/ChangeLog             |  5 +++++
 gdb/testsuite/gdb.python/python.exp | 13 ++++++++++++-
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/gdb/python/python.c b/gdb/python/python.c
index e89c90f8d9f..66a43123a0a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1043,10 +1043,9 @@ gdbpy_run_events (int error, gdb_client_data client_data)
       if (gdbpy_event_list == NULL)
 	gdbpy_event_list_end = &gdbpy_event_list;
 
-      /* Ignore errors.  */
       gdbpy_ref<> call_result (PyObject_CallObject (item->event, NULL));
       if (call_result == NULL)
-	PyErr_Clear ();
+	gdbpy_print_stack ();
 
       Py_DECREF (item->event);
       xfree (item);
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 1d7ea5b6b4b..7c753215c7f 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -134,7 +134,18 @@ gdb_py_test_multiple "post event insertion" \
   "end" ""
 
 gdb_test "python print (someVal)" "1" "test post event execution"
-gdb_test "python gdb.post_event(str(1))" "RuntimeError: Posted event is not callable.*" "test non callable class"
+gdb_test "python gdb.post_event(str(1))" "RuntimeError: Posted event is not callable.*" \
+    "test non callable class"
+
+send_gdb "python gdb.post_event(lambda: invalid())\n"
+gdb_expect {
+    -re "name 'invalid' is not defined" {
+	pass "test post_event error on receipt"
+    }
+    default {
+	fail "test post_event error on receipt"
+    }
+}
 
 # Test (no) pagination of the executed command.
 gdb_test "show height" {Number of lines gdb thinks are in a page is unlimited\.}
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/7] Preserve sign when converting gdb.Value to Python int
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-15  7:25 ` [PATCH 4/7] Report Python errors coming from gdb.post_event Tom Tromey
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR python/20126 points out that sometimes the conversion of a
gdb.Value can result in a negative Python integer.  This happens
because valpy_int does not examine the signedness of the value's type.

gdb/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/20126:
	* python/py-value.c (valpy_int): Respect type sign.

gdb/testsuite/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/20126:
	* gdb.python/py-value.exp (test_value_numeric_ops): Add
	signed-ness conversion tests.
---
 gdb/ChangeLog                         | 5 +++++
 gdb/python/py-value.c                 | 5 ++++-
 gdb/testsuite/ChangeLog               | 6 ++++++
 gdb/testsuite/gdb.python/py-value.exp | 5 +++++
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 9abcf9212e6..5c6792f85fc 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1514,7 +1514,10 @@ valpy_int (PyObject *self)
     }
   END_CATCH
 
-  return gdb_py_object_from_longest (l);
+  if (TYPE_UNSIGNED (type))
+    return gdb_py_object_from_ulongest (l);
+  else
+    return gdb_py_object_from_longest (l);
 }
 #endif
 
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 9e8fa15c28e..aed50d1c8cf 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -140,6 +140,11 @@ proc test_value_numeric_ops {} {
   gdb_test "python print ('result = ' + str(\[1,2,3\]\[gdb.Value(0)\]))" \
     "result = 1" "use value as array index"
 
+  gdb_test "python print('%x' % int(gdb.parse_and_eval('-1ull')))" \
+      "f+" "int conversion respect type sign"
+  gdb_test "python print('%x' % long(gdb.parse_and_eval('-1ull')))" \
+      "f+" "long conversion respect type sign"
+
   # Test some invalid operations.
 
   gdb_test_multiple "python print ('result = ' + str(i+'foo'))" "catch error in python type conversion" {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 5/7] Check for negative argument in Type.template_argument
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
                   ` (5 preceding siblings ...)
  2018-09-15  7:25 ` [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  6 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

typy_template_argument did not check if the template argument was
non-negative.  A negative value could cause a gdb crash.

gdb/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/17284:
	* python/py-type.c (typy_template_argument): Check for negative
	argument number.

gdb/testsuite/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/17284:
	* gdb.python/py-template.exp (test_template_arg): Add test for
	negative template argument number.
---
 gdb/ChangeLog                            | 6 ++++++
 gdb/python/py-type.c                     | 7 +++++++
 gdb/testsuite/ChangeLog                  | 6 ++++++
 gdb/testsuite/gdb.python/py-template.exp | 4 ++++
 4 files changed, 23 insertions(+)

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index c7cad2e6628..897ad9374af 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -930,6 +930,13 @@ typy_template_argument (PyObject *self, PyObject *args)
   if (! PyArg_ParseTuple (args, "i|O", &argno, &block_obj))
     return NULL;
 
+  if (argno < 0)
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Template argument number must be non-negative"));
+      return NULL;
+    }
+
   if (block_obj)
     {
       block = block_object_to_block (block_obj);
diff --git a/gdb/testsuite/gdb.python/py-template.exp b/gdb/testsuite/gdb.python/py-template.exp
index 793e68d7fe4..96383a73c51 100644
--- a/gdb/testsuite/gdb.python/py-template.exp
+++ b/gdb/testsuite/gdb.python/py-template.exp
@@ -54,6 +54,10 @@ proc test_template_arg {exefile type} {
     # Replace '*' with '\*' in regex.
     regsub -all {\*} $type {\*} t
     gdb_test "python print (foo.type.template_argument(0))" $t $type
+
+    gdb_test "python print(foo.type.template_argument(-1))" \
+	"Template argument number must be non-negative\r\nError while executing Python code." \
+	"negative template argument number"
 }
 
 test_template_arg "${binfile}-ci" "const int"
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] Allow more Python scalar conversions
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
                   ` (2 preceding siblings ...)
  2018-09-15  7:25 ` [PATCH 3/7] Allow conversion of pointers to Python int Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-24  2:06   ` Simon Marchi
  2018-09-15  7:25 ` [PATCH 6/7] Consolidate gdb.GdbError handling Tom Tromey
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR python/18352 points out that the gdb Python code can't convert an
integer-valued gdb.Value to a Python float.  While writing the test I
noticed that, similarly, converting integer gdb.Values to float does
not work.  However, all of these cases seem reasonable.

gdb/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/18352;
	* python/py-value.c (valpy_float): Allow conversions from int or
	char.
	(valpy_int, valpy_long): Allow conversions from float.

gdb/testsuite/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/18352;
	* gdb.python/py-value.exp (test_float_conversion): New proc.
	Use it.
---
 gdb/ChangeLog                         |  7 +++++++
 gdb/python/py-value.c                 | 25 ++++++++++++++++++++++---
 gdb/testsuite/ChangeLog               |  6 ++++++
 gdb/testsuite/gdb.python/py-value.exp | 10 ++++++++++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 1b880fa1917..9abcf9212e6 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1497,6 +1497,12 @@ valpy_int (PyObject *self)
 
   TRY
     {
+      if (is_floating_value (value))
+	{
+	  type = builtin_type_pylong;
+	  value = value_cast (type, value);
+	}
+
       if (!is_integral_type (type))
 	error (_("Cannot convert value to int."));
 
@@ -1522,6 +1528,12 @@ valpy_long (PyObject *self)
 
   TRY
     {
+      if (is_floating_value (value))
+	{
+	  type = builtin_type_pylong;
+	  value = value_cast (type, value);
+	}
+
       type = check_typedef (type);
 
       if (!is_integral_type (type)
@@ -1554,10 +1566,17 @@ valpy_float (PyObject *self)
     {
       type = check_typedef (type);
 
-      if (TYPE_CODE (type) != TYPE_CODE_FLT || !is_floating_value (value))
+      if (TYPE_CODE (type) == TYPE_CODE_FLT && is_floating_value (value))
+	d = target_float_to_host_double (value_contents (value), type);
+      else if (TYPE_CODE (type) == TYPE_CODE_INT)
+	{
+	  /* Note that valpy_long accepts TYPE_CODE_PTR and some
+	     others here here -- but casting a pointer or bool to a
+	     float seems wrong.  */
+	  d = value_as_long (value);
+	}
+      else
 	error (_("Cannot convert value to float."));
-
-      d = target_float_to_host_double (value_contents (value), type);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index eb82a7776fa..9e8fa15c28e 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -483,6 +483,15 @@ proc test_value_hash {} {
     gdb_test "python print (one.__hash__() == hash(one))" "True" "test inbuilt hash"
 }
 
+proc test_float_conversion {} {
+    gdb_test "python print int(gdb.Value(0))" "0"
+    gdb_test "python print int(gdb.Value(0.0))" "0"
+    gdb_test "python print long(gdb.Value(0))" "0"
+    gdb_test "python print long(gdb.Value(0.0))" "0"
+    gdb_test "python print float(gdb.Value(0.0))" "0\\.0"
+    gdb_test "python print float(gdb.Value(0))" "0\\.0"
+}
+
 # Build C version of executable.  C++ is built later.
 if { [build_inferior "${binfile}" "c"] < 0 } {
     return -1
@@ -501,6 +510,7 @@ test_value_compare
 test_objfiles
 test_parse_and_eval
 test_value_hash
+test_float_conversion
 
 # The following tests require execution.
 
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 6/7] Consolidate gdb.GdbError handling
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
                   ` (3 preceding siblings ...)
  2018-09-15  7:25 ` [PATCH 1/7] Allow more Python scalar conversions Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-15  7:25 ` [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError Tom Tromey
  2018-09-15  7:25 ` [PATCH 5/7] Check for negative argument in Type.template_argument Tom Tromey
  6 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed two nearly identical copies of the same code for handling
gdb.GdbError.  The only differences were in some error messages.
These differences didn't seem very important, so this patch pulls the
code out into a new function.

gdb/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	* python/py-function.c (fnpy_call): Use gdbpy_handle_exception.
	* python/py-cmd.c (cmdpy_function): Use gdbpy_handle_exception.
	* python/python-internal.h (gdbpy_handle_exception): Declare.
	* python/py-utils.c (gdbpy_handle_exception): New function.
---
 gdb/ChangeLog                |  7 +++++
 gdb/python/py-cmd.c          | 48 +------------------------------
 gdb/python/py-function.c     | 51 +-------------------------------
 gdb/python/py-utils.c        | 56 ++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |  1 +
 5 files changed, 66 insertions(+), 97 deletions(-)

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 27c4689413a..e7eb66f515c 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -145,53 +145,7 @@ cmdpy_function (struct cmd_list_element *command,
 						  NULL));
 
   if (result == NULL)
-    {
-      PyObject *ptype, *pvalue, *ptraceback;
-
-      PyErr_Fetch (&ptype, &pvalue, &ptraceback);
-
-      /* Try to fetch an error message contained within ptype, pvalue.
-	 When fetching the error message we need to make our own copy,
-	 we no longer own ptype, pvalue after the call to PyErr_Restore.  */
-
-      gdb::unique_xmalloc_ptr<char>
-	msg (gdbpy_exception_to_string (ptype, pvalue));
-
-      if (msg == NULL)
-	{
-	  /* An error occurred computing the string representation of the
-	     error message.  This is rare, but we should inform the user.  */
-	  printf_filtered (_("An error occurred in a Python command\n"
-			     "and then another occurred computing the "
-			     "error message.\n"));
-	  gdbpy_print_stack ();
-	}
-
-      /* Don't print the stack for gdb.GdbError exceptions.
-	 It is generally used to flag user errors.
-
-	 We also don't want to print "Error occurred in Python command"
-	 for user errors.  However, a missing message for gdb.GdbError
-	 exceptions is arguably a bug, so we flag it as such.  */
-
-      if (! PyErr_GivenExceptionMatches (ptype, gdbpy_gdberror_exc)
-	  || msg == NULL || *msg == '\0')
-	{
-	  PyErr_Restore (ptype, pvalue, ptraceback);
-	  gdbpy_print_stack ();
-	  if (msg != NULL && *msg != '\0')
-	    error (_("Error occurred in Python command: %s"), msg.get ());
-	  else
-	    error (_("Error occurred in Python command."));
-	}
-      else
-	{
-	  Py_XDECREF (ptype);
-	  Py_XDECREF (pvalue);
-	  Py_XDECREF (ptraceback);
-	  error ("%s", msg.get ());
-	}
-    }
+    gdbpy_handle_exception ();
 }
 
 /* Helper function for the Python command completers (both "pure"
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index c4612316df6..1900f0ff0c0 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -83,56 +83,7 @@ fnpy_call (struct gdbarch *gdbarch, const struct language_defn *language,
     }
 
   if (result == NULL)
-    {
-      PyObject *ptype, *pvalue, *ptraceback;
-
-      PyErr_Fetch (&ptype, &pvalue, &ptraceback);
-
-      /* Try to fetch an error message contained within ptype, pvalue.
-	 When fetching the error message we need to make our own copy,
-	 we no longer own ptype, pvalue after the call to PyErr_Restore.  */
-
-      gdb::unique_xmalloc_ptr<char>
-	msg (gdbpy_exception_to_string (ptype, pvalue));
-
-      if (msg == NULL)
-	{
-	  /* An error occurred computing the string representation of the
-	     error message.  This is rare, but we should inform the user.  */
-
-	  printf_filtered (_("An error occurred in a Python "
-			     "convenience function\n"
-			     "and then another occurred computing the "
-			     "error message.\n"));
-	  gdbpy_print_stack ();
-	}
-
-      /* Don't print the stack for gdb.GdbError exceptions.
-	 It is generally used to flag user errors.
-
-	 We also don't want to print "Error occurred in Python command"
-	 for user errors.  However, a missing message for gdb.GdbError
-	 exceptions is arguably a bug, so we flag it as such.  */
-
-      if (!PyErr_GivenExceptionMatches (ptype, gdbpy_gdberror_exc)
-	  || msg == NULL || *msg == '\0')
-	{
-	  PyErr_Restore (ptype, pvalue, ptraceback);
-	  gdbpy_print_stack ();
-	  if (msg != NULL && *msg != '\0')
-	    error (_("Error occurred in Python convenience function: %s"),
-		   msg.get ());
-	  else
-	    error (_("Error occurred in Python convenience function."));
-	}
-      else
-	{
-	  Py_XDECREF (ptype);
-	  Py_XDECREF (pvalue);
-	  Py_XDECREF (ptraceback);
-	  error ("%s", msg.get ());
-	}
-    }
+    gdbpy_handle_exception ();
 
   value = convert_value_from_python (result.get ());
   if (value == NULL)
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 01fd6ade8d5..6ef0d7efd39 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -384,3 +384,59 @@ gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object)
     Py_DECREF (object);
   return result;
 }
+
+/* Handle a Python exception when the special gdb.GdbError treatment
+   is desired.  This should only be called when an exception is set.
+   If the exception is a gdb.GdbError, throw a gdb exception with the
+   exception text.  For other exceptions, print the Python stack and
+   then throw a gdb exception.  */
+
+void
+gdbpy_handle_exception ()
+{
+  PyObject *ptype, *pvalue, *ptraceback;
+
+  PyErr_Fetch (&ptype, &pvalue, &ptraceback);
+
+  /* Try to fetch an error message contained within ptype, pvalue.
+     When fetching the error message we need to make our own copy,
+     we no longer own ptype, pvalue after the call to PyErr_Restore.  */
+
+  gdb::unique_xmalloc_ptr<char>
+    msg (gdbpy_exception_to_string (ptype, pvalue));
+
+  if (msg == NULL)
+    {
+      /* An error occurred computing the string representation of the
+	 error message.  This is rare, but we should inform the user.  */
+      printf_filtered (_("An error occurred in Python "
+			 "and then another occurred computing the "
+			 "error message.\n"));
+      gdbpy_print_stack ();
+    }
+
+  /* Don't print the stack for gdb.GdbError exceptions.
+     It is generally used to flag user errors.
+
+     We also don't want to print "Error occurred in Python command"
+     for user errors.  However, a missing message for gdb.GdbError
+     exceptions is arguably a bug, so we flag it as such.  */
+
+  if (! PyErr_GivenExceptionMatches (ptype, gdbpy_gdberror_exc)
+      || msg == NULL || *msg == '\0')
+    {
+      PyErr_Restore (ptype, pvalue, ptraceback);
+      gdbpy_print_stack ();
+      if (msg != NULL && *msg != '\0')
+	error (_("Error occurred in Python: %s"), msg.get ());
+      else
+	error (_("Error occurred in Python."));
+    }
+  else
+    {
+      Py_XDECREF (ptype);
+      Py_XDECREF (pvalue);
+      Py_XDECREF (ptraceback);
+      error ("%s", msg.get ());
+    }
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 785ad171511..d395adc26a3 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -674,6 +674,7 @@ extern const struct language_defn *python_language;
 
 int gdbpy_print_python_errors_p (void);
 void gdbpy_print_stack (void);
+void gdbpy_handle_exception () ATTRIBUTE_NORETURN;
 
 PyObject *python_string_to_unicode (PyObject *obj);
 gdb::unique_xmalloc_ptr<char> unicode_to_target_string (PyObject *unicode_str);
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 0/7] some small python fixes (one including a doc patch)
@ 2018-09-15  7:25 Tom Tromey
  2018-09-15  7:25 ` [PATCH 2/7] Preserve sign when converting gdb.Value to Python int Tom Tromey
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches

This series fixes a few Python paper-cuts.

Tested on x86-64 Fedora 28.

Tom


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/7] Allow conversion of pointers to Python int
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
  2018-09-15  7:25 ` [PATCH 2/7] Preserve sign when converting gdb.Value to Python int Tom Tromey
  2018-09-15  7:25 ` [PATCH 4/7] Report Python errors coming from gdb.post_event Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-15  7:25 ` [PATCH 1/7] Allow more Python scalar conversions Tom Tromey
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR python/18170 questions why it's not possible to convert a pointer
value to a Python int.

Digging a bit shows that the Python 2.7 int() constructor will happily
return a long in some cases.  And, it seems gdb already understands
this in other places -- this is why gdb_py_object_from_longest
handles.

So, this patch simply extends valpy_int to allow pointer conversions,
as valpy_long does.

gdb/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/18170:
	* python/py-value.c (valpy_int): Allow conversion from pointer
	type.

gdb/testsuite/ChangeLog
2018-09-14  Tom Tromey  <tom@tromey.com>

	PR python/18170:
	* gdb.python/py-value.exp (test_value_numeric_ops): Add tests to
	convert pointers to int and long.
---
 gdb/ChangeLog                         | 6 ++++++
 gdb/python/py-value.c                 | 3 ++-
 gdb/testsuite/ChangeLog               | 6 ++++++
 gdb/testsuite/gdb.python/py-value.exp | 3 +++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 5c6792f85fc..26e91ff2b27 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1503,7 +1503,8 @@ valpy_int (PyObject *self)
 	  value = value_cast (type, value);
 	}
 
-      if (!is_integral_type (type))
+      if (!is_integral_type (type)
+	  && TYPE_CODE (type) != TYPE_CODE_PTR)
 	error (_("Cannot convert value to int."));
 
       l = value_as_long (value);
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index aed50d1c8cf..222cf1dea6f 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -129,6 +129,9 @@ proc test_value_numeric_ops {} {
   gdb_test "print (void *) 5" ".*" ""
   gdb_test_no_output "python b = gdb.history (0)" ""
 
+  gdb_test "python print(int(b))" "5" "convert pointer to int"
+  gdb_test "python print(long(b))" "5" "convert pointer to long"
+
   gdb_test "python print ('result = ' + str(a+5))" " = 0x7( <.*>)?" "add pointer value with python integer"
   gdb_test "python print ('result = ' + str(b-2))" " = 0x3( <.*>)?" "subtract python integer from pointer value"
   gdb_test "python print ('result = ' + str(b-a))" " = 3" "subtract two pointer values"
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
                   ` (4 preceding siblings ...)
  2018-09-15  7:25 ` [PATCH 6/7] Consolidate gdb.GdbError handling Tom Tromey
@ 2018-09-15  7:25 ` Tom Tromey
  2018-09-15  9:16   ` Eli Zaretskii
  2018-09-15  7:25 ` [PATCH 5/7] Check for negative argument in Type.template_argument Tom Tromey
  6 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2018-09-15  7:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

A convention in the Python layer is that raising a gdb.GdbError will
not print the Python stack -- instead the exception is treated as any
other gdb exception.

PR python/18852 asks that this treatment be extended the the
get_set_value method of gdb.Parameter.  This makes sense, because it
lets Python-created parameters act like gdb parameters.

gdb/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/18852:
	* python/py-param.c (get_set_value): Use gdbpy_handle_exception.

gdb/doc/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/18852:
	* python.texi (Parameters In Python): Document exception behavior
	of get_set_string.

gdb/testsuite/ChangeLog
2018-09-15  Tom Tromey  <tom@tromey.com>

	PR python/18852:
	* gdb.python/py-parameter.exp: Add test for parameter that throws
	on "set".
---
 gdb/ChangeLog                             |  5 +++++
 gdb/doc/ChangeLog                         |  6 ++++++
 gdb/doc/python.texi                       | 24 +++++++++++++++++++++++
 gdb/python/py-param.c                     |  5 +----
 gdb/testsuite/ChangeLog                   |  6 ++++++
 gdb/testsuite/gdb.python/py-parameter.exp | 15 ++++++++++++++
 6 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index aca6ec858cf..0487a6f3bf3 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3824,6 +3824,30 @@ example, @kbd{set foo off}).  The @code{value} attribute has already
 been populated with the new value and may be used in output.  This
 method must return a string.  If the returned string is not empty,
 @value{GDBN} will present it to the user.
+
+If this method raises @code{gdb.GdbError} (@pxref{Exception
+Handling}), then gdb will print the exception's string and the
+@code{set} command will fail.  Note, however, that the @code{value}
+attribute will not be reset in this case.  So, if your parameter must
+validate values, it should store the old value internally and reset
+the exposed value, like so:
+
+@smallexample
+class ExampleParam (gdb.Parameter):
+   def __init__ (self, name):
+      super (ExampleParam, self).__init__ (name,
+                   gdb.COMMAND_DATA,
+                   gdb.PARAM_BOOLEAN)
+      self.value = True
+      self.saved_value = True
+   def validate(self):
+      return False
+   def get_set_string (self):
+      if not self.validate():
+        self.value = self.saved_value
+        raise gdb.GdbError('Failed to validate')
+      self.saved_value = self.value
+@end smallexample
 @end defun
 
 @defun Parameter.get_show_string (self, svalue)
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 0f0214befe0..bff5d60fc26 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -396,10 +396,7 @@ get_set_value (const char *args, int from_tty,
     {
       set_doc_string = call_doc_function (obj, set_doc_func.get (), NULL);
       if (! set_doc_string)
-	{
-	  gdbpy_print_stack ();
-	  return;
-	}
+	gdbpy_handle_exception ();
     }
 
   const char *str = set_doc_string.get ();
diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
index 0508544f9d9..9a36e6372de 100644
--- a/gdb/testsuite/gdb.python/py-parameter.exp
+++ b/gdb/testsuite/gdb.python/py-parameter.exp
@@ -203,3 +203,18 @@ foreach kind {PARAM_ZUINTEGER PARAM_ZUINTEGER_UNLIMITED} {
 	    "check that PARAM_ZUINTEGER value is -1 after setting"
     }
 }
+
+gdb_py_test_multiple "Throwing gdb parameter" \
+    "python" "" \
+    "class TestThrowParam (gdb.Parameter):" "" \
+    "   def __init__ (self, name):" "" \
+    "      super (TestThrowParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_STRING)" "" \
+    "      self.value = True" "" \
+    "   def get_set_string (self):" "" \
+    "      raise gdb.GdbError('Ordinary gdb error')" "" \
+    "test_throw_param = TestThrowParam ('print test-throw-param')" ""\
+    "end"
+
+gdb_test "set print test-throw-param whoops" \
+    "Ordinary gdb error" \
+    "gdb.GdbError does not show Python stack"
-- 
2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-15  7:25 ` [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError Tom Tromey
@ 2018-09-15  9:16   ` Eli Zaretskii
  2018-09-24  2:39     ` Simon Marchi
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-09-15  9:16 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Sat, 15 Sep 2018 01:24:59 -0600
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index aca6ec858cf..0487a6f3bf3 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3824,6 +3824,30 @@ example, @kbd{set foo off}).  The @code{value} attribute has already
>  been populated with the new value and may be used in output.  This
>  method must return a string.  If the returned string is not empty,
>  @value{GDBN} will present it to the user.
> +
> +If this method raises @code{gdb.GdbError} (@pxref{Exception

"raises the 'gdb.GdbError' exception", I presume?

> +Handling}), then gdb will print the exception's string and the
                    ^^^
@value{GDBN}

OK for the documentation parts with these fixed.

Thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] Allow more Python scalar conversions
  2018-09-15  7:25 ` [PATCH 1/7] Allow more Python scalar conversions Tom Tromey
@ 2018-09-24  2:06   ` Simon Marchi
  2018-09-24  4:58     ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Marchi @ 2018-09-24  2:06 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 2018-09-15 3:24 a.m., Tom Tromey wrote:
> +proc test_float_conversion {} {
> +    gdb_test "python print int(gdb.Value(0))" "0"
> +    gdb_test "python print int(gdb.Value(0.0))" "0"
> +    gdb_test "python print long(gdb.Value(0))" "0"
> +    gdb_test "python print long(gdb.Value(0.0))" "0"
> +    gdb_test "python print float(gdb.Value(0.0))" "0\\.0"
> +    gdb_test "python print float(gdb.Value(0))" "0\\.0"
> +}

This LGTM, but I just wanted to mention that I would probably have tested
with another value than 0.  For example, test that the truncation (int(2.3)
which becomes 2) works correctly.  Not really a big deal.

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-15  9:16   ` Eli Zaretskii
@ 2018-09-24  2:39     ` Simon Marchi
  2018-09-24  4:58       ` Tom Tromey
  2018-09-24  7:10       ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: Simon Marchi @ 2018-09-24  2:39 UTC (permalink / raw)
  To: Eli Zaretskii, Tom Tromey; +Cc: gdb-patches

On 2018-09-15 5:15 a.m., Eli Zaretskii wrote:
>> From: Tom Tromey <tom@tromey.com>
>> Cc: Tom Tromey <tom@tromey.com>
>> Date: Sat, 15 Sep 2018 01:24:59 -0600
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index aca6ec858cf..0487a6f3bf3 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -3824,6 +3824,30 @@ example, @kbd{set foo off}).  The @code{value} attribute has already
>>  been populated with the new value and may be used in output.  This
>>  method must return a string.  If the returned string is not empty,
>>  @value{GDBN} will present it to the user.
>> +
>> +If this method raises @code{gdb.GdbError} (@pxref{Exception
> 
> "raises the 'gdb.GdbError' exception", I presume?

I think that was intentional, I often see it written this way.  Here's
an example from the Python doc:

https://docs.python.org/2/library/string.html#string.index

Everything in that series looks reasonable to me, thanks for doing this.

Have you tried to run your tests on Python 3 though?  I get some failures
because long doesn't exist there.

Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-24  2:39     ` Simon Marchi
@ 2018-09-24  4:58       ` Tom Tromey
  2018-09-24  5:14         ` Tom Tromey
  2018-09-24  7:10       ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2018-09-24  4:58 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Eli Zaretskii, Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Everything in that series looks reasonable to me, thanks for doing this.

Simon> Have you tried to run your tests on Python 3 though?  I get some failures
Simon> because long doesn't exist there.

I forgot to try.
I'm doing it now, patch-by-patch.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] Allow more Python scalar conversions
  2018-09-24  2:06   ` Simon Marchi
@ 2018-09-24  4:58     ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-24  4:58 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2018-09-15 3:24 a.m., Tom Tromey wrote:
>> +proc test_float_conversion {} {
>> +    gdb_test "python print int(gdb.Value(0))" "0"
>> +    gdb_test "python print int(gdb.Value(0.0))" "0"
>> +    gdb_test "python print long(gdb.Value(0))" "0"
>> +    gdb_test "python print long(gdb.Value(0.0))" "0"
>> +    gdb_test "python print float(gdb.Value(0.0))" "0\\.0"
>> +    gdb_test "python print float(gdb.Value(0))" "0\\.0"
>> +}

Simon> This LGTM, but I just wanted to mention that I would probably have tested
Simon> with another value than 0.  For example, test that the truncation (int(2.3)
Simon> which becomes 2) works correctly.  Not really a big deal.

I've changed the ones with floating point arguments.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-24  4:58       ` Tom Tromey
@ 2018-09-24  5:14         ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2018-09-24  5:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, Eli Zaretskii, gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Everything in that series looks reasonable to me, thanks for doing this.

Simon> Have you tried to run your tests on Python 3 though?  I get some failures
Simon> because long doesn't exist there.

Tom> I forgot to try.
Tom> I'm doing it now, patch-by-patch.

Mostly this involved avoiding "long", but in one patch I'd also
forgotten that print needs parens as well.

I'm checking this series in now.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-24  2:39     ` Simon Marchi
  2018-09-24  4:58       ` Tom Tromey
@ 2018-09-24  7:10       ` Eli Zaretskii
  2018-09-24 10:24         ` Tom Tromey
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2018-09-24  7:10 UTC (permalink / raw)
  To: Simon Marchi; +Cc: tom, gdb-patches

> Cc: gdb-patches@sourceware.org
> From: Simon Marchi <simark@simark.ca>
> Date: Sun, 23 Sep 2018 22:39:03 -0400
> 
> >> +If this method raises @code{gdb.GdbError} (@pxref{Exception
> > 
> > "raises the 'gdb.GdbError' exception", I presume?
> 
> I think that was intentional, I often see it written this way.  Here's
> an example from the Python doc:
> 
> https://docs.python.org/2/library/string.html#string.index

I understand, but this text could be confusing, especially to
non-native English speakers, and is more confusing as the length of
the exception's name becomes longer and includes stuff other than just
"error".

So I think we should not use this paradigm here, to keep the confusion
level down.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-24  7:10       ` Eli Zaretskii
@ 2018-09-24 10:24         ` Tom Tromey
  2018-09-24 12:39           ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2018-09-24 10:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Simon Marchi, tom, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> So I think we should not use this paradigm here, to keep the confusion
Eli> level down.

FWIW I made the requested change before pushing:

>> >> +If this method raises @code{gdb.GdbError} (@pxref{Exception

>> > "raises the 'gdb.GdbError' exception", I presume?

However I wonder if it should read "a" rather the "the".

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError
  2018-09-24 10:24         ` Tom Tromey
@ 2018-09-24 12:39           ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2018-09-24 12:39 UTC (permalink / raw)
  To: Tom Tromey; +Cc: simark, gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Simon Marchi <simark@simark.ca>,  tom@tromey.com,  gdb-patches@sourceware.org
> Date: Mon, 24 Sep 2018 04:24:07 -0600
> 
> FWIW I made the requested change before pushing:

Thanks.

> >> >> +If this method raises @code{gdb.GdbError} (@pxref{Exception
> 
> >> > "raises the 'gdb.GdbError' exception", I presume?
> 
> However I wonder if it should read "a" rather the "the".

My gut feeling is "the", but your English is more native than mine, so
I'll defer to your opinion.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-09-24 12:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-15  7:25 [PATCH 0/7] some small python fixes (one including a doc patch) Tom Tromey
2018-09-15  7:25 ` [PATCH 2/7] Preserve sign when converting gdb.Value to Python int Tom Tromey
2018-09-15  7:25 ` [PATCH 4/7] Report Python errors coming from gdb.post_event Tom Tromey
2018-09-15  7:25 ` [PATCH 3/7] Allow conversion of pointers to Python int Tom Tromey
2018-09-15  7:25 ` [PATCH 1/7] Allow more Python scalar conversions Tom Tromey
2018-09-24  2:06   ` Simon Marchi
2018-09-24  4:58     ` Tom Tromey
2018-09-15  7:25 ` [PATCH 6/7] Consolidate gdb.GdbError handling Tom Tromey
2018-09-15  7:25 ` [PATCH 7/7] Allow setting a parameter to raise gdb.GdbError Tom Tromey
2018-09-15  9:16   ` Eli Zaretskii
2018-09-24  2:39     ` Simon Marchi
2018-09-24  4:58       ` Tom Tromey
2018-09-24  5:14         ` Tom Tromey
2018-09-24  7:10       ` Eli Zaretskii
2018-09-24 10:24         ` Tom Tromey
2018-09-24 12:39           ` Eli Zaretskii
2018-09-15  7:25 ` [PATCH 5/7] Check for negative argument in Type.template_argument 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).