This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v6 7/9] Explicit locations: add UI features for CLI
- From: Doug Evans <xdje42 at gmail dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 10 Aug 2015 12:41:45 -0700
- Subject: Re: [PATCH v6 7/9] Explicit locations: add UI features for CLI
- Authentication-results: sourceware.org; auth=none
- References: <20150805232802 dot 21646 dot 88440 dot stgit at valrhona dot uglyboxes dot com> <20150805233245 dot 21646 dot 41251 dot stgit at valrhona dot uglyboxes dot com>
Keith Seitz <keiths@redhat.com> writes:
> Differences in this revision:
>
> 1. Add comma-handling in explicit_location_lex_one. Require whitespace
> in options to be quoted.
> 2. Add tests for above.
> 3. if {0} completer tests that cause internal-error.
> 4. Change explicit_location_completer to use complete_on_enum.
> 5. Updated memory managemenet in string_to_explicit_location.
> 6. Add tests for explicit ranges.
> 7. My regexp fix for the capture_command_output has not been approved
> upstream. It is included here to prevent regressions.
>
> --
>
> This patch exposes explicit locations to the CLI user. This enables
> users to "explicitly" specify attributes of the breakpoint location
> to avoid any ambiguity that might otherwise exist with linespecs.
>
> The general syntax of explicit locations is:
> -source SOURCE_FILENAME -line {+-}LINE -function FUNCTION_NAME
> -label LABEL_NAME
>
> Option names may be abbreviated, e.g., "-s SOURCE_FILENAME -li 3" and users
> may use the completer with either options or values.
>
> gdb/ChangeLog:
>
> * completer.c: Include location.h.
> (enum match_type): New enum.
> (location_completer): Rename to ...
> (linespec_completer): ... this.
> (collect_explicit_location_matches, backup_text_ptr)
> (explicit_location_completer): New functions.
> (location_completer): "New" function; handle linespec
> and explicit location completions.
> (complete_line_internal): Remove all location completer-specific
> handling.
> * linespec.c (linespec_lexer_lex_keyword, is_ada_operator)
> (find_toplevel_char): Export.
> (linespec_parse_line_offset): Export.
> Issue error if STRING is not numerical.
> (gdb_get_linespec_parser_quote_characters): New function.
> * linespec.h (linespec_parse_line_offset): Declare.
> (get_gdb_linespec_parser_quote_characters): Declare.
> (is_ada_operator): Declare.
> (find_toplevel_char): Declare.
> (linespec_lexer_lex_keyword): Declare.
> * location.c (explicit_to_event_location): New function.
> (explicit_location_lex_one): New function.
> (string_to_explicit_location): New function.
> (string_to_event_location): Handle explicit locations.
> * location.h (explicit_to_event_location): Declare.
> (string_to_explicit_location): Declare.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.linespec/3explicit.c: New file.
> * gdb.linespec/cpexplicit.cc: New file.
> * gdb.linespec/cpexplicit.exp: New file.
> * gdb.linespec/explicit.c: New file.
> * gdb.linespec/explicit.exp: New file.
> * gdb.linespec/explicit2.c: New file.
> * gdb.linespec/ls-errs.exp: Add explicit location tests.
> * lib/gdb.exp (capture_command_output): Regexp-escape `command'
> before using in the matching pattern.
> Clarify that `prefix' is a regular expression.
Hi.
Two nits. Ok with those changes.
[no need to resubmit for review]
> ...
> diff --git a/gdb/location.c b/gdb/location.c
> index 7882b2d..1d35b6b 100644
> --- a/gdb/location.c
> +++ b/gdb/location.c
> @@ -442,6 +442,196 @@ event_location_to_string (struct event_location *location)
> return EL_STRING (location);
> }
>
> +/* A lexer for explicit locations. This function will advance INP
> + past any strings that it lexes. Returns a malloc'd copy of the
> + lexed string or NULL if no lexing was done. */
> +
> +static char *
> +explicit_location_lex_one (const char **inp,
> + const struct language_defn *language)
> +{
> + const char *start = *inp;
> +
> + if (*start == '\0')
> + return NULL;
> +
> + /* If quoted, skip to the ending quote. */
> + if (strchr (get_gdb_linespec_parser_quote_characters (), *start))
> + {
> + char quote_char = *start;
> +
> + /* If the input is not an Ada operator, skip to the matching
> + closing quote and return the string. */
> + if (!(language->la_language == language_ada
> + && quote_char == '\"' && is_ada_operator (start)))
> + {
> + const char *end = find_toplevel_char (start + 1, quote_char);
> +
> + if (end == NULL)
> + error (_("Unmatched quote, %s."), start);
> + *inp = end + 1;
> + return savestring (start + 1, *inp - start - 2);
> + }
> + }
> +
> + /* If the input starts with '-' or '+', the string ends with the next
> + whitespace or comma. */
> + if (*start == '-' || *start == '+')
> + {
> + while (*inp[0] != '\0' && *inp[0] != ',' && !isspace (*inp[0]))
> + ++(*inp);
> + }
> + else
> + {
> + /* Handle numbers first, stopping at the next whitespace or ','. */
> + while (isdigit (*inp[0]))
> + ++(*inp);
> + if (*inp[0] == '\0' || isspace (*inp[0]) || *inp[0] == ',')
> + return savestring (start, *inp - start);
> +
> + /* Otherwise stop at the next occurrence of "SPACE -", '\0',
> + keyword, or ','. */
This comment needs updating (right?).
> + *inp = start;
> + while ((*inp)[0]
> + && (*inp)[0] != ','
> + && !(isspace ((*inp)[0])
> + || linespec_lexer_lex_keyword (&(*inp)[1])))
> + {
> + /* Special case: C++ operator,. */
> + if (language->la_language == language_cplus
> + && strncmp (*inp, "operator", 8)
> + && (*inp)[9] == ',')
> + (*inp) += 9;
> + ++(*inp);
> + }
> ...
> diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
> new file mode 100644
> index 0000000..414ec1c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.linespec/explicit.exp
> @@ -0,0 +1,410 @@
> +# Copyright 2012-2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Tests for explicit locations
> +
> +standard_testfile explicit.c explicit2.c 3explicit.c
> +set exefile $testfile
> +
> +if {[prepare_for_testing $testfile $exefile \
> + [list $srcfile $srcfile2 $srcfile3] {debug nowarnings}]} {
> + return -1
> +}
> +
> +# Wrap the entire test in a namespace to avoid contaminating other tests.
> +namespace eval $testfile {
> +
> + # Test the given (explicit) LINESPEC which should cause gdb to break
> + # at LOCATION.
> + proc test_breakpoint {linespec location} {
> +
> + set testname "set breakpoint at \"$linespec\""
> + # Delete all breakpoints, set a new breakpoint at LINESPEC,
> + # and attempt to run to it.
> + delete_breakpoints
> + if {[gdb_breakpoint $linespec]} {
> + pass $testname
> + send_log "\nexpecting locpattern \"$location\"\n"
> + gdb_continue_to_breakpoint $linespec $location
> + } else {
> + fail $testname
> + }
> + }
> +
> + # Add the given LINESPEC to the array named in THEARRAY. GDB is expected
> + # to stop at LOCATION.
> + proc add {thearray linespec location} {
> + upvar $thearray ar
> +
> + lappend ar(linespecs) $linespec
> + lappend ar(locations) $location
> + }
> +
> + # A list of all explicit linespec arguments.
> + variable all_arguments
> + set all_arguments {"source" "function" "label" "line"}
> +
> + # Some locations used in this test
> + variable lineno
> + variable location
> + set lineno(normal) [gdb_get_line_number "myfunction location" $srcfile]
> + set lineno(top) [gdb_get_line_number "top location" $srcfile]
> + foreach v [array names lineno] {
> + set location($v) ".*[string_to_regexp "$srcfile:$lineno($v)"].*"
> + }
> +
> + # A list of explicit locations and the corresponding location.
> + variable linespecs
> + set linespecs(linespecs) {}
> + set linespecs(location) {}
> +
> + add linespecs "-source $srcfile -function myfunction" $location(normal)
> + add linespecs "-source $srcfile -function myfunction -label top" \
> + $location(top)
> +
> + # This isn't implemented yet; -line is silently ignored.
> + add linespecs "-source $srcfile -function myfunction -label top -line 3" \
> + $location(top)
> + add linespecs "-source $srcfile -line $lineno(top)" $location(top)
> + add linespecs "-function myfunction" $location(normal)
> + add linespecs "-function myfunction -label top" $location(top)
> +
> + # These are also not yet supported; -line is silently ignored.
> + add linespecs "-function myfunction -line 3" $location(normal)
> + add linespecs "-function myfunction -label top -line 3" $location(top)
> + add linespecs "-line 3" $location(normal)
> +
> + # Test that static tracepoints on marker ID are not interpreted
> + # as an erroneous explicit option.
> + gdb_test "strace -m gdbfoobarbaz" "You can't do that.*"
> +
> + # Fire up gdb.
> + if {![runto_main]} {
> + return -1
> + }
> +
> + # Turn off queries
> + gdb_test_no_output "set confirm off"
> +
> + # Simple error tests (many more are tested in ls-err.exp)
> + foreach arg $all_arguments {
> + # Test missing argument
> + gdb_test "break -$arg" \
> + [string_to_regexp "missing argument for \"-$arg\""]
> +
> + # Test abbreviations
> + set short [string range $arg 0 3]
> + gdb_test "break -$short" \
> + [string_to_regexp "missing argument for \"-$short\""]
> + }
> +
> + # Test invalid arguments
> + foreach arg {"-foo" "-foo bar" "-function myfunction -foo" \
> + "-function -myfunction -foo bar"} {
> + gdb_test "break $arg" \
> + [string_to_regexp "invalid explicit location argument, \"-foo\""]
> + }
> +
> + # Test explicit locations, with and without conditions.
> + # For these tests, it is easiest to turn of pending breakpoint.
> + gdb_test_no_output "set breakpoint pending off" \
> + "turn off pending breakpoints"
> +
> + foreach linespec $linespecs(linespecs) loc_pattern $linespecs(locations) {
> +
> + # Test the linespec
> + test_breakpoint $linespec $loc_pattern
> +
> + # Test with a valid condition
> + delete_breakpoints
> + set tst "set breakpoint at \"$linespec\" with valid condition"
> + if {[gdb_breakpoint "$linespec if arg == 0"]} {
> + pass $tst
> +
> + gdb_test "info break" ".*stop only if arg == 0.*" \
> + "info break of conditional breakpoint at \"$linespec\""
> + } else {
> + fail $tst
> + }
> +
> + # Test with invalid condition
> + gdb_test "break $linespec if foofoofoo == 1" \
> + ".*No symbol \"foofoofoo\" in current context.*" \
> + "set breakpoint at \"$linespec\" with invalid condition"
> +
> + # Test with thread
> + delete_breakpoints
> + gdb_test "break $linespec thread 123" "Unknown thread 123."
> + }
> +
> + # Test the explicit location completer
> + foreach abbrev {"fun" "so" "lab" "li"} full {"function" "source" "label" "line"} {
> + set tst "complete 'break -$abbrev'"
> + send_gdb "break -${abbrev}\t"
> + gdb_test_multiple "" $tst {
> + "break -$full " {
> + send_gdb "\n"
> + gdb_test_multiple "" $tst {
> + -re "missing argument for \"-$full\".*$gdb_prompt " {
> + pass $tst
> + }
> + }
> + }
> + }
> + set tst "complete -$full with no value"
> + send_gdb "break -$full \t"
> + gdb_test_multiple "" $tst {
> + -re ".*break -$full " {
> + send_gdb "\n"
> + gdb_test_multiple "" $tst {
> + -re ".*Source filename requires function, label, or line offset\..*$gdb_prompt " {
> + if {[string equal $full "source"]} {
> + pass $tst
> + } else {
> + faill $tst
> + }
> + }
> + -re "missing argument for \"-$full\".*$gdb_prompt " {
> + pass $tst
> + }
> + }
> + }
> + }
> + }
> +
> + set tst "complete unique function name"
> + send_gdb "break -function mai\t"
> + gdb_test_multiple "" $tst {
> + "break -function mai\\\x07n" {
> + send_gdb "\n"
> + gdb_test "" ".*Breakpoint \[0-9\]+.*" $tst
> + gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
> + }
> + }
> +
> + set tst "complete non-unique function name"
> + send_gdb "break -function myfunc\t"
> + gdb_test_multiple "" $tst {
> + "break -function myfunc\\\x07tion" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + -re "\\\x07\r\nmyfunction\[ \t\]+myfunction2\[ \t\]+myfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
> + gdb_test "2" ".*Breakpoint \[0-9\]+.*" $tst
> + gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
> + }
> + }
> + }
> + }
> +
> + set tst "complete non-existant function name"
> + send_gdb "break -function foo\t"
> + gdb_test_multiple "" $tst {
> + "break -function foo\\\x07" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + -re "\\\x07\\\x07" {
> + send_gdb "\n"
> + gdb_test "" {Function "foo" not defined.} $tst
> + }
> + }
> + }
> + }
> +
> + set tst "complete unique file name"
> + send_gdb "break -source 3ex\t"
> + gdb_test_multiple "" $tst {
> + "break -source 3explicit.c " {
> + send_gdb "\n"
> + gdb_test "" \
> + {Source filename requires function, label, or line offset.} $tst
> + }
> + }
> +
> + set tst "complete non-unique file name"
> + send_gdb "break -source exp\t"
> + gdb_test_multiple "" $tst {
> + "break -source exp\\\x07licit" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + -re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+\r\n$gdb_prompt" {
> + send_gdb "\n"
> + gdb_test "" \
> + {Source filename requires function, label, or line offset.} \
> + $tst
> + }
> + }
> + }
> +
> + "break -source exp\\\x07l" {
> + # This pattern may occur when glibc debuginfo is installed.
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + -re "\\\x07\r\nexplicit.c\[ \t\]+explicit2.c\[ \t\]+expl.*\r\n$gdb_prompt" {
> + send_gdb "\n"
> + gdb_test "" \
> + {Source filename requires function, label, or line offset.} \
> + $tst
> + }
> + }
> + }
> + }
> +
> + set tst "complete non-existant file name"
> + send_gdb "break -source foo\t"
> + gdb_test_multiple "" $tst {
> + "break -source foo" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + "\\\x07\\\x07" {
> + send_gdb "\n"
> + gdb_test "" \
> + {Source filename requires function, label, or line offset.} \
> + $tst
> + }
> + }
> + }
> + }
> +
> + # These tests are disabled pending resolution of gdb/17960.
> + # They cannot be XFAILed because they cause an internal-error.
> + if {0} {
> + set tst "complete filename and unique function name"
> + send_gdb "break -source explicit.c -function ma\t"
> + gdb_test_multiple "" $tst {
> + "break -source explicit.c -function main " {
> + send_gdb "\n"
> + gdb_test "" ".*Breakpoint .*" $tst
> + gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
> + }
> + }
> +
> + set tst "complete filename and non-unique function name"
> + send_gdb "break -so 3explicit.c -func myfunc\t"
> + gdb_test_multiple "" $tst {
> + "break -so 3explicit.c -func myfunc\\\x07tion" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + -re "\\\x07\r\nmyfunction3\[ \t\]+myfunction4\[ \t\]+\r\n$gdb_prompt " {
> + gdb_test "3" ".*Breakpoint \[0-9\]+.*" $tst
> + gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
> + }
> + }
> + }
> + }
> +
> + set tst "complete filename and non-existant function name"
> + send_gdb "break -sou 3explicit.c -fun foo\t"
> + gdb_test_multiple "" $tst {
> + "break -sou 3explicit.c -fun foo\\\x07" {
> + send_gdb "\t\t"
> + gdb_test_multiple "" $tst {
> + "\\\x07\\\x07" {
> + send_gdb "\n"
> + gdb_test "" \
> + {Function "foo" not defined in "3explicit.c".} $tst
> + }
> + }
> + }
> + }
> +
> + set tst "complete filename and function reversed"
> + send_gdb "break -func myfunction4 -source 3ex\t"
> + gdb_test_multiple "" $tst {
> + "break -func myfunction4 -source 3explicit.c " {
> + send_gdb "\n"
> + gdb_test "" "Breakpoint \[0-9\]+.*" $tst
> + gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"
> + }
> + }
> + }
I have checked in the temp fix for 17960 so the "if {0}" can be removed.
Thanks!