From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57844 invoked by alias); 10 Aug 2015 19:42:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 57834 invoked by uid 89); 10 Aug 2015 19:42:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_05,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f51.google.com Received: from mail-pa0-f51.google.com (HELO mail-pa0-f51.google.com) (209.85.220.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 10 Aug 2015 19:42:43 +0000 Received: by pawu10 with SMTP id u10so146690397paw.1 for ; Mon, 10 Aug 2015 12:42:41 -0700 (PDT) X-Received: by 10.68.109.34 with SMTP id hp2mr47014254pbb.52.1439235761259; Mon, 10 Aug 2015 12:42:41 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by smtp.gmail.com with ESMTPSA id ts1sm20831339pbc.74.2015.08.10.12.42.39 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Aug 2015 12:42:40 -0700 (PDT) From: Doug Evans To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v6 7/9] Explicit locations: add UI features for CLI References: <20150805232802.21646.88440.stgit@valrhona.uglyboxes.com> <20150805233245.21646.41251.stgit@valrhona.uglyboxes.com> Date: Mon, 10 Aug 2015 19:42:00 -0000 In-Reply-To: <20150805233245.21646.41251.stgit@valrhona.uglyboxes.com> (Keith Seitz's message of "Wed, 05 Aug 2015 16:33:16 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00226.txt.bz2 Keith Seitz 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 . > + > +# 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!