Patchwork [python] Allow explicit locations in breakpoints.

login
register
mail settings
Submitter Phil Muldoon
Date Dec. 7, 2017, 12:15 p.m.
Message ID <ef1506b1-5678-de2a-3490-ebc16ba1e9bc@redhat.com>
Download mbox | patch
Permalink /patch/24774/
State New
Headers show

Comments

Phil Muldoon - Dec. 7, 2017, 12:15 p.m.
On 07/12/17 10:02, Phil Muldoon wrote:
> On 23/11/17 22:17, Simon Marchi wrote:
>> On 2017-11-17 06:02 AM, Phil Muldoon wrote:
>>> All,
>>>
>>> Here's an updated patch. ChangeLog remains the same (except for
>>> additional NEWS entry)
>>>
>>> I just realised this needs a doc review also.
>>>
>>> Cheers
>>>
>>> Phil
>>
>> Hi Phil,
> 
> Here's a new patch. I think I've addressed all of the issues.
> 
> Cheers
> 
> Phil

As my Christmas vacation is rapidly approaching and Pedro mentioned
he'd like to see this for 8.1, he asked me to format the patch to be
git am'able in case he would have to shepherd the last bits of this
patch in. I've included the patch, duly formatted, as an attachment
to this email 

Cheers,

Phil
Simon Marchi - Dec. 7, 2017, 2:54 p.m.
On 2017-12-07 07:15, Phil Muldoon wrote:
> On 07/12/17 10:02, Phil Muldoon wrote:
>> On 23/11/17 22:17, Simon Marchi wrote:
>>> On 2017-11-17 06:02 AM, Phil Muldoon wrote:
>>>> All,
>>>> 
>>>> Here's an updated patch. ChangeLog remains the same (except for
>>>> additional NEWS entry)
>>>> 
>>>> I just realised this needs a doc review also.
>>>> 
>>>> Cheers
>>>> 
>>>> Phil
>>> 
>>> Hi Phil,
>> 
>> Here's a new patch. I think I've addressed all of the issues.
>> 
>> Cheers
>> 
>> Phil
> 
> As my Christmas vacation is rapidly approaching and Pedro mentioned
> he'd like to see this for 8.1, he asked me to format the patch to be
> git am'able in case he would have to shepherd the last bits of this
> patch in. I've included the patch, duly formatted, as an attachment
> to this email
> 
> Cheers,
> 
> Phil

Thanks for the git-am format, it's usually easier to apply.  However, I 
can't seem to find the right baseline.  Which commit did you base this 
on?  If it is old-ish, would it be possible for you to rebase it to 
current master?

Thanks,

Simon

Patch

From 319d9e802995aafcdd14ce8cf63aec1320519079 Mon Sep 17 00:00:00 2001
From: Phil Muldoon <pmuldoon@redhat.com>
Date: Tue, 07 Dec 2017 12:09:08 +0100
Subject: [PATCH] Implement explicit locations for Python breakpoints.
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

This introduces several new keywords to the bppy_init constructor.
The spec parameter is now optional but mutually exclusive to the
explicit keywords source, label, function and line.

gdb/ChangeLog

2017-12-07  Phil Muldoon  <pmuldoon@redhat.com>

       * python/py-breakpoint.c (bppy_init): Use string_to_event_location
       over basic location code. Implement explicit location keywords.
       (bppy_init_validate_args): New function.
       * NEWS: Document Python explicit breakpoint locations.

doc/ChangeLog

2017-12-07  Phil Muldoon  <pmuldoon@redhat.com>

       * python.texi (Breakpoints In Python): Add text relating
       to allowed explicit locations and keywords in gdb.Breakpoints.

testsuite/ChangeLog

2017-12-07  Phil Muldoon  <pmuldoon@redhat.com>

       * gdb.python/py-breakpoint.exp (test_bkpt_explicit_loc): Add new
       tests for explicit locations.
---
 gdb/NEWS                                   |   3 +
 gdb/doc/python.texi                        |  41 ++++-----
 gdb/python/py-breakpoint.c                 | 132 ++++++++++++++++++++++++++---
 gdb/testsuite/gdb.python/py-breakpoint.c   |   2 +-
 gdb/testsuite/gdb.python/py-breakpoint.exp |  80 +++++++++++++++++
 5 files changed, 226 insertions(+), 32 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9246659bfb..592fe70156 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -24,6 +24,9 @@ 
      gdb.new_thread are emitted.  See the manual for further
      description of these.
 
+  ** Python breakpoints can now accept explicit locations.  See the
+     manual for a further description of this feature.
+
 * New features in the GDB remote stub, GDBserver
 
   ** New "--selftest" command line option runs some GDBserver self
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index f661e489bb..25cee3f93e 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4861,27 +4861,30 @@  represented as Python @code{Long} values.
 Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
 class.
 
-@defun Breakpoint.__init__ (spec @r{[}, type @r{[}, wp_class @r{[},internal @r{[},temporary@r{]]]]})
+@defun Breakpoint.__init__ (spec @r{[}, type @r{[}, wp_class @r{[}, internal @r{[}, temporary @r{]}, source @r{]}, function @r{]}, label @r{]}, line @r{]]]]]]]]})
 Create a new breakpoint according to @var{spec}, which is a string
 naming the location of the breakpoint, or an expression that defines a
-watchpoint.  The contents can be any location recognized by the
-@code{break} command, or in the case of a watchpoint, by the
-@code{watch} command.  The optional @var{type} denotes the breakpoint
-to create from the types defined later in this chapter.  This argument
-can be either @code{gdb.BP_BREAKPOINT} or @code{gdb.BP_WATCHPOINT}; it
-defaults to @code{gdb.BP_BREAKPOINT}.  The optional @var{internal}
-argument allows the breakpoint to become invisible to the user.  The
-breakpoint will neither be reported when created, nor will it be
-listed in the output from @code{info breakpoints} (but will be listed
-with the @code{maint info breakpoints} command).  The optional
-@var{temporary} argument makes the breakpoint a temporary breakpoint.
-Temporary breakpoints are deleted after they have been hit.  Any
-further access to the Python breakpoint after it has been hit will
-result in a runtime error (as that breakpoint has now been
-automatically deleted).  The optional @var{wp_class} argument defines
-the class of watchpoint to create, if @var{type} is
-@code{gdb.BP_WATCHPOINT}.  If a watchpoint class is not provided, it
-is assumed to be a @code{gdb.WP_WRITE} class.
+watchpoint. The contents can be any location recognized by the
+@code{break} command or, in the case of a watchpoint, by the
+@code{watch} command.  Alternatively, create a new a explicit location
+breakpoint (@pxref{Explicit Locations}) according to the
+specifications contained in the key words @var{source},
+@var{function}, @var{label} and @var{line}.  The optional @var{type}
+denotes the breakpoint to create from the types defined later in this
+chapter.  This argument can be either @code{gdb.BP_BREAKPOINT} or
+@code{gdb.BP_WATCHPOINT}; it defaults to @code{gdb.BP_BREAKPOINT}.
+The optional @var{internal} argument allows the breakpoint to become
+invisible to the user.  The breakpoint will neither be reported when
+created, nor will it be listed in the output from @code{info
+breakpoints} (but will be listed with the @code{maint info
+breakpoints} command).  The optional @var{temporary} argument makes
+the breakpoint a temporary breakpoint.  Temporary breakpoints are
+deleted after they have been hit.  Any further access to the Python
+breakpoint after it has been hit will result in a runtime error (as
+that breakpoint has now been automatically deleted).  The optional
+@var{wp_class} argument defines the class of watchpoint to create, if
+@var{type} is @code{gdb.BP_WATCHPOINT}.  If a watchpoint class is not
+provided, it is assumed to be a @code{gdb.WP_WRITE} class.
 @end defun
 
 The available types are represented by constants defined in the @code{gdb}
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index d57c2fa05e..417f02d612 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -32,6 +32,7 @@ 
 #include "language.h"
 #include "location.h"
 #include "py-event.h"
+#include "linespec.h"
 
 /* Number of live breakpoints.  */
 static int bppy_live;
@@ -633,25 +634,103 @@  bppy_get_ignore_count (PyObject *self, void *closure)
   return PyInt_FromLong (self_bp->bp->ignore_count);
 }
 
+/* Internal function to validate the Python parameters/keywords
+   provided to bppy_init.  */
+static int
+bppy_init_validate_args (const char *spec, char *source,
+			 char *function, char *label,
+			 char *line, enum bptype type)
+{
+  /* If spec is defined, ensure that none of the explicit location
+     keywords are also defined.  */
+  if (spec != NULL)
+    {
+      if (source != NULL || function != NULL || label != NULL || line != NULL)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Breakpoints specified with spec cannot "
+			     "have source, function, label or line defined."));
+	  return -1;
+	}
+    }
+  else
+    {
+      /* If spec isn't defined, ensure that the user is not trying to
+	 define a watchpoint with an explicit location.  */
+      if (type == bp_watchpoint)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Watchpoints cannot be set by explicit "
+			     "location parameters."));
+	  return -1;
+	}
+      else
+	{
+	  /* Otherwise, ensure some explicit locations are defined.  */
+	  if (source == NULL && function == NULL && label == NULL
+	      && line == NULL)
+	    {
+	      PyErr_SetString (PyExc_RuntimeError,
+			       _("Neither spec nor explicit location set."));
+	      return -1;
+	    }
+	  /* Finally, if source is specified, ensure that line, label
+	     or function are specified too.  */
+	  if (source != NULL && function == NULL && label == NULL
+	      && line == NULL)
+	    {
+	      PyErr_SetString (PyExc_RuntimeError,
+			       _("Specifying a source must also include a "
+				 "line, label or function."));
+	      return -1;
+	    }
+	}
+    }
+  return 1;
+}
+
 /* Python function to create a new breakpoint.  */
 static int
 bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 {
   static const char *keywords[] = { "spec", "type", "wp_class", "internal",
-				    "temporary", NULL };
-  const char *spec;
-  int type = bp_breakpoint;
+				    "temporary","source", "function",
+				    "label", "line", NULL };
+  const char *spec = NULL;
+  enum bptype type = bp_breakpoint;
   int access_type = hw_write;
   PyObject *internal = NULL;
   PyObject *temporary = NULL;
+  PyObject *lineobj = NULL;;
   int internal_bp = 0;
   int temporary_bp = 0;
+  gdb::unique_xmalloc_ptr<char> line;
+  char *label = NULL;
+  char *source = NULL;
+  char *function = NULL;
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiOO", keywords,
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOsssO", keywords,
 					&spec, &type, &access_type,
-					&internal, &temporary))
+					&internal,
+					&temporary, &source,
+					&function, &label, &lineobj))
     return -1;
 
+
+  if (lineobj != NULL)
+    {
+      if (PyInt_Check (lineobj))
+	line.reset (xstrprintf ("%ld", PyInt_AsLong (lineobj)));
+      else if (PyString_Check (lineobj))
+	line = python_string_to_host_string (lineobj);
+      else
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Line keyword should be an integer or a string. "));
+	  return -1;
+	}
+    }
+
   if (internal)
     {
       internal_bp = PyObject_IsTrue (internal);
@@ -666,22 +745,47 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	return -1;
     }
 
+  if (bppy_init_validate_args (spec, source, function, label, line.get (),
+			       type) == -1)
+    return -1;
+
   bppy_pending_object = (gdbpy_breakpoint_object *) self;
   bppy_pending_object->number = -1;
   bppy_pending_object->bp = NULL;
 
   TRY
     {
-      gdb::unique_xmalloc_ptr<char>
-	copy_holder (xstrdup (skip_spaces (spec)));
-      char *copy = copy_holder.get ();
-
       switch (type)
 	{
 	case bp_breakpoint:
 	  {
-	    event_location_up location
-	      = string_to_event_location_basic (&copy, current_language);
+	    event_location_up location;
+
+	    if (spec != NULL)
+	      {
+		gdb::unique_xmalloc_ptr<char>
+		  copy_holder (xstrdup (skip_spaces (spec)));
+		char *copy = copy_holder.get ();
+
+		location  = string_to_event_location (&copy,
+						      current_language);
+	      }
+	    else
+	      {
+		struct explicit_location explicit_loc;
+
+		initialize_explicit_location (&explicit_loc);
+		explicit_loc.source_filename = source;
+		explicit_loc.function_name = function;
+		explicit_loc.label_name = label;
+
+		if (line != NULL)
+		  explicit_loc.line_offset =
+		    linespec_parse_line_offset (line.get ());
+
+		location = new_explicit_location (&explicit_loc);
+	      }
+
 	    create_breakpoint (python_gdbarch,
 			       location.get (), NULL, -1, NULL,
 			       0,
@@ -692,8 +796,12 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 			       0, 1, internal_bp, 0);
 	    break;
 	  }
-        case bp_watchpoint:
+	case bp_watchpoint:
 	  {
+	    gdb::unique_xmalloc_ptr<char>
+	      copy_holder (xstrdup (skip_spaces (spec)));
+	    char *copy = copy_holder.get ();
+
 	    if (access_type == hw_write)
 	      watch_command_wrapper (copy, 0, internal_bp);
 	    else if (access_type == hw_access)
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.c b/gdb/testsuite/gdb.python/py-breakpoint.c
index df6163e53a..562cab35f7 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.c
+++ b/gdb/testsuite/gdb.python/py-breakpoint.c
@@ -24,7 +24,7 @@  int multiply (int i)
 
 int add (int i)
 {
-  return i + i; 
+  return i + i;  /* Break at function add.  */
 }
 
 
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index bd138ac3d2..2eca7f366a 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -547,6 +547,85 @@  proc test_bkpt_events {} {
     check_last_event breakpoint_deleted
 }
 
+proc test_bkpt_explicit_loc {} {
+    global srcfile testfile
+
+    with_test_prefix test_bkpt_invisible {
+	# Start with a fresh gdb.
+	clean_restart ${testfile}
+
+	if ![runto_main] then {
+	    fail "cannot run to main."
+	    return 0
+	}
+
+	delete_breakpoints
+
+	set bp_location1 [gdb_get_line_number "Break at multiply."]
+	set bp_location2 [gdb_get_line_number "Break at add."]
+
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=$bp_location1)" \
+	    "set explicit breakpoint by line" 0
+	gdb_continue_to_breakpoint "break at multiply for explicit line" \
+	    ".*Break at multiply.*"
+
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=\"+1\")" \
+	    "set explicit breakpoint by relative line" 0
+	gdb_continue_to_breakpoint "break at add for relative line" \
+	    ".*Break at add.*"
+
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (line=\"-1\")" \
+	    "set explicit breakpoint by relative negative line" 0
+	gdb_continue_to_breakpoint "break at multiply for negative line" \
+	    ".*Break at multiply.*"
+
+	delete_breakpoints
+	gdb_test "python bp1 = gdb.Breakpoint (line=bp1)" \
+	    "RuntimeError: Line keyword should be an integer or a string.*" \
+	    "set explicit breakpoint by invalid line type"
+
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (function=\"add\")" \
+	    "set explicit breakpoint by function" 0
+	gdb_continue_to_breakpoint "break at function add for function" \
+	    ".*Break at function add.*"
+
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (source=\"$srcfile\", function=\"add\")" \
+	    "set explicit breakpoint by source file and function" 0
+	gdb_continue_to_breakpoint "break at function add for source and function" \
+	    ".*Break at function add.*"
+
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"$bp_location2\")" \
+	    "set explicit breakpoint by source file and line number" 0
+	gdb_continue_to_breakpoint "break at add for source and line" \
+	    ".*Break at add.*"
+
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-source $srcfile -line $bp_location2\")" \
+	    "set explicit breakpoint by source file and line number in spec" 0
+	gdb_continue_to_breakpoint "break at add for source and line in spec" \
+	    ".*Break at add.*"
+
+	delete_breakpoints
+	gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\")" \
+	    "RuntimeError: Specifying a source must also include a line, label or function.*" \
+	    "set invalid explicit breakpoint by source only"
+
+	gdb_test "python bp1 = gdb.Breakpoint (source=\"foo.c\", line=\"5\")" \
+	    "No source file named foo.*" \
+	    "set invalid explicit breakpoint by missing source and line"
+	gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
+	    "No line 900 in file \"$srcfile\".*" \
+	    "set invalid explicit breakpoint by source and invalid line"
+	gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
+	    "Function \"blah\" not defined.*" \
+	    "set invalid explicit breakpoint by missing function"
+    }
+}
+
 test_bkpt_basic
 test_bkpt_deletion
 test_bkpt_cond_and_cmds
@@ -558,3 +637,4 @@  test_bkpt_temporary
 test_bkpt_address
 test_bkpt_pending
 test_bkpt_events
+test_bkpt_explicit_loc
-- 
2.14.3