* [python] Allow explicit locations in breakpoints. @ 2017-08-23 13:59 Phil Muldoon 2017-08-23 17:51 ` Keith Seitz ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Phil Muldoon @ 2017-08-23 13:59 UTC (permalink / raw) To: gdb-patches 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. -- 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 (©, current_language); + = string_to_event_location (©, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-08-23 13:59 [python] Allow explicit locations in breakpoints Phil Muldoon @ 2017-08-23 17:51 ` Keith Seitz 2017-08-23 18:31 ` Phil Muldoon 2017-09-12 10:03 ` Phil Muldoon 2017-10-16 18:31 ` Simon Marchi 2 siblings, 1 reply; 24+ messages in thread From: Keith Seitz @ 2017-08-23 17:51 UTC (permalink / raw) To: Phil Muldoon, gdb-patches 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 (©, current_language); > + = string_to_event_location (©, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-08-23 17:51 ` Keith Seitz @ 2017-08-23 18:31 ` Phil Muldoon 2017-10-16 18:23 ` Simon Marchi 0 siblings, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-08-23 18:31 UTC (permalink / raw) To: Keith Seitz, gdb-patches 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 (©, current_language); >> + = string_to_event_location (©, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-08-23 18:31 ` Phil Muldoon @ 2017-10-16 18:23 ` Simon Marchi 2017-10-16 18:33 ` Simon Marchi 2017-10-16 20:24 ` Phil Muldoon 0 siblings, 2 replies; 24+ messages in thread From: Simon Marchi @ 2017-10-16 18:23 UTC (permalink / raw) To: Phil Muldoon, Keith Seitz, gdb-patches 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 (©, current_language); >>> + = string_to_event_location (©, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 18:23 ` Simon Marchi @ 2017-10-16 18:33 ` Simon Marchi 2017-10-16 20:24 ` Phil Muldoon 1 sibling, 0 replies; 24+ messages in thread From: Simon Marchi @ 2017-10-16 18:33 UTC (permalink / raw) To: Phil Muldoon, Keith Seitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 18:23 ` Simon Marchi 2017-10-16 18:33 ` Simon Marchi @ 2017-10-16 20:24 ` Phil Muldoon 2017-10-16 21:26 ` Simon Marchi 1 sibling, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-10-16 20:24 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 20:24 ` Phil Muldoon @ 2017-10-16 21:26 ` Simon Marchi 2017-10-16 22:01 ` Phil Muldoon 0 siblings, 1 reply; 24+ messages in thread From: Simon Marchi @ 2017-10-16 21:26 UTC (permalink / raw) To: Phil Muldoon, Keith Seitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 21:26 ` Simon Marchi @ 2017-10-16 22:01 ` Phil Muldoon 2017-10-16 22:26 ` Simon Marchi 0 siblings, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-10-16 22:01 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 22:01 ` Phil Muldoon @ 2017-10-16 22:26 ` Simon Marchi 2017-11-17 11:02 ` Phil Muldoon 0 siblings, 1 reply; 24+ messages in thread From: Simon Marchi @ 2017-10-16 22:26 UTC (permalink / raw) To: Phil Muldoon, Keith Seitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-16 22:26 ` Simon Marchi @ 2017-11-17 11:02 ` Phil Muldoon 2017-11-17 13:31 ` Eli Zaretskii 2017-11-23 22:17 ` Simon Marchi 0 siblings, 2 replies; 24+ messages in thread From: Phil Muldoon @ 2017-11-17 11:02 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, eliz, gdb-patches On 16/10/17 23:25, Simon Marchi wrote: > 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 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 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..85d514f57a 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; @@ -638,20 +639,73 @@ static int bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) { static const char *keywords[] = { "spec", "type", "wp_class", "internal", - "temporary", NULL }; - const char *spec; + "temporary","source", "function", + "label", "line", NULL }; + const char *spec = NULL; int type = bp_breakpoint; int access_type = hw_write; PyObject *internal = NULL; PyObject *temporary = NULL; int internal_bp = 0; int temporary_bp = 0; + char *line = NULL; + char *label = NULL; + char *source = NULL; + char *function = NULL; - if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiOO", keywords, + if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOssss", keywords, &spec, &type, &access_type, - &internal, &temporary)) + &internal, + &temporary, &source, + &function, &label, &line)) return -1; + /* 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; + } + } + } + if (internal) { internal_bp = PyObject_IsTrue (internal); @@ -672,16 +726,37 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) 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 (©, 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 (©, + 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); + + location = new_explicit_location (&explicit_loc); + } + create_breakpoint (python_gdbarch, location.get (), NULL, -1, NULL, 0, @@ -692,8 +767,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..d91e5deb50 100644 --- a/gdb/testsuite/gdb.python/py-breakpoint.exp +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp @@ -547,6 +547,74 @@ 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" \ + ".*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" \ + ".*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" \ + ".*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\", line=\"$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: 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 +626,4 @@ test_bkpt_temporary test_bkpt_address test_bkpt_pending test_bkpt_events +test_bkpt_explicit_loc ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-11-17 11:02 ` Phil Muldoon @ 2017-11-17 13:31 ` Eli Zaretskii 2017-11-17 14:02 ` Phil Muldoon 2017-11-23 22:17 ` Simon Marchi 1 sibling, 1 reply; 24+ messages in thread From: Eli Zaretskii @ 2017-11-17 13:31 UTC (permalink / raw) To: Phil Muldoon; +Cc: simon.marchi, keiths, gdb-patches > From: Phil Muldoon <pmuldoon@redhat.com> > Date: Fri, 17 Nov 2017 11:02:08 +0000 > > I just realised this needs a doc review also. Doc review coming up. > 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. I think "a further" should lose the "a" part. Also, how about mentioning the node name in the manual where this is described? > +watchpoint. The contents can be any location recognized by the ^^ Two spaces here, please. > +@code{break} command or, in the case of a watchpoint, by the > +@code{watch} command. Alternatively, create a new a explicit location ^^^^^^^^^^^^^^^^^^^^^^^ The second "a" should be deleted. > +@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 I think "denotes the breakpoint to create from the types" is at least confusing. What did you mean here? > +@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 ^ I think you want a colon here, not a period, because the next sentence explains what does it mean for a breakpoint to be "invisible". Alternatively, start the next sentence with "Invisible breakpoints will neither be reported ...". > +created, nor will it be listed in the output from @code{info > +breakpoints} (but will be listed with the @code{maint info ^^^^ "by", I think, not "with". > +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 I think you want to say "to the temporary breakpoint" here. The fact that it is a Python breakpoint is not really relevant. > +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. What are the other supported classes? Either state that here or point to where that is described. Thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-11-17 13:31 ` Eli Zaretskii @ 2017-11-17 14:02 ` Phil Muldoon 0 siblings, 0 replies; 24+ messages in thread From: Phil Muldoon @ 2017-11-17 14:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: simon.marchi, keiths, gdb-patches On 17/11/17 13:30, Eli Zaretskii wrote: >> From: Phil Muldoon <pmuldoon@redhat.com> >> Date: Fri, 17 Nov 2017 11:02:08 +0000 >> >> I just realised this needs a doc review also. > > Doc review coming up. > >> 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. > > I think "a further" should lose the "a" part. Also, how about > mentioning the node name in the manual where this is described? OK. >> +@code{break} command or, in the case of a watchpoint, by the >> +@code{watch} command. Alternatively, create a new a explicit location > ^^^^^^^^^^^^^^^^^^^^^^^ > The second "a" should be deleted. OK. The other comments refer to existing documentation that I have not actually changed. I formatted the paragraph with the additional sentence I added and re-formatted (the wrapping affected the whole paragraph). I have no problem making the changes you suggest, they are good, but can I please make them in another patchset so the changes made in this one stay related? For the purposes of clarity, the related changes in this patch are: +@defun Breakpoint.__init__ (spec @r{[}, type @r{[}, wp_class @r{[}, internal @r{[}, temporary @r{]}, source @r{]}, function @r{]}, label @r{]}, line @r{]]]]]]]]}) and +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}. Cheers Phil ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-11-17 11:02 ` Phil Muldoon 2017-11-17 13:31 ` Eli Zaretskii @ 2017-11-23 22:17 ` Simon Marchi 2017-11-24 14:07 ` Phil Muldoon 2017-12-07 10:02 ` Phil Muldoon 1 sibling, 2 replies; 24+ messages in thread From: Simon Marchi @ 2017-11-23 22:17 UTC (permalink / raw) To: Phil Muldoon, Keith Seitz, eliz, gdb-patches 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, That > --- 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; > @@ -638,20 +639,73 @@ static int > bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) > { > static const char *keywords[] = { "spec", "type", "wp_class", "internal", > - "temporary", NULL }; > - const char *spec; > + "temporary","source", "function", > + "label", "line", NULL }; > + const char *spec = NULL; > int type = bp_breakpoint; > int access_type = hw_write; > PyObject *internal = NULL; > PyObject *temporary = NULL; > int internal_bp = 0; > int temporary_bp = 0; > + char *line = NULL; > + char *label = NULL; > + char *source = NULL; > + char *function = NULL; > > - if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiOO", keywords, > + if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOssss", keywords, > &spec, &type, &access_type, > - &internal, &temporary)) > + &internal, > + &temporary, &source, > + &function, &label, &line)) > return -1; > > + /* 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; > + } > + } > + } > + I would suggest putting the argument validation code in its own function (e.g. bppy_init_validate_args), to keep bppy_init of a reasonnable size. > if (internal) > { > internal_bp = PyObject_IsTrue (internal); > @@ -672,16 +726,37 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) > > 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 (©, 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 (©, > + 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); For convenience, I think it would be nice to accept integers for line. So perhaps something like this: From d0e3122bca8b4fcb2d6f303bdc66f68929bdc14f Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@ericsson.com> Date: Thu, 23 Nov 2017 16:58:35 -0500 Subject: [PATCH] suggestion --- gdb/python/py-breakpoint.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index 7fb35ce..5ee0fed 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -648,23 +648,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) PyObject *temporary = NULL; int internal_bp = 0; int temporary_bp = 0; - char *line = NULL; + PyObject *lineobj = NULL; char *label = NULL; char *source = NULL; char *function = NULL; - if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOssss", keywords, + if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOsssO", keywords, &spec, &type, &access_type, &internal, &temporary, &source, - &function, &label, &line)) + &function, &label, &lineobj)) return -1; /* 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) + if (source != NULL || function != NULL || label != NULL || lineobj != NULL) { PyErr_SetString (PyExc_RuntimeError, _("Breakpoints specified with spec cannot " @@ -687,7 +687,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) { /* Otherwise, ensure some explicit locations are defined. */ if (source == NULL && function == NULL && label == NULL - && line == NULL) + && lineobj == NULL) { PyErr_SetString (PyExc_RuntimeError, _("Neither spec nor explicit location set.")); @@ -696,7 +696,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) /* Finally, if source is specified, ensure that line, label or function are specified too. */ if (source != NULL && function == NULL && label == NULL - && line == NULL) + && lineobj == NULL) { PyErr_SetString (PyExc_RuntimeError, _("Specifying a source must also include a " @@ -706,6 +706,13 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) } } + if (lineobj != NULL && !PyInt_Check (lineobj) && !PyString_Check (lineobj)) + { + PyErr_SetString (PyExc_TypeError, + _ ("line must be an integer or a string.")); + return -1; + } + if (internal) { internal_bp = PyObject_IsTrue (internal); @@ -750,9 +757,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) explicit_loc.function_name = function; explicit_loc.label_name = label; - if (line != NULL) - explicit_loc.line_offset = - linespec_parse_line_offset (line); + if (lineobj != NULL) + { + gdb::unique_xmalloc_ptr<char> line; + + if (PyInt_Check (lineobj)) + line.reset (xstrprintf ("%ld", PyInt_AsLong (lineobj))); + else if (PyString_Check (lineobj)) + line = python_string_to_host_string (lineobj); + else + { + /* We should have validated the type up there... */ + gdb_assert_not_reached ("Unexpected type of lineobj."); + } + + explicit_loc.line_offset + = linespec_parse_line_offset (line.get ()); + } location = new_explicit_location (&explicit_loc); } -- 2.7.4 > + > + location = new_explicit_location (&explicit_loc); > + } > + > create_breakpoint (python_gdbarch, > location.get (), NULL, -1, NULL, > 0, > @@ -692,8 +767,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..d91e5deb50 100644 > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp > @@ -547,6 +547,74 @@ 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" \ > + ".*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" \ > + ".*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" \ > + ".*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\", line=\"$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: 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" > + } The test names should start with a lower case letter: https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Follow_the_test_name_convention Also make sure that test names are not duplicated. You can use this command to quickly find duplicated names in gdb.sum: $ cat testsuite/gdb.sum | grep -e PASS -e FAIL | sort | uniq -c | sort -n Would it be good to test that explicit breakpoint locations through the spec argument work as well? Things like gdb.Breakpoint('-line 5') ? Thanks, Simon ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-11-23 22:17 ` Simon Marchi @ 2017-11-24 14:07 ` Phil Muldoon 2017-12-07 10:02 ` Phil Muldoon 1 sibling, 0 replies; 24+ messages in thread From: Phil Muldoon @ 2017-11-24 14:07 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, eliz, gdb-patches On 23/11/17 22:17, Simon Marchi wrote: > On 2017-11-17 06:02 AM, Phil Muldoon wrote: > > I would suggest putting the argument validation code in its own function (e.g. > bppy_init_validate_args), to keep bppy_init of a reasonnable size. Okay, will do. > For convenience, I think it would be nice to accept integers for line. So perhaps > something like this: > > From d0e3122bca8b4fcb2d6f303bdc66f68929bdc14f Mon Sep 17 00:00:00 2001 > From: Simon Marchi <simon.marchi@ericsson.com> > Date: Thu, 23 Nov 2017 16:58:35 -0500 > Subject: [PATCH] suggestion Yeah good idea. I remember we talked about this, but I must have forgotten about it. I'll add your patch in. >> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp >> index bd138ac3d2..d91e5deb50 100644 >> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp >> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp >> @@ -547,6 +547,74 @@ 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" \ >> + ".*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" \ >> + ".*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" \ >> + ".*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\", line=\"$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: 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" >> + } > > The test names should start with a lower case letter: > > https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Follow_the_test_name_convention > > Also make sure that test names are not duplicated. You can use this command to quickly > find duplicated names in gdb.sum: > > $ cat testsuite/gdb.sum | grep -e PASS -e FAIL | sort | uniq -c | sort -n I thought they all were. Regardless, I'll check again before resubmitting. > Would it be good to test that explicit breakpoint locations through the spec > argument work as well? Things like > > gdb.Breakpoint('-line 5') Yeah, good idea. They all originally were but I converted them to the keywords without thinking of the spec case Cheers Phil ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-11-23 22:17 ` Simon Marchi 2017-11-24 14:07 ` Phil Muldoon @ 2017-12-07 10:02 ` Phil Muldoon 2017-12-07 12:16 ` Phil Muldoon 2017-12-08 13:50 ` Eli Zaretskii 1 sibling, 2 replies; 24+ messages in thread From: Phil Muldoon @ 2017-12-07 10:02 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, eliz, gdb-patches 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 -- 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 (©, 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 (©, + 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-12-07 10:02 ` Phil Muldoon @ 2017-12-07 12:16 ` Phil Muldoon 2017-12-07 14:54 ` Simon Marchi 2017-12-08 13:50 ` Eli Zaretskii 1 sibling, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-12-07 12:16 UTC (permalink / raw) To: Simon Marchi, Keith Seitz, palves, eliz, gdb-patches [-- Attachment #1: Type: text/plain, Size: 725 bytes --] 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 [-- Attachment #2: 0001-Implement-explicit-locations-for-Python-breakpoints.patch --] [-- Type: text/x-patch, Size: 14915 bytes --] 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 (©, 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 (©, + 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-12-07 12:16 ` Phil Muldoon @ 2017-12-07 14:54 ` Simon Marchi 2017-12-07 15:12 ` Phil Muldoon 0 siblings, 1 reply; 24+ messages in thread From: Simon Marchi @ 2017-12-07 14:54 UTC (permalink / raw) To: Phil Muldoon; +Cc: Simon Marchi, Keith Seitz, palves, eliz, gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-12-07 14:54 ` Simon Marchi @ 2017-12-07 15:12 ` Phil Muldoon 2017-12-07 16:41 ` Simon Marchi 0 siblings, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-12-07 15:12 UTC (permalink / raw) To: Simon Marchi; +Cc: Simon Marchi, Keith Seitz, palves, eliz, gdb-patches [-- Attachment #1: Type: text/plain, Size: 606 bytes --] On 07/12/17 14:54, Simon Marchi wrote: > 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: >> 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 Apologies, my github branch was more behind master than I at first thought. I've attached the update patch. Cheers, Phil [-- Attachment #2: 0001-Implement-explicit-locations-for-Python-breakpoints.patch --] [-- Type: text/x-patch, Size: 14956 bytes --] From: Phil Muldoon <pmuldoon@redhat.com> Date: Thu, 7 Dec 2017 15:05:51 +0000 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 | 4 + gdb/doc/python.texi | 41 +++++---- gdb/python/py-breakpoint.c | 134 ++++++++++++++++++++++++++--- gdb/testsuite/gdb.python/py-breakpoint.c | 2 +- gdb/testsuite/gdb.python/py-breakpoint.exp | 80 +++++++++++++++++ 5 files changed, 228 insertions(+), 33 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 2262b259a8..72b4057f4d 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -115,6 +115,10 @@ command allows the setting of a large number of breakpoints via a regex pattern in Python. See the manual for further details. + ** 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 ** GDBserver is now able to start inferior processes with a diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index f411f60d7e..28a7a1a9f5 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -4878,27 +4878,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 2574683ed4..f865317ab3 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; @@ -631,25 +632,104 @@ 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); @@ -664,23 +744,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))); - const char *copy = copy_holder.get (); - switch (type) { case bp_breakpoint: { - event_location_up location - = string_to_event_location_basic (©, current_language, - symbol_name_match_type::WILD); + event_location_up location; + + if (spec != NULL) + { + gdb::unique_xmalloc_ptr<char> + copy_holder (xstrdup (skip_spaces (spec))); + const char *copy = copy_holder.get (); + + location = string_to_event_location (©, + 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, @@ -691,8 +795,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 6c39f13696..e89b9b8446 100644 --- a/gdb/testsuite/gdb.python/py-breakpoint.exp +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp @@ -531,6 +531,85 @@ proc_with_prefix 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 @@ -542,3 +621,4 @@ test_bkpt_temporary test_bkpt_address test_bkpt_pending test_bkpt_events +test_bkpt_explicit_loc -- 2.14.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-12-07 15:12 ` Phil Muldoon @ 2017-12-07 16:41 ` Simon Marchi 0 siblings, 0 replies; 24+ messages in thread From: Simon Marchi @ 2017-12-07 16:41 UTC (permalink / raw) To: Phil Muldoon, Simon Marchi; +Cc: Keith Seitz, palves, eliz, gdb-patches On 2017-12-07 10:12 AM, Phil Muldoon wrote: > Apologies, my github branch was more behind master than I at first > thought. I've attached the update patch. > > Cheers, > > Phil > Hi Phil, That version is fine with me. As mentioned on IRC, it would be nice to have a Python parameter equivalent to the -q/--qualified CLI switch. I'd be fine with it being done as another patch on top. Simon ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-12-07 10:02 ` Phil Muldoon 2017-12-07 12:16 ` Phil Muldoon @ 2017-12-08 13:50 ` Eli Zaretskii 1 sibling, 0 replies; 24+ messages in thread From: Eli Zaretskii @ 2017-12-08 13:50 UTC (permalink / raw) To: Phil Muldoon; +Cc: simon.marchi, keiths, gdb-patches > From: Phil Muldoon <pmuldoon@redhat.com> > Date: Thu, 7 Dec 2017 10:02:44 +0000 > > 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. ^^^^^^^^^ "the detailed"? Otherwise the documentation parts are okay. Thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-08-23 13:59 [python] Allow explicit locations in breakpoints Phil Muldoon 2017-08-23 17:51 ` Keith Seitz @ 2017-09-12 10:03 ` Phil Muldoon 2017-10-02 15:18 ` Phil Muldoon 2017-10-16 18:31 ` Simon Marchi 2 siblings, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-09-12 10:03 UTC (permalink / raw) To: gdb-patches 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-09-12 10:03 ` Phil Muldoon @ 2017-10-02 15:18 ` Phil Muldoon 2017-10-16 11:14 ` Phil Muldoon 0 siblings, 1 reply; 24+ messages in thread From: Phil Muldoon @ 2017-10-02 15:18 UTC (permalink / raw) To: gdb-patches 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-10-02 15:18 ` Phil Muldoon @ 2017-10-16 11:14 ` Phil Muldoon 0 siblings, 0 replies; 24+ messages in thread From: Phil Muldoon @ 2017-10-16 11:14 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [python] Allow explicit locations in breakpoints. 2017-08-23 13:59 [python] Allow explicit locations in breakpoints Phil Muldoon 2017-08-23 17:51 ` Keith Seitz 2017-09-12 10:03 ` Phil Muldoon @ 2017-10-16 18:31 ` Simon Marchi 2 siblings, 0 replies; 24+ messages in thread From: Simon Marchi @ 2017-10-16 18:31 UTC (permalink / raw) To: Phil Muldoon, gdb-patches; +Cc: Keith Seitz > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-12-08 13:50 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-23 13:59 [python] Allow explicit locations in breakpoints Phil Muldoon 2017-08-23 17:51 ` Keith Seitz 2017-08-23 18:31 ` Phil Muldoon 2017-10-16 18:23 ` Simon Marchi 2017-10-16 18:33 ` Simon Marchi 2017-10-16 20:24 ` Phil Muldoon 2017-10-16 21:26 ` Simon Marchi 2017-10-16 22:01 ` Phil Muldoon 2017-10-16 22:26 ` Simon Marchi 2017-11-17 11:02 ` Phil Muldoon 2017-11-17 13:31 ` Eli Zaretskii 2017-11-17 14:02 ` Phil Muldoon 2017-11-23 22:17 ` Simon Marchi 2017-11-24 14:07 ` Phil Muldoon 2017-12-07 10:02 ` Phil Muldoon 2017-12-07 12:16 ` Phil Muldoon 2017-12-07 14:54 ` Simon Marchi 2017-12-07 15:12 ` Phil Muldoon 2017-12-07 16:41 ` Simon Marchi 2017-12-08 13:50 ` Eli Zaretskii 2017-09-12 10:03 ` Phil Muldoon 2017-10-02 15:18 ` Phil Muldoon 2017-10-16 11:14 ` Phil Muldoon 2017-10-16 18:31 ` Simon Marchi
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).