* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
@ 2021-12-07 12:17 ` aburgess at redhat dot com
2021-12-07 16:03 ` simark at simark dot ca
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: aburgess at redhat dot com @ 2021-12-07 12:17 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Andrew Burgess <aburgess at redhat dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2021-12-07
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
CC| |aburgess at redhat dot com
--- Comment #1 from Andrew Burgess <aburgess at redhat dot com> ---
The `list task 123` problem is a result of "task" being a linespec keyword.
GDB parses the linespec 'task 123' and stops when it sees the keyword 'task'.
As a result the event_location::u::linespec_location::spec_string is left as
nullptr.
Later in event_location_to_sals, the spec_string is passed to parse_linespec,
which doesn't check for the incoming string being nullptr.
The error will reproduce for any location that starts with a keyword, e.g.
(gdb) list if (1)
**crash**
(gdb) list thread 1
**crash**
(gdb) list -force-condition
**crash**
This seems to be a long standing issue with the location/linespec parsing, its
present at least as far back as the gdb 8.x series.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
2021-12-07 12:17 ` [Bug cli/28665] " aburgess at redhat dot com
@ 2021-12-07 16:03 ` simark at simark dot ca
2021-12-07 23:25 ` aburgess at redhat dot com
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: simark at simark dot ca @ 2021-12-07 16:03 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Simon Marchi <simark at simark dot ca> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |simark at simark dot ca
--- Comment #2 from Simon Marchi <simark at simark dot ca> ---
(In reply to Itay Va from comment #0)
> Hey , I found a command that crashes gdb and decided to fuzz for more.
Nice, please keep doing this, we need more such testing :)
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
2021-12-07 12:17 ` [Bug cli/28665] " aburgess at redhat dot com
2021-12-07 16:03 ` simark at simark dot ca
@ 2021-12-07 23:25 ` aburgess at redhat dot com
2022-02-02 16:27 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: aburgess at redhat dot com @ 2021-12-07 23:25 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--- Comment #3 from Andrew Burgess <aburgess at redhat dot com> ---
I posted a possible fix:
https://sourceware.org/pipermail/gdb-patches/2021-December/184314.html
Please excuse that email mentioning a different bug number, that was just a
result of me copying the URL from the wrong browser tab.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
` (2 preceding siblings ...)
2021-12-07 23:25 ` aburgess at redhat dot com
@ 2022-02-02 16:27 ` cvs-commit at gcc dot gnu.org
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-02 16:27 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e454b9c61b6d3a80ea4bc840e808e1564d94ec7
commit 8e454b9c61b6d3a80ea4bc840e808e1564d94ec7
Author: Andrew Burgess <aburgess@redhat.com>
Date: Tue Dec 7 13:25:47 2021 +0000
gdb: add empty string check in parse_linespec
If parse_linespec (linespec.c) is passed ARG as an empty string then
we end up calling `strchr (linespec_quote_characters, '\0')`, which
will return a pointer to the '\0' at the end of
linespec_quote_characters. This then results in GDB calling
skip_quote_char with `ARG + 1`, which is undefined behaviour (as ARG
only contained a single character, the '\0').
Fix this by checking for the first character of ARG being '\0' before
the call to strchr.
I have additionally added an assertion that ARG can't itself be
nullptr, as calling is_ada_operator with nullptr can end up calling
'startswith' on the nullptr, which is undefined behaviour.
Finally, I moved the declaration of TOKEN into the body of
parse_linespec, to where TOKEN is defined.
This patch came about while I was working on fixes for PR cli/28665
and PR gdb/28797. The actual fixes for these two issues will be in a
later commit in this series, but, with this patch in place, both of
the above bugs would hit the new assertion rather than accessing
invalid memory and crashing. The '\0' check is not currently ever
hit, but just makes the code a little safer.
Because this patch only changes the nature of the failure for the
above two bugs, there's no tests here. A later commit will fix the
above two issues, at which point I'll add some tests.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28797
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
` (3 preceding siblings ...)
2022-02-02 16:27 ` cvs-commit at gcc dot gnu.org
@ 2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-02 16:28 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6
commit 3c5fcec6dccb0e598d1e64640e55d50ed3ddedb6
Author: Andrew Burgess <aburgess@redhat.com>
Date: Tue Dec 7 14:01:23 2021 +0000
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
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
` (4 preceding siblings ...)
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
@ 2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
2022-02-02 16:30 ` aburgess at redhat dot com
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-02 16:28 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=82d0a72cdc9ca6cd37a8987e2bcd2eb707c51149
commit 82d0a72cdc9ca6cd37a8987e2bcd2eb707c51149
Author: Andrew Burgess <aburgess@redhat.com>
Date: Tue Dec 7 22:26:05 2021 +0000
gdb: handle calls to edit command passing only a linespec condition
While working on the previous commit to fix PR cli/28665, I noticed
that the 'edit' command would suffer from the same problem. That is,
something like:
(gdb) edit task 123
would cause GDB to break. For a full explanation of what's going on
here, see the commit message for the previous commit.
As with the previous commit, this issue can be prevented by detecting,
and throwing, a junk at the end of the line error earlier, before
calling decode_line_1.
So, that's what this commit does. I've also added some tests for this
issue.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
` (5 preceding siblings ...)
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
@ 2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
2022-02-02 16:30 ` aburgess at redhat dot com
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-02 16:28 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a591084285cc16a435258641691aa0a8f5bf42ee
commit a591084285cc16a435258641691aa0a8f5bf42ee
Author: Andrew Burgess <aburgess@redhat.com>
Date: Thu Jan 27 15:12:25 2022 +0000
gdb: test to check one aspect of the linespec parsing code
While working on the fix for PR cli/28665 (see previous couple of
commits), I was playing with making a change in the linespec parsing
code. Specifically, I was thinking about whether the spec_string for
LINESPEC_LOCATION locations should ever be nullptr.
I made a change to prevent the spec_string from ever being nullptr,
tested gdb, and saw no regressions.
However, as part of this work I was reviewing how the breakpoint code
handles this case (spec_string being nullptr), and spotted that in
parse_breakpoint_sals the nullptr case is specifically handled, so
changing this should have caused a regression. But I didn't see one.
So, this commit adds a comment in location.c mentioning that the
nullptr case is (a) not an oversight, and (b) is required. Then I add
a new test to gdb.base/break.exp that ensures a change in this area
will cause a regression.
This test passes on current gdb, but with my modified (and broken)
gdb, the test would fail.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread* [Bug cli/28665] CLI commands that cause crash
2021-12-07 11:01 [Bug cli/28665] New: CLI commands that cause crash gosebam at gmail dot com
` (6 preceding siblings ...)
2022-02-02 16:28 ` cvs-commit at gcc dot gnu.org
@ 2022-02-02 16:30 ` aburgess at redhat dot com
7 siblings, 0 replies; 9+ messages in thread
From: aburgess at redhat dot com @ 2022-02-02 16:30 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Andrew Burgess <aburgess at redhat dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #8 from Andrew Burgess <aburgess at redhat dot com> ---
I believe this issue is now fixed. If you still see any failures in this area,
please feel free to reopen the bug.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread