diff options
author | Andrew Burgess <aburgess@redhat.com> | 2021-12-07 14:01:23 +0000 |
---|---|---|
committer | Andrew Burgess <aburgess@redhat.com> | 2022-02-02 16:27:36 +0000 |
commit | 3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6 (patch) | |
tree | dfdaf5edb8dbd051f0c4700f689479bb0321e07b | |
parent | ed2a947856f959a9c52c82a67f781df9909187a8 (diff) | |
download | gdb-3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6.zip gdb-3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6.tar.gz gdb-3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6.tar.bz2 |
gdb: handle calls to list command passing only a linespec condition
In PR cli/28665, it was reported that GDB would crash when given a
command like:
(gdb) list task 123
The problem here is that in cli/cli-cmd.c:list_command, the string
'task 123' is passed to string_to_event_location in find a location
specification. However, this location parsing understands about
breakpoint conditions, and so, will stop parsing when it sees
something that looks like a condition, in this case, the 'task 123'
looks like a breakpoint condition.
As a result, the location we get back from string_to_event_location
has no actual location specification attached to it. The actual call
path is:
list_command
string_to_event_location
string_to_event_location_basic
new_linespec_location
In new_linespec_location we call linespec_lex_to_end, which looks at
'task 123' and decides that there's nothing there that describes a
location. As such, in new_linespec_location, the spec_string field of
the location is left as nullptr.
Back in list_command we then call decode_line_1, which calls
event_location_to_sals, which calls parse_linespec, which takes the
spec_string we found earlier, and tries to converts this into a list
of sals.
However, parse_linespec is not intended to be passed a nullptr, for
example, calling is_ada_operator will try to access through the
nullptr, causing undefined behaviour. But there are other cases
within parse_linespec which don't expect to see a nullptr.
When looking at how to fix this issue, I first considered having
linespec_lex_to_end detect the problem. That function understands
when the first thing in the linespec is a condition keyword, and so,
could throw an error saying something like: "no linespec before
condition keyword", however, this is not going to work, at least, not
without additional changes to GDB, it is valid to place a breakpoint
like:
(gdb) break task 123
This will place a breakpoint at the current location with the
condition 'task 123', and changing linespec_lex_to_end breaks this
behaviour.
So, next, I considered what would happen if I added a condition to an
otherwise valid list command, this is what I see:
(gdb) list file.c:1 task 123
Junk at end of line specification.
(gdb)
So, then I wondered, could we just pull the "Junk" detection forward,
so that we throw the error earlier, before we call decode_line_1?
It turns out that yes we can. Well, sort of.
It is simpler, I think, to add a separate check into the list_command
function, after calling string_to_event_location, but before calling
decode_line_1. We know when we call string_to_event_location that the
string in question is not empty, so, after calling
string_to_event_location, if non of the string has been consumed, then
the content of the string must be junk - it clearly doesn't look like
a location specification.
I've reused the same "Junk at end of line specification." error for
consistency, and added a few tests to cover this issue.
While the first version of this patch was on the mailing list, a
second bug PR gdb/28797 was raised. This was for a very similar
issue, but this time the problem command was:
(gdb) list ,,
Here the list command understands about the first comma, list can have
two arguments separated by a comma, and the first argument can be
missing. So we end up trying to parse the second command "," as a
linespec.
However, in linespec_lex_to_end, we will stop parsing a linespec at a
comma, so, in the above case we end up with an empty linespec (between
the two commas), and, like above, this results in the spec_string
being nullptr.
As with the previous case, I've resolved this issue by adding an extra
check for junk at the end of the line - after parsing (or failing to
parse) the nothing between the two commas, we still have the "," left
at the end of the list command line - when we see this we can throw
the same "junk at the end of the line" error, and all is good.
I've added tests for this case too.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28797
-rw-r--r-- | gdb/cli/cli-cmds.c | 12 | ||||
-rw-r--r-- | gdb/testsuite/gdb.linespec/errors.exp | 12 |
2 files changed, 24 insertions, 0 deletions
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 2f5ce3e..648005f 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1238,6 +1238,15 @@ list_command (const char *arg, int from_tty) { event_location_up location = string_to_event_location (&arg1, current_language); + + /* We know that the ARG string is not empty, yet the attempt to parse + a location from the string consumed no characters. This most + likely means that the first thing in ARG looks like a location + condition, and so the string_to_event_location call stopped + parsing. */ + if (arg1 == arg) + error (_("Junk at end of line specification.")); + sals = decode_line_1 (location.get (), DECODE_LINE_LIST_MODE, NULL, NULL, 0); filter_sals (sals); @@ -1286,6 +1295,9 @@ list_command (const char *arg, int from_tty) event_location_up location = string_to_event_location (&arg1, current_language); + if (*arg1) + error (_("Junk at end of line specification.")); + std::vector<symtab_and_line> sals_end = (dummy_beg ? decode_line_1 (location.get (), DECODE_LINE_LIST_MODE, diff --git a/gdb/testsuite/gdb.linespec/errors.exp b/gdb/testsuite/gdb.linespec/errors.exp index 0baef18..e258f6b 100644 --- a/gdb/testsuite/gdb.linespec/errors.exp +++ b/gdb/testsuite/gdb.linespec/errors.exp @@ -27,3 +27,15 @@ gdb_test "list c:/foo/bar/baz.c:1" "No source file named c:/foo/bar/baz.c." gdb_test "list c:/foo/bar/baz.c" "Function \"c:/foo/bar/baz.c\" not defined." gdb_test "list fooc:/foo/bar/baz.c:1" "No source file named fooc." gdb_test "list fooc:/foo/bar/baz.c" "No source file named fooc." + +# PR cli/28665, gdb/28797 +gdb_test "list task 123" \ + "Junk at end of line specification\\." +gdb_test "list if (0)" \ + "Junk at end of line specification\\." +gdb_test "list thread 1" \ + "Junk at end of line specification\\." +gdb_test "list -force-condition" \ + "Junk at end of line specification\\." +gdb_test "list ,," \ + "Junk at end of line specification\\." |