Re: [PATCH] Validate explicit locations with early termination

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2017-12-07 04:20 PM, Keith Seitz wrote:
> breakpoints/22569 involves an internal error generated by the rather
> innocent looking command:
> 
> (gdb) break -source test.cpp main
> .../linespec.c:3302: internal-error: void decode_line_full(...):
> Assertion `result.size () == 1 || canonical->pre_expanded' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> 
> The input string is tokenized into "-source", "test.cpp", and "main"
> (input parsing breaks on whitespace). create_breakpoint is then called with
> the explicit location (containing only the source file name) and "main" as
> the extra_string argument.
> 
> No SaLs are created for this underspecified explicit location, and the
> "result.size () == 1" evaluates false (as does the pre_expanded condition).
> This triggers the assertion.
> 
> Normally string_to_explicit_location validates the input string.  However,
> the presence of the string "main" causes the parser to exit early:
> 
>    802        else
>    803          {
>    804            /* End of the explicit location specification.
>    805               Stop parsing and return whatever explicit location was
>    806               parsed.  */
>    807            *argp = start;
>    808            return location;
>    809          }
> 
> This bypasses the validation that is done a few lines down in this function
> which would have emitted the expected error.  This patch fixes that.
> 
> Additionally, this patch also fixes an inconsistency with error reporting
> in this use case:
> 
> (gdb) b -source foo
> Source filename requires function, label, or line offset.
> (gdb) b -source foo main
> No source file named foo.
> 
> These two commands should have elicited the same error message.
> 
> gdb/ChangeLog:
> 
> 	PR breakpoints/22569
> 	* location.c (string_to_explicit_location): When terminating
> 	parsing early, break out of enclosing loop instead of returning.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR breakpoints/22569
> 	* gdb.linespec/ls-errs.exp: Change expected result of "break
> 	-source this file has spaces.c -line 3".
> 	Check that an explicit source file followed by whitespace is
> 	identified as an invalid explicit location.

This is quick customer support!  This patch fixes the reported issue
and LGTM.

Thanks!

Simon