This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/7] python: Create Python bindings for record history.

On 2016-10-27 02:28, Tim Wiederhake wrote:
This patch adds three new functions to the gdb module in Python:
	- start_recording
	- stop_recording
	- current_recording
start_recording and current_recording return an object of the new type
gdb.Record, which can be used to access the recorded data.

Hi Tim,

Thanks for these patches, I think that offering a Python API for controlling and querying process record is very interesting.

2016-10-26  Tim Wiederhake  <>


	* (SUBDIR_PYTHON_OBS): Add python/py-record.o.
	(SUBDIR_PYTHON_SRCS): Add python/py-record.c.
	(py-record.o): New rule.
	* python/py-record.c: New file.
	* python/py-record.h: New file.
	* python/python-internal.h (gdbpy_start_recording,
	gdbpy_current_recording, gdpy_stop_recording,
	gdbpy_initialize_record): New export.
	* python/python.c (_initialize_python): Add gdbpy_initialize_record.
	(python_GdbMethods): Add gdbpy_start_recording,
	gdbpy_current_recording and gdbpy_stop_recording.
	* target-debug.h (target_debug_print_struct_record_python_interface):
	New define.
	* target-delegates.c: Regenerated.

Little detail: consistently use the present tense in the ChangeLog entries (e.g. Regenerate instead of Regenerated).

	* target.c (target_record_python_interface): New function.
	* target.h: Added struct record_python_interface forward declaration.
	Export target_record_python_interface.
	(struct target_ops): Added to_record_python_interface function.


+/* Executes a command silently. Returns non-zero on success; returns zero and
+   sets Python exception on failure.  */
+static int
+gdbpy_execute_silenty (char *command)

silenty -> silently?

Ideally we should really be using internal APIs instead of using this function... unfortunately they don't seem to exist yet for these tasks. Having GDB build command strings, only to parse them itself right away feels like a waste of resources, but it also makes the code harder to follow.

+/* Implementation of gdb.start_recording (method) -> gdb.Record.  */
+PyObject *
+gdbpy_start_recording (PyObject *self, PyObject *args)
+  char *method = "full";

Should probably be const.

+  if (!PyArg_ParseTuple (args, "|s", &method))
+    return NULL;
+  if (strncmp (method, "full", sizeof ("full")) == 0)
+    {
+      if (!gdbpy_execute_silenty ("record full"))
+	return NULL;
+    }
+  else if (strncmp (method, "btrace", sizeof ("btrace")) == 0)
+    {
+      if (!gdbpy_execute_silenty ("record btrace pt"))
+	{
+	  PyErr_Clear ();
+	  if (!gdbpy_execute_silenty ("record btrace bts"))
+	    return NULL;
+	}
+    }
+  else if ((strncmp (method, "pt", sizeof ("pt")) == 0)
+	   || (strncmp (method, "btrace pt", sizeof ("btrace pt")) == 0))
+    {
+      if (!gdbpy_execute_silenty ("record btrace pt"))
+	return NULL;
+    }
+  else if ((strncmp (method, "bts", sizeof ("bts")) == 0)
+	   || (strncmp (method, "btrace bts", sizeof ("btrace bts")) == 0))
+    {
+      if (!gdbpy_execute_silenty ("record btrace bts"))
+	return NULL;
+    }
+  else
+ return PyErr_Format (PyExc_TypeError, _("Unknown recording method."));
+  return gdbpy_current_recording (self, args);

This function seems to have too much knowledge of the record formats, and will need to be updated for every new record format. And if we want to offer a similar API in the Guile bindings, the knowledge will need to be duplicated. This is where an internal API would be interesting. Assuming there would be an internal function

  record_start (const char *method, const char *format);

Then gdbpy_start_recording could be a simple wrapper around that. If the method/format doesn't exist, it will throw an exception, which you can catch and format correctly.

diff --git a/gdb/target.h b/gdb/target.h
index 176f332..faaed0e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -39,6 +39,7 @@ struct traceframe_info;
 struct expression;
 struct dcache_struct;
 struct inferior;
+struct record_python_interface;

 #include "infrun.h" /* For enum exec_direction_kind.  */
 #include "breakpoint.h" /* For enum bptype.  */
@@ -1230,6 +1231,12 @@ struct target_ops
 				   ULONGEST begin, ULONGEST end, int flags)
       TARGET_DEFAULT_NORETURN (tcomplain ());

+    /* Fill in the record python interface object and return non-zero.
+       Return zero on failure or if no recording is active.  */
+    int (*to_record_python_interface) (struct target_ops *,
+				       struct record_python_interface *)

I am going to nag you again with "internal API" stuff :). Introducing some Python-specific stuff here doesn't feel right. It should probably use a structure (or class) similar to record_python_interface, but which doesn't know anything about Python. Then it's up to the Python layer to query whatever it needs and convert that into whatever structure it wants. But at least, other parts of GDB could use it too.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]