Discussion:
Explicit Linespecs Branch Created
Keith Seitz
2012-06-11 20:26:53 UTC
Permalink
Hi,

For the past few weeks, I've been working on a little project to
implement what I've been calling "explicit linespecs." These are
linespecs that bypass the linespec parser altogether. [Example: "break
-function main -label foo -offset 3"]

This feature has been requested a few times, most notably as mi/13139.

A consequence of this patch series is also that all linespecs (on
supported breakpoint types) are now converted to this explicit form. The
addr_string is basically used for display purposes only.

Since this patchset is so large and invasive, I have checked it into the
Archer repository for "pre-review." It's in the
archer-keiths-explicit-linespecs branch. I would especially appreciate
feedback on:

- Syntax of the CLI and MI breakpoint commands.

+ MI: -break-insert -c 'foo == bar' -t -e -s source.c -f function -l
label -o offset

Currently, the explicit linespec flag (-e) must be the last option in
the command. Everything after "-e" is restricted to explicit linespecs.
[This restriction was enabled so that we don't have to require some
really ugly quoting, e.g., -break-insert -e "-f main -offset 3" -c "foo
== bar".

+ CLI: break -source source.c -function function -label label -offset
offset -condition "foo == bar" -thread 1

The various flags may be abbreviated, "-func" or "-f" is acceptable.
["thread" takes precedence over "task"]

When setting an explicit linespec, users may not use the keywords "if",
"thread", or "task". If using explicit linespecs, *everything* must be
explicitly specified.

- New error messages

+ I have introduced several new error messages. They should be
double-checked:
o "invalid linespec argument, \"%s\"" : This is output when the
user attempts to use an invalid explicit linespec flag, e.g., "-foobar".
o "missing argument for \"%s\" : This is output when the user omits
an argument for a given explicit linespec flag.

When these syntaxes are finalized, I will commit new documentation for
them to the branch. [As usual, it's the last task on my TODO list. If I
would stop finding bugs, I could probably get around to this sooner than
later.]

This patchset is not ready for prime time (certainly not until after 7.5
branches). I'm still dreaming up test cases, finding new bugs (e.g.,
"cond" doesn't play nicely with this yet) and tweaking this and that,
but AFAIK, it causes no regressions in the test suite.

Any feedback appreciated,

Keith
Tom Tromey
2012-06-15 18:31:43 UTC
Permalink
Keith> Since this patchset is so large and invasive, I have checked it into
Keith> the Archer repository for "pre-review." It's in the
Keith> archer-keiths-explicit-linespecs branch.

I started looking at it, but really I think you should either do the
final polishing (docs and ChangeLog) and submit it; or send an RFC to
the gdb@ list. The problem with posting here is that, even if we all
agree about everything, it'll still have to go through another round of
kibitzing when you submit it, and so you might as well skip the middle
man.

Keith> + MI: -break-insert -c 'foo == bar' -t -e -s source.c -f function -l
Keith> label -o offset

Keith> Currently, the explicit linespec flag (-e) must be the last option in
Keith> the command. Everything after "-e" is restricted to explicit
Keith> linespecs. [This restriction was enabled so that we don't have to
Keith> require some really ugly quoting, e.g., -break-insert -e "-f main
Keith> -offset 3" -c "foo == bar".

I don't understand why MI needs a -e option at all.

You can either have the explicit bits parsed directly, by adding options
to 'opts' in mi_cmd_break_insert; or you can just add a new
-break-insert-explicit command.

Keith> + CLI: break -source source.c -function function -label label -offset
Keith> offset -condition "foo == bar" -thread 1
[...]
Keith> When setting an explicit linespec, users may not use the keywords
Keith> "if", "thread", or "task". If using explicit linespecs, *everything*
Keith> must be explicitly specified.

I think quoting any expression here is going to be a pain, but I'm
curious to hear what others think.

What is the reason for this approach?

How does this interact with systemtap probe point specifications?

How hard is it to add support for this to other linespec-using commands?
Also I wonder what the best way to expose this to Python might be; for
example if one wanted to write a Python command that accepted any sort
of linespec.

Keith> + I have introduced several new error messages. They should be
Keith> double-checked:
Keith> o "invalid linespec argument, \"%s\"" : This is output when the
Keith> user attempts to use an invalid explicit linespec flag, e.g.,
Keith> "-foobar".
Keith> o "missing argument for \"%s\" : This is output when the user
Keith> omits an argument for a given explicit linespec flag.

Looks good to me.

Tom
Keith Seitz
2012-06-21 19:08:39 UTC
Permalink
Post by Tom Tromey
I started looking at it, but really I think you should either do the
final polishing (docs and ChangeLog) and submit it; or send an RFC to
agree about everything, it'll still have to go through another round of
kibitzing when you submit it, and so you might as well skip the middle
man.
On reflection I probably should have posted this to gdb-patches as an
RFC. Next time.
Post by Tom Tromey
I don't understand why MI needs a -e option at all.
Me either! :-) I've removed that stoopidity.
Post by Tom Tromey
You can either have the explicit bits parsed directly, by adding options
to 'opts' in mi_cmd_break_insert; or you can just add a new
-break-insert-explicit command.
I've opted to insert the options directly to -break-insert. [Other MI
commands wishing to support this could easily do the same.]
Post by Tom Tromey
Keith> + CLI: break -source source.c -function function -label label -offset
Keith> offset -condition "foo == bar" -thread 1
[...]
Keith> When setting an explicit linespec, users may not use the keywords
Keith> "if", "thread", or "task". If using explicit linespecs, *everything*
Keith> must be explicitly specified.
I think quoting any expression here is going to be a pain, but I'm
curious to hear what others think.
What is the reason for this approach?
The "problem" is parsing the condition/thread/task (btw, task is not
propagated up the create_breakpoint API -- poor ada). It isn't a problem
per se, but I was attempting to avoid the "groking UI input in a backend
API" style that has caused me no end of difficulties over the years
(e.g., find_condition_and_thread). IMO, that is a function of the UI to
parse out those bits, not the internal breakpoint creation API.

IOW, I am/was trying to avoid implementing backend gdb APIs in terms of
the command line.
Post by Tom Tromey
How does this interact with systemtap probe point specifications?
It shouldn't. Explicit linespecs are only accepted for tracepoints and
breakpoints. The explicit-releated members of struct breakpoint_ops for
all other breakpoint types (like probes) is not defined/supported.
Post by Tom Tromey
How hard is it to add support for this to other linespec-using commands?
That all depends on the UI. For the CLI, the relevant bits of code could
be put into a separate function trivially enough. I didn't do that this
round because I was not planning to add this feature to anything more
than break_command. [list_command would be a follow-on feature
addition.] But I'll do that before submitting again.

For MI, it is pretty trivial to implement with its built-in getopt
functionality.
Post by Tom Tromey
Also I wonder what the best way to expose this to Python might be; for
example if one wanted to write a Python command that accepted any sort
of linespec.
This would also be pretty easy, I should think, but I'm only a python
novice. I don't see why the python API couldn't either add explicit
versions of commands (eeew) or accept "function=XXX, label=xxx" tuples
for those commands that accept linespecs, just like MI does (or will do).

You would know better in this case.

Thank you for taking a look.

Keith
Tom Tromey
2012-06-21 20:37:44 UTC
Permalink
Keith> IOW, I am/was trying to avoid implementing backend gdb APIs in terms
Keith> of the command line.

I see. Well, that does make sense. It just seems like it will be hard
to use from the CLI.

Tom> How hard is it to add support for this to other linespec-using
Tom> commands?

Keith> That all depends on the UI. For the CLI, the relevant bits of code
Keith> could be put into a separate function trivially enough. I didn't do
Keith> that this round because I was not planning to add this feature to
Keith> anything more than break_command. [list_command would be a follow-on
Keith> feature addition.] But I'll do that before submitting again.

FWIW I'd rather get the first bits in than delay the patch for a new
feature. It's easy enough to do follow-ups, and they're easier to
review besides.

Tom
Tom Tromey
2012-06-29 13:31:32 UTC
Permalink
Keith> IOW, I am/was trying to avoid implementing backend gdb APIs in terms
Keith> of the command line.

Tom> I see. Well, that does make sense. It just seems like it will be hard
Tom> to use from the CLI.

I've been thinking about this a bit.

It seems to me that making the user interface harder to use for the sake
of implementation cleanliness (without additional factors like
scalability) is the wrong tradeoff.

Tom

Loading...