[RFA] Add basic Python API for convenience variables

Message ID 20180422211309.31251-1-tom@tromey.com
State New
Headers show
Series
  • [RFA] Add basic Python API for convenience variables
Related show

Commit Message

Tom Tromey April 22, 2018, 9:13 p.m.
This adds a basic Python API for accessing convenience variables.
With this, convenience variables can be read and set from Python.
Although gdb supports convenience variables whose value changes at
each call, this is not exposed to Python; it could be, but I think
it's just as good to write a convenience function in this situation.

This is PR python/23080.

Tested on x86-64 Fedora 26.

ChangeLog
2018-04-22  Tom Tromey  <tom@tromey.com>

	PR python/23080:
	* NEWS: Update for new functions.
	* python/py-value.c (gdbpy_set_convenience_variable)
	(gdbpy_convenience_variable): New functions.
	* python/python-internal.h (gdbpy_convenience_variable)
	(gdbpy_set_convenience_variable): Declare.
	* python/python.c (python_GdbMethods): Add convenience_variable,
	set_convenience_variable.

doc/ChangeLog
2018-04-22  Tom Tromey  <tom@tromey.com>

	PR python/23080:
	* python.texi (Basic Python): Document gdb.convenience_variable,
	gdb.set_convenience_variable.

testsuite/ChangeLog
2018-04-22  Tom Tromey  <tom@tromey.com>

	PR python/23080:
	* gdb.python/python.exp: Add convenience variable tests.
---
 gdb/ChangeLog                       | 11 ++++++
 gdb/NEWS                            |  6 +++
 gdb/doc/ChangeLog                   |  6 +++
 gdb/doc/python.texi                 | 23 ++++++++++-
 gdb/python/py-value.c               | 77 +++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h        |  2 +
 gdb/python/python.c                 |  8 ++++
 gdb/testsuite/ChangeLog             |  5 +++
 gdb/testsuite/gdb.python/python.exp | 20 ++++++++++
 9 files changed, 156 insertions(+), 2 deletions(-)

-- 
2.13.6

Comments

Eli Zaretskii April 23, 2018, 4:53 p.m. | #1
> From: Tom Tromey <tom@tromey.com>

> Cc: Tom Tromey <tom@tromey.com>

> Date: Sun, 22 Apr 2018 15:13:09 -0600

> 

> This adds a basic Python API for accessing convenience variables.

> With this, convenience variables can be read and set from Python.

> Although gdb supports convenience variables whose value changes at

> each call, this is not exposed to Python; it could be, but I think

> it's just as good to write a convenience function in this situation.

> 

> This is PR python/23080.


Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS

> index 63fe30d175..157756597a 100644

> --- a/gdb/NEWS

> +++ b/gdb/NEWS

> @@ -24,6 +24,12 @@ set|show record btrace cpu

>    Controls the processor to be used for enabling errata workarounds for

>    branch trace decode.

>  

> +* Python API

> +

> +  ** The new functions gdb.convenience_variable and

> +     gdb.set_convenience_variable can be used to get and set the value

> +     of convenience variables.

> +

>  * New targets


This part is OK.

> --- a/gdb/doc/python.texi

> +++ b/gdb/doc/python.texi

> @@ -288,6 +288,26 @@ If no exception is raised, the return value is always an instance of

>  @code{gdb.Value} (@pxref{Values From Inferior}).

>  @end defun


This is also OK, but I have a few nit-picking comments:

> +@findex gdb.convenience_variable

> +@defun gdb.convenience_variable (name)

> +Return the value of the convenience variable (@pxref{Convenience

> +Vars}) named @var{name}.  @var{name} must be a string.  The name

> +should not include the @samp{$} that is used to mark a convenience

> +variable in an expression.  If the convenience variable does not

> +exist, then @code{None} is returned.

> +@end defun

> +

> +@findex gdb.set_convenience_variable

> +@defun gdb.set_convenience_variable (name, value)

> +Set the value of the convenience variable (@pxref{Convenience Vars})

> +named @var{name}.  @var{name} must be a string.  The name should not

> +include the @samp{$} that is used to mark a convenience variable in an

> +expression.  If @var{value} is @code{None}, then the convenience

> +variable is removed.  Otherwise, if @var{value} is not a

> +@code{gdb.Value} (@pxref{Values From Inferior}), it is is converted

> +using the @code{gdb.Value} constructor.

> +@end defun


First, can we avoid repeating what NAME can and cannot be or include,
and have that only once?

Second, "named @var{name}" sounds awkward, I would perhaps suggest
"specified by @var{name}" or somesuch.

And lastly, I would generally suggest to avoid starting a sentence
with @var{something} because in the printed manual this produces a
sentence that begins with a lower-case word.

Since these are minor issues, if you don't feel like fixing them, I
won't object.
Phil Muldoon April 30, 2018, 1:11 p.m. | #2
On 22/04/18 22:13, Tom Tromey wrote:
> This adds a basic Python API for accessing convenience variables.

> With this, convenience variables can be read and set from Python.

> Although gdb supports convenience variables whose value changes at

> each call, this is not exposed to Python; it could be, but I think

> it's just as good to write a convenience function in this situation.

>

>  

>  @findex gdb.find_pc_line

> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c

> index bba6d0b8a4..30a0082da8 100644

> --- a/gdb/python/py-value.c

> +++ b/gdb/python/py-value.c

> @@ -1746,6 +1746,83 @@ gdbpy_history (PyObject *self, PyObject *args)

>    return value_to_value_object (res_val);

>  }

>  

> +/* Return the value of a convenience variable.  */

> +PyObject *

> +gdbpy_convenience_variable (PyObject *self, PyObject *args)

> +{

> +  const char *varname;

> +  struct value *res_val = NULL;

> +

> +  if (!PyArg_ParseTuple (args, "s", &varname))

> +    return NULL;

> +

> +  TRY

> +    {

> +      struct internalvar *var = lookup_only_internalvar (varname);

> +

> +      if (var != NULL)

> +	{

> +	  res_val = value_of_internalvar (python_gdbarch, var);

> +	  if (TYPE_CODE (value_type (res_val)) == TYPE_CODE_VOID)

> +	    res_val = NULL;

> +	}

> +    }

> +  CATCH (except, RETURN_MASK_ALL)

> +    {

> +      GDB_PY_HANDLE_EXCEPTION (except);

> +    }

> +  END_CATCH

> +

> +  if (res_val == NULL)

> +    Py_RETURN_NONE;

> +

> +  return value_to_value_object (res_val);

> +}

> +

> +/* Set the value of a convenience variable.  */

> +PyObject *

> +gdbpy_set_convenience_variable (PyObject *self, PyObject *args)

> +{

> +  const char *varname;

> +  PyObject *value_obj;

> +  struct value *value = NULL;

> +

> +  if (!PyArg_ParseTuple (args, "sO", &varname, &value_obj))

> +    return NULL;

> +

> +  /* None means to clear the variable.  */

> +  if (value_obj != Py_None)

> +    {

> +      value = convert_value_from_python (value_obj);

> +      if (value == NULL)

> +	return NULL;

> +    }

> +

> +  TRY

> +    {

> +      if (value == NULL)

> +	{

> +	  struct internalvar *var = lookup_only_internalvar (varname);

> +

> +	  if (var != NULL)

> +	    clear_internalvar (var);

> +	}

> +      else

> +	{

> +	  struct internalvar *var = lookup_internalvar (varname);

> +

> +	  set_internalvar (var, value);

> +	}

> +    }

> +  CATCH (except, RETURN_MASK_ALL)

> +    {

> +      GDB_PY_HANDLE_EXCEPTION (except);

> +    }

> +  END_CATCH

> +

> +  Py_RETURN_NONE;

> +}

> +


I've read the patch and it LGTM.

Cheers

Phil
Tom Tromey May 9, 2018, 3:43 p.m. | #3
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


Tom> This adds a basic Python API for accessing convenience variables.
Tom> With this, convenience variables can be read and set from Python.
Tom> Although gdb supports convenience variables whose value changes at
Tom> each call, this is not exposed to Python; it could be, but I think
Tom> it's just as good to write a convenience function in this situation.

Tom> This is PR python/23080.

Ping.

Tom
Tom Tromey May 25, 2018, 5:29 p.m. | #4
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


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

Tom> This adds a basic Python API for accessing convenience variables.
Tom> With this, convenience variables can be read and set from Python.
Tom> Although gdb supports convenience variables whose value changes at
Tom> each call, this is not exposed to Python; it could be, but I think
Tom> it's just as good to write a convenience function in this situation.

Tom> This is PR python/23080.

Tom> Ping.

Ping.

Tom
Joel Brobecker May 30, 2018, 11:05 p.m. | #5
Hi Tom,

> This adds a basic Python API for accessing convenience variables.

> With this, convenience variables can be read and set from Python.

> Although gdb supports convenience variables whose value changes at

> each call, this is not exposed to Python; it could be, but I think

> it's just as good to write a convenience function in this situation.

> 

> This is PR python/23080.

> 

> Tested on x86-64 Fedora 26.

> 

> ChangeLog

> 2018-04-22  Tom Tromey  <tom@tromey.com>

> 

> 	PR python/23080:

> 	* NEWS: Update for new functions.

> 	* python/py-value.c (gdbpy_set_convenience_variable)

> 	(gdbpy_convenience_variable): New functions.

> 	* python/python-internal.h (gdbpy_convenience_variable)

> 	(gdbpy_set_convenience_variable): Declare.

> 	* python/python.c (python_GdbMethods): Add convenience_variable,

> 	set_convenience_variable.

> 

> doc/ChangeLog

> 2018-04-22  Tom Tromey  <tom@tromey.com>

> 

> 	PR python/23080:

> 	* python.texi (Basic Python): Document gdb.convenience_variable,

> 	gdb.set_convenience_variable.

> 

> testsuite/ChangeLog

> 2018-04-22  Tom Tromey  <tom@tromey.com>

> 

> 	PR python/23080:

> 	* gdb.python/python.exp: Add convenience variable tests.


Really very sorry about the delay. The patch looks good to me,
so go ahead and push.

Thanks!
-- 
Joel

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 63fe30d175..157756597a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -24,6 +24,12 @@  set|show record btrace cpu
   Controls the processor to be used for enabling errata workarounds for
   branch trace decode.
 
+* Python API
+
+  ** The new functions gdb.convenience_variable and
+     gdb.set_convenience_variable can be used to get and set the value
+     of convenience variables.
+
 * New targets
 
 RiscV ELF			riscv*-*-elf
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index ebd48fffe7..82174fd8a3 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -288,6 +288,26 @@  If no exception is raised, the return value is always an instance of
 @code{gdb.Value} (@pxref{Values From Inferior}).
 @end defun
 
+@findex gdb.convenience_variable
+@defun gdb.convenience_variable (name)
+Return the value of the convenience variable (@pxref{Convenience
+Vars}) named @var{name}.  @var{name} must be a string.  The name
+should not include the @samp{$} that is used to mark a convenience
+variable in an expression.  If the convenience variable does not
+exist, then @code{None} is returned.
+@end defun
+
+@findex gdb.set_convenience_variable
+@defun gdb.set_convenience_variable (name, value)
+Set the value of the convenience variable (@pxref{Convenience Vars})
+named @var{name}.  @var{name} must be a string.  The name should not
+include the @samp{$} that is used to mark a convenience variable in an
+expression.  If @var{value} is @code{None}, then the convenience
+variable is removed.  Otherwise, if @var{value} is not a
+@code{gdb.Value} (@pxref{Values From Inferior}), it is is converted
+using the @code{gdb.Value} constructor.
+@end defun
+
 @findex gdb.parse_and_eval
 @defun gdb.parse_and_eval (expression)
 Parse @var{expression}, which must be a string, as an expression in
@@ -297,8 +317,7 @@  the current language, evaluate it, and return the result as a
 This function can be useful when implementing a new command
 (@pxref{Commands In Python}), as it provides a way to parse the
 command's argument as an expression.  It is also useful simply to
-compute values, for example, it is the only way to get the value of a
-convenience variable (@pxref{Convenience Vars}) as a @code{gdb.Value}.
+compute values.
 @end defun
 
 @findex gdb.find_pc_line
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index bba6d0b8a4..30a0082da8 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1746,6 +1746,83 @@  gdbpy_history (PyObject *self, PyObject *args)
   return value_to_value_object (res_val);
 }
 
+/* Return the value of a convenience variable.  */
+PyObject *
+gdbpy_convenience_variable (PyObject *self, PyObject *args)
+{
+  const char *varname;
+  struct value *res_val = NULL;
+
+  if (!PyArg_ParseTuple (args, "s", &varname))
+    return NULL;
+
+  TRY
+    {
+      struct internalvar *var = lookup_only_internalvar (varname);
+
+      if (var != NULL)
+	{
+	  res_val = value_of_internalvar (python_gdbarch, var);
+	  if (TYPE_CODE (value_type (res_val)) == TYPE_CODE_VOID)
+	    res_val = NULL;
+	}
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  END_CATCH
+
+  if (res_val == NULL)
+    Py_RETURN_NONE;
+
+  return value_to_value_object (res_val);
+}
+
+/* Set the value of a convenience variable.  */
+PyObject *
+gdbpy_set_convenience_variable (PyObject *self, PyObject *args)
+{
+  const char *varname;
+  PyObject *value_obj;
+  struct value *value = NULL;
+
+  if (!PyArg_ParseTuple (args, "sO", &varname, &value_obj))
+    return NULL;
+
+  /* None means to clear the variable.  */
+  if (value_obj != Py_None)
+    {
+      value = convert_value_from_python (value_obj);
+      if (value == NULL)
+	return NULL;
+    }
+
+  TRY
+    {
+      if (value == NULL)
+	{
+	  struct internalvar *var = lookup_only_internalvar (varname);
+
+	  if (var != NULL)
+	    clear_internalvar (var);
+	}
+      else
+	{
+	  struct internalvar *var = lookup_internalvar (varname);
+
+	  set_internalvar (var, value);
+	}
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  END_CATCH
+
+  Py_RETURN_NONE;
+}
+
 /* Returns 1 in OBJ is a gdb.Value object, 0 otherwise.  */
 
 int
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 26400f4fba..495655e759 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -477,6 +477,8 @@  extern enum ext_lang_rc gdbpy_get_matching_xmethod_workers
 
 
 PyObject *gdbpy_history (PyObject *self, PyObject *args);
+PyObject *gdbpy_convenience_variable (PyObject *self, PyObject *args);
+PyObject *gdbpy_set_convenience_variable (PyObject *self, PyObject *args);
 PyObject *gdbpy_breakpoints (PyObject *, PyObject *);
 PyObject *gdbpy_frame_stop_reason_string (PyObject *, PyObject *);
 PyObject *gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9eae8a1aef..4aefe1a373 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -2116,6 +2116,14 @@  Return a tuple containing all inferiors." },
 Invalidate any cached frame objects in gdb.\n\
 Intended for internal use only." },
 
+  { "convenience_variable", gdbpy_convenience_variable, METH_VARARGS,
+    "convenience_variable (NAME) -> value.\n\
+Return the value of the convenience variable $NAME,\n\
+or None if not set." },
+  { "set_convenience_variable", gdbpy_set_convenience_variable, METH_VARARGS,
+    "convenience_variable (NAME, VALUE) -> None.\n\
+Set the value of the convenience variable $NAME." },
+
   {NULL, NULL, 0, NULL}
 };
 
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index cee195f315..45e5aeb691 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -474,3 +474,23 @@  gdb_py_test_silent_cmd "step" "Step into func2" 1
 gdb_py_test_silent_cmd "up" "Step out of func2" 1
 
 gdb_test "python print (gdb.find_pc_line(gdb.selected_frame().pc()).line > line)" "True" "test find_pc_line with resume address"
+
+gdb_test_no_output "set variable \$cvar1 = 23" "set convenience variable"
+gdb_test "python print(gdb.convenience_variable('cvar1'))" "23"
+gdb_test "python print(gdb.convenience_variable('cvar2'))" "None"
+gdb_test_no_output "python gdb.set_convenience_variable('cvar1', 89)" \
+    "change convenience variable from python"
+gdb_test "python print(gdb.convenience_variable('cvar1'))" "89" \
+    "print new value of convenience variable from python"
+gdb_test "print \$cvar1" " = 89" \
+    "print new value of convenience variable from CLI"
+gdb_test_no_output "python gdb.set_convenience_variable('cvar3', -5)" \
+    "make convenience variable from python"
+gdb_test "python print(gdb.convenience_variable('cvar3'))" "-5" \
+    "print value of new convenience variable from python"
+gdb_test_no_output "python gdb.set_convenience_variable('cvar3', None)" \
+    "reset convenience variable from python"
+gdb_test "python print(gdb.convenience_variable('cvar3'))" "None" \
+    "print reset convenience variable from python"
+gdb_test "print \$cvar3" "= void" \
+    "print reset convenience variable from CLI"