diff mbox

[python] Allow explicit locations in breakpoints.

Message ID 04ccc2c4-7827-eedc-d8db-a83a0167acb6@redhat.com
State New
Headers show

Commit Message

Phil Muldoon Aug. 23, 2017, 1:58 p.m. UTC
Hello,

This minor patch allows for explicit locations to be specified in the
gdb.Breakpoint constructor.  It's a minor tweak, and the patch largely
consists of tests to make sure the existing explicit locations
mechanisms work in Python.

Cheers

Phil

--

2017-08-23  Phil Muldoon  <pmuldoon@redhat.com>

       * python/py-breakpoint.c (bppy_init): Use string_to_event_location
       over basic location code.

2017-08-23  Phil Muldoon  <pmuldoon@redhat.com>

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

2017-08-23  Phil Muldoon  <pmuldoon@redhat.com>

       * gdb.python/py-breakpoint.exp (test_bkpt_explicit_loc): Add new
       tests for explicit locations.

--

Comments

Keith Seitz Aug. 23, 2017, 5:51 p.m. UTC | #1
On 08/23/2017 06:58 AM, Phil Muldoon wrote:

> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index 6156eb6179..8431bed939 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -681,7 +681,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>  	case bp_breakpoint:
>  	  {
>  	    event_location_up location
> -	      = string_to_event_location_basic (&copy, current_language);
> +	      = string_to_event_location (&copy, current_language);
>  	    create_breakpoint (python_gdbarch,
>  			       location.get (), NULL, -1, NULL,
>  			       0,

This binds python interfaces to the CLI, and I don't think we want that. I would have expected (perhaps naively) to see explicit locations supported using a more natural python convention, such as using PyArg_ParseTupleAndKeywords.

For example, in MI (mi_cmd_break_insert_1) , we do not use string_to_event_location. We support MI-centric calling conventions by using mi_getopt for argument processing. While MI does use the same option names, they don't (or didn't) have to be. The comments for string_to_event_location should be clearer that this is a CLI-specific implementation. [Perhaps that entire function could be moved to somewhere in cli/?]

I admit, like the MI case, it is almost busywork, but it does (at least) isolate those interfaces from any internal API churn that GDB might undergo.

WDYT?

Keith
Phil Muldoon Aug. 23, 2017, 6:30 p.m. UTC | #2
On 23/08/17 18:51, Keith Seitz wrote:
> On 08/23/2017 06:58 AM, Phil Muldoon wrote:
> 
>> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
>> index 6156eb6179..8431bed939 100644
>> --- a/gdb/python/py-breakpoint.c
>> +++ b/gdb/python/py-breakpoint.c
>> @@ -681,7 +681,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>>  	case bp_breakpoint:
>>  	  {
>>  	    event_location_up location
>> -	      = string_to_event_location_basic (&copy, current_language);
>> +	      = string_to_event_location (&copy, current_language);
>>  	    create_breakpoint (python_gdbarch,
>>  			       location.get (), NULL, -1, NULL,
>>  			       0,
> 
> This binds python interfaces to the CLI, and I don't think we want
> that. I would have expected (perhaps naively) to see explicit
> locations supported using a more natural python convention, such as
> using PyArg_ParseTupleAndKeywords.

My original implementation was that. I had function="foo" etc as
parameters to the constructor. The problem is with the -line
parameters and relative parameters. So line=+3 won't work in the
Python parameter sense. So it would have to be line="+3" in the
constructor. If keywords are all strings, I'm not sure I see the point
of parsing them as keywords when you can just specify the whole string
(i.e.  gdb.Breakpoint("-source=foo.c -line=28"). This is what we do,
more less, with the current constructor: IE

foo = gdb.Breakpoint("bar.c:23")
or
foo = gdb.Breakpoint("functionName").

So this patch allows the addition of explicit location parsing in much
the same vein as we handle regular breakpoints. (That is, with a
CLI-like interface). The gdb.Breakpoint class, for better or for
worse, is based pretty much on the interface to create_breakpoint. I'm
not adverse to implementing keywords; it's a little extra string
wrangling, but I'm not clear on the benefit? The gdb.Breakpoint class
has always handed over the interpretation of breakpoints to GDB using
the CLI like syntax. I can see why MI separates it out, and I take
your point well, in a machine context.

Cheers

Phil
Phil Muldoon Sept. 12, 2017, 10:03 a.m. UTC | #3
On 23/08/17 14:58, Phil Muldoon wrote:
> 
> Hello,
> 
> This minor patch allows for explicit locations to be specified in the
> gdb.Breakpoint constructor.  It's a minor tweak, and the patch largely
> consists of tests to make sure the existing explicit locations
> mechanisms work in Python.

Ping on this.
Phil Muldoon Oct. 2, 2017, 3:18 p.m. UTC | #4
On 12/09/17 11:03, Phil Muldoon wrote:
> On 23/08/17 14:58, Phil Muldoon wrote:
>>
>> Hello,
>>
>> This minor patch allows for explicit locations to be specified in the
>> gdb.Breakpoint constructor.  It's a minor tweak, and the patch largely
>> consists of tests to make sure the existing explicit locations
>> mechanisms work in Python.
> 
> Ping on this.
> 

Ping.
Phil Muldoon Oct. 16, 2017, 11:14 a.m. UTC | #5
On 02/10/17 16:18, Phil Muldoon wrote:
> On 12/09/17 11:03, Phil Muldoon wrote:
>> On 23/08/17 14:58, Phil Muldoon wrote:
>>>
>>> Hello,
>>>
>>> This minor patch allows for explicit locations to be specified in the
>>> gdb.Breakpoint constructor.  It's a minor tweak, and the patch largely
>>> consists of tests to make sure the existing explicit locations
>>> mechanisms work in Python.
>>
>> Ping on this.
>>
> 
> Ping.
> 

Ping

Cheers

Phil
Simon Marchi Oct. 16, 2017, 6:23 p.m. UTC | #6
On 2017-08-23 02:30 PM, Phil Muldoon wrote:
> On 23/08/17 18:51, Keith Seitz wrote:
>> On 08/23/2017 06:58 AM, Phil Muldoon wrote:
>>
>>> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
>>> index 6156eb6179..8431bed939 100644
>>> --- a/gdb/python/py-breakpoint.c
>>> +++ b/gdb/python/py-breakpoint.c
>>> @@ -681,7 +681,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>>>  	case bp_breakpoint:
>>>  	  {
>>>  	    event_location_up location
>>> -	      = string_to_event_location_basic (&copy, current_language);
>>> +	      = string_to_event_location (&copy, current_language);
>>>  	    create_breakpoint (python_gdbarch,
>>>  			       location.get (), NULL, -1, NULL,
>>>  			       0,
>>
>> This binds python interfaces to the CLI, and I don't think we want
>> that. I would have expected (perhaps naively) to see explicit
>> locations supported using a more natural python convention, such as
>> using PyArg_ParseTupleAndKeywords.
> 
> My original implementation was that. I had function="foo" etc as
> parameters to the constructor. The problem is with the -line
> parameters and relative parameters. So line=+3 won't work in the
> Python parameter sense. So it would have to be line="+3" in the
> constructor. If keywords are all strings, I'm not sure I see the point
> of parsing them as keywords when you can just specify the whole string
> (i.e.  gdb.Breakpoint("-source=foo.c -line=28"). This is what we do,
> more less, with the current constructor: IE
> 
> foo = gdb.Breakpoint("bar.c:23")
> or
> foo = gdb.Breakpoint("functionName").
> 
> So this patch allows the addition of explicit location parsing in much
> the same vein as we handle regular breakpoints. (That is, with a
> CLI-like interface). The gdb.Breakpoint class, for better or for
> worse, is based pretty much on the interface to create_breakpoint. I'm
> not adverse to implementing keywords; it's a little extra string
> wrangling, but I'm not clear on the benefit? The gdb.Breakpoint class
> has always handed over the interpretation of breakpoints to GDB using
> the CLI like syntax. I can see why MI separates it out, and I take
> your point well, in a machine context.

I think for Python it would make sense to support the two paradigms.  If you
are writing a Python command that ends up installing a breakpoint, it would
be nice if you could directly pass what you received to the gdb.Breakpoint
constructor and have it parse it (including explicit locations).  For example,

  (gdb) special-break -file foo.c -line 17

But it would also be nice to have a keywords based API, for when the line/file/function
information is already split.  It would avoid having to build an explicit linespec
string just to have GDB parse it after.

In terms of API, I think the "spec" argument could be mutually exclusive with
the function/file/line/etc keywork arguments, which would be added.  An error
would be thrown if you try to use both ways at the same time.

About the line="+3" issue, because this is Python, the line keyword could
probably accept integers and strings.  And if it's a string, there could
be some validation on the format.

Simon
Simon Marchi Oct. 16, 2017, 6:30 p.m. UTC | #7
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index bd138ac3d2..228489f74e 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -547,6 +547,72 @@ proc test_bkpt_events {} {
>      check_last_event breakpoint_deleted
>  }
>  
> +proc test_bkpt_explicit_loc {} {
> +    global srcfile testfile
> +
> +    with_test_prefix test_bkpt_invisible {

This should be test_bkpt_explicit_loc.  But I think I'll do a pass
and make all these procs use "proc_with_prefix", to avoid having to
repeat the proc name.

> +	# 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 (\"-li $bp_location1\")" \
> +	    "Set explicit breakpoint by line" 0> +	gdb_continue_to_breakpoint "Break at multiply" \
> +	    ".*Break at multiply.*"
> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-li +1\")" \
> +	    "Set explicit breakpoint by relative line" 0
> +	gdb_continue_to_breakpoint "Break at add" \
> +	    ".*Break at add.*"
> +	delete_breakpoints
> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-li -1\")" \
> +	    "Set explicit breakpoint by relative negative line" 0
> +	gdb_continue_to_breakpoint "Break at multiply" \
> +	    ".*Break at multiply.*"
> +	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" \
> +	    ".*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" \
> +	    ".*Break at function add.*"
> +	delete_breakpoints
> +	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-source $srcfile -li $bp_location2\")" \
> +	    "Set explicit breakpoint by source file and line number" 0
> +	gdb_continue_to_breakpoint "Break at add" \
> +	    ".*Break at add.*"
> +	delete_breakpoints
> +	gdb_test "python bp1 = gdb.Breakpoint (\"-source $srcfile\")" \
> +	    "RuntimeError: Source filename requires function, label, or line offset.*" \
> +	    "Set invalid explicit breakpoint by source only"
> +	# The below will print a warning but set pending breakpoints.
> +	gdb_test "python bp1 = gdb.Breakpoint (\"-source foo.c -li 5\")" \
> +	    "No source file named foo.*" \
> +	    "Set invalid explicit breakpoint by missing source and line"
> +	gdb_test "python bp1 = gdb.Breakpoint (\"-source $srcfile -li 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."
> +	# Invalid explicit location flags.
> +	gdb_test "python bp1 = gdb.Breakpoint (\"-foo -li 5\")" \
> +	    "RuntimeError: invalid explicit location argument, \"-foo\".*" \
> +	    "Set invalid explicit breakpoint by wrong flag"

For readability, could you add some empty lines between the logical blocks above?

> +    }
> +}
> +
>  test_bkpt_basic
>  test_bkpt_deletion
>  test_bkpt_cond_and_cmds
> @@ -558,3 +624,4 @@ test_bkpt_temporary
>  test_bkpt_address
>  test_bkpt_pending
>  test_bkpt_events
> +test_bkpt_explicit_loc
> 

Simon
Simon Marchi Oct. 16, 2017, 6:32 p.m. UTC | #8
On 2017-10-16 02:23 PM, Simon Marchi wrote:
> I think for Python it would make sense to support the two paradigms.  If you
> are writing a Python command that ends up installing a breakpoint, it would
> be nice if you could directly pass what you received to the gdb.Breakpoint
> constructor and have it parse it (including explicit locations).  For example,
> 
>   (gdb) special-break -file foo.c -line 17
> 
> But it would also be nice to have a keywords based API, for when the line/file/function
> information is already split.  It would avoid having to build an explicit linespec
> string just to have GDB parse it after.
> 
> In terms of API, I think the "spec" argument could be mutually exclusive with
> the function/file/line/etc keywork arguments, which would be added.  An error
> would be thrown if you try to use both ways at the same time.
> 
> About the line="+3" issue, because this is Python, the line keyword could
> probably accept integers and strings.  And if it's a string, there could
> be some validation on the format.
> 
> Simon

Btw, if we went with the idea described above, I think your patch would be
acceptable as-is, and the work to do add keyword arguments could be done separately
(by you or somebody else).

Simon
Phil Muldoon Oct. 16, 2017, 8:24 p.m. UTC | #9
On 16/10/17 19:23, Simon Marchi wrote:
> On 2017-08-23 02:30 PM, Phil Muldoon wrote:
>> On 23/08/17 18:51, Keith Seitz wrote:
>>> On 08/23/2017 06:58 AM, Phil Muldoon wrote:

> I think for Python it would make sense to support the two paradigms.  If you
> are writing a Python command that ends up installing a breakpoint, it would
> be nice if you could directly pass what you received to the gdb.Breakpoint
> constructor and have it parse it (including explicit locations).  For example,
> 
>   (gdb) special-break -file foo.c -line 17
> 
> But it would also be nice to have a keywords based API, for when the line/file/function
> information is already split.  It would avoid having to build an explicit linespec
> string just to have GDB parse it after.
> 
> In terms of API, I think the "spec" argument could be mutually exclusive with
> the function/file/line/etc keywork arguments, which would be added.  An error
> would be thrown if you try to use both ways at the same time.
> 
> About the line="+3" issue, because this is Python, the line keyword could
> probably accept integers and strings.  And if it's a string, there could
> be some validation on the format.
> 

Simon,

Thanks for the review. For the record I have no objection to the
keywords API in addition to the spec line.

But I'm not sure what you mean about the line argument taking an
integer or a string. So line is a problem; it can be:

- line=3 (at line three in the source code)
- line=+3 (plus three lines from current source location)
- line=-3 (minus three lines from current source location)

I'm not sure how I could write a ParseTupleAndKeyword to accept that
in any form other than a string? The -3 will be a minus three, the +3
will just be a 3, and the =3 will be a 3 too. The problem is the
relative "+" information gets lost. Did you have something else in
mind? I guess I could use the O& in the format string to invoke a
converter function? I'm not quite sure what you intend though?

For now, though, I'll add the keywords (as strings) in. This really
prompts me to think we should rewrite the gdb.Breakpoint constructor
to not use create_breakpoint and be more MI-like in the creation of
breakpoints.

Cheers

Phil
Simon Marchi Oct. 16, 2017, 9:25 p.m. UTC | #10
On 2017-10-16 04:24 PM, Phil Muldoon wrote:
> On 16/10/17 19:23, Simon Marchi wrote:
>> On 2017-08-23 02:30 PM, Phil Muldoon wrote:
>>> On 23/08/17 18:51, Keith Seitz wrote:
>>>> On 08/23/2017 06:58 AM, Phil Muldoon wrote:
> 
>> I think for Python it would make sense to support the two paradigms.  If you
>> are writing a Python command that ends up installing a breakpoint, it would
>> be nice if you could directly pass what you received to the gdb.Breakpoint
>> constructor and have it parse it (including explicit locations).  For example,
>>
>>   (gdb) special-break -file foo.c -line 17
>>
>> But it would also be nice to have a keywords based API, for when the line/file/function
>> information is already split.  It would avoid having to build an explicit linespec
>> string just to have GDB parse it after.
>>
>> In terms of API, I think the "spec" argument could be mutually exclusive with
>> the function/file/line/etc keywork arguments, which would be added.  An error
>> would be thrown if you try to use both ways at the same time.
>>
>> About the line="+3" issue, because this is Python, the line keyword could
>> probably accept integers and strings.  And if it's a string, there could
>> be some validation on the format.
>>
> 
> Simon,
> 
> Thanks for the review. For the record I have no objection to the
> keywords API in addition to the spec line.
> 
> But I'm not sure what you mean about the line argument taking an
> integer or a string. So line is a problem; it can be:
> 
> - line=3 (at line three in the source code)
> - line=+3 (plus three lines from current source location)
> - line=-3 (minus three lines from current source location)
> 
> I'm not sure how I could write a ParseTupleAndKeyword to accept that
> in any form other than a string? The -3 will be a minus three, the +3
> will just be a 3, and the =3 will be a 3 too. The problem is the
> relative "+" information gets lost. Did you have something else in
> mind? I guess I could use the O& in the format string to invoke a
> converter function? I'm not quite sure what you intend though?

I think we could support all of these:

  line=3
  line='3'
  line='+3'
  line='-3'

I was thinking about using the O modifier, to get a plain PyObject*, and
then check what type it really is (int or string).  But I didn't know
about O&, which looks like a good fit.  The converter function could
make use of linespec_parse_line_offset if the passed argument is a string.

> For now, though, I'll add the keywords (as strings) in. This really
> prompts me to think we should rewrite the gdb.Breakpoint constructor
> to not use create_breakpoint and be more MI-like in the creation of
> breakpoints.
I'm not sure what you mean, MI uses create_breakpoint in mi_cmd_break_insert_1.

Simon
Phil Muldoon Oct. 16, 2017, 10:01 p.m. UTC | #11
On 16/10/17 22:25, Simon Marchi wrote:

> 
>> For now, though, I'll add the keywords (as strings) in. This really
>> prompts me to think we should rewrite the gdb.Breakpoint constructor
>> to not use create_breakpoint and be more MI-like in the creation of
>> breakpoints.
> I'm not sure what you mean, MI uses create_breakpoint in mi_cmd_break_insert_1.
> 
> Simon
> 
Simon,

My apologies, on reading back I see I was pretty vague. I meant to
create an explicit location using "new_explicit_location" function as
MI does in that function you mentioned instead of
"string_to_event_location". Keith mentioned it in the original email,
I think, and that "string_to_event_location" was designed expressly
for the command-line invocation. I wanted to see if Keith's comment
would work in a gdb.Breakpoint.  The downside is, if we do that (use
new_explicit_location), we won't be able to accept explicit locations
in the spec keyword and only via specific line, function, source-file,
etc keyword based instantiation.  I'll hack on the patch tomorrow and
try to decide which.  I'll repost the patch soon.

Cheers

Phil
Simon Marchi Oct. 16, 2017, 10:25 p.m. UTC | #12
On 2017-10-16 06:01 PM, Phil Muldoon wrote:
> On 16/10/17 22:25, Simon Marchi wrote:
> 
>>
>>> For now, though, I'll add the keywords (as strings) in. This really
>>> prompts me to think we should rewrite the gdb.Breakpoint constructor
>>> to not use create_breakpoint and be more MI-like in the creation of
>>> breakpoints.
>> I'm not sure what you mean, MI uses create_breakpoint in mi_cmd_break_insert_1.
>>
>> Simon
>>
> Simon,
> 
> My apologies, on reading back I see I was pretty vague. I meant to
> create an explicit location using "new_explicit_location" function as
> MI does in that function you mentioned instead of
> "string_to_event_location". Keith mentioned it in the original email,
> I think, and that "string_to_event_location" was designed expressly
> for the command-line invocation. I wanted to see if Keith's comment
> would work in a gdb.Breakpoint.  The downside is, if we do that (use
> new_explicit_location), we won't be able to accept explicit locations
> in the spec keyword and only via specific line, function, source-file,
> etc keyword based instantiation.  I'll hack on the patch tomorrow and
> try to decide which.  I'll repost the patch soon.

But why can't we support both modes?

If "spec" is set, it's a CLI-like location, so you can feed it to
string_to_event_location.

If the keywords line/function/file are set (some of them), use them
with new_explicit_location.

If both spec and line/function/file are used, throw an error.

Simon
diff mbox

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 32d7939e66..a214c2a9e7 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4825,7 +4825,8 @@  class.
 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{break} command, including those set as explicit locations
+(@pxref{Explicit Locations}), 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
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 6156eb6179..8431bed939 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -681,7 +681,7 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	case bp_breakpoint:
 	  {
 	    event_location_up location
-	      = string_to_event_location_basic (&copy, current_language);
+	      = string_to_event_location (&copy, current_language);
 	    create_breakpoint (python_gdbarch,
 			       location.get (), NULL, -1, NULL,
 			       0,
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..228489f74e 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -547,6 +547,72 @@  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 (\"-li $bp_location1\")" \
+	    "Set explicit breakpoint by line" 0
+	gdb_continue_to_breakpoint "Break at multiply" \
+	    ".*Break at multiply.*"
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-li +1\")" \
+	    "Set explicit breakpoint by relative line" 0
+	gdb_continue_to_breakpoint "Break at add" \
+	    ".*Break at add.*"
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-li -1\")" \
+	    "Set explicit breakpoint by relative negative line" 0
+	gdb_continue_to_breakpoint "Break at multiply" \
+	    ".*Break at multiply.*"
+	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" \
+	    ".*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" \
+	    ".*Break at function add.*"
+	delete_breakpoints
+	gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"-source $srcfile -li $bp_location2\")" \
+	    "Set explicit breakpoint by source file and line number" 0
+	gdb_continue_to_breakpoint "Break at add" \
+	    ".*Break at add.*"
+	delete_breakpoints
+	gdb_test "python bp1 = gdb.Breakpoint (\"-source $srcfile\")" \
+	    "RuntimeError: Source filename requires function, label, or line offset.*" \
+	    "Set invalid explicit breakpoint by source only"
+	# The below will print a warning but set pending breakpoints.
+	gdb_test "python bp1 = gdb.Breakpoint (\"-source foo.c -li 5\")" \
+	    "No source file named foo.*" \
+	    "Set invalid explicit breakpoint by missing source and line"
+	gdb_test "python bp1 = gdb.Breakpoint (\"-source $srcfile -li 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."
+	# Invalid explicit location flags.
+	gdb_test "python bp1 = gdb.Breakpoint (\"-foo -li 5\")" \
+	    "RuntimeError: invalid explicit location argument, \"-foo\".*" \
+	    "Set invalid explicit breakpoint by wrong flag"
+    }
+}
+
 test_bkpt_basic
 test_bkpt_deletion
 test_bkpt_cond_and_cmds
@@ -558,3 +624,4 @@  test_bkpt_temporary
 test_bkpt_address
 test_bkpt_pending
 test_bkpt_events
+test_bkpt_explicit_loc