[Bug gdb/28797] New: Aborted (core dumped) when `l ,,`

[Bug gdb/28797] New: Aborted (core dumped) when `l ,,`

[wuzy01 at qq dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

            Bug ID: 28797
           Summary: Aborted (core dumped) when `l ,,`
           Product: gdb
           Version: 11.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: wuzy01 at qq dot com
  Target Milestone: ---

I found when I press `list ,,` by mistake (the correct command should be `list
,N`, and N is a line number), gdb will `Aborted (core dumped)`. So I would
like to create a bug for gdb.

Environment information is as following:

```shell
$ uname -r
5.15.11-arch2-1
$ pacman -Qi gdb|rg Version
Version         : 11.1-4
```

This C file can be a test file.

```c
#include <stdio.h>
int main(int argc, char *argv[])
{
        int a = 1;
        int b = 2;
        a = a + b;
        printf("%s\n");
        return 0;
}
```

```
gcc -g main.c -o main
gdb main
```

Then press `l ,,`. gdb will `Aborted (core dumped)`

Thanks.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28797] Aborted (core dumped) when `l ,,`

[wuzy01 at qq dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

wuzy01 <wuzy01 at qq dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wuzy01 at qq dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28797] Aborted (core dumped) when `l ,,`

[aburgess at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |aburgess at redhat dot com
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-01-27

--- Comment #1 from Andrew Burgess <aburgess at redhat dot com> ---
This is related, but not an exact duplicate of this bug:
  https://sourceware.org/bugzilla/show_bug.cgi?id=28665

for which I posted some patches here:
  https://sourceware.org/pipermail/gdb-patches/2021-December/184314.html

unfortunately, those patches don't fix this issue, but they do change the
failure mode from an abort, to an assertion failure.

I'll take a look and update that series to cover this issue too.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28797] Aborted (core dumped) when `l ,,`

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

--- Comment #2 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.

[Bug gdb/28797] Aborted (core dumped) when `l ,,`

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

--- Comment #3 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.

[Bug gdb/28797] Aborted (core dumped) when `l ,,`

[aburgess at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28797

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Andrew Burgess <aburgess at redhat dot com> ---
I believe this issue is now fixed.  If you are still seeing 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.