From patchwork Wed Sep 3 19:32:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 2643 Received: (qmail 7657 invoked by alias); 3 Sep 2014 19:33:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7434 invoked by uid 89); 3 Sep 2014 19:33:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, KAM_ADVERT2, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 03 Sep 2014 19:33:02 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s83JWj9n030672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 3 Sep 2014 15:32:45 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s83JWisi003291 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 3 Sep 2014 15:32:44 -0400 Message-ID: <54076CDC.4030205@redhat.com> Date: Wed, 03 Sep 2014 12:32:44 -0700 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Doug Evans CC: "gdb-patches@sourceware.org ml" Subject: Re: [RFA 7/9] Explicit locations v2 - CLI for explicit locations References: <536BC707.4030300@redhat.com> <5388CBB0.4040406@redhat.com> <21473.45879.780609.430872@ruffy.mtv.corp.google.com> In-Reply-To: <21473.45879.780609.430872@ruffy.mtv.corp.google.com> X-IsSubscribed: yes On 08/05/2014 09:46 PM, Doug Evans wrote: > > +/* A helper function to collect explicit location matches for the given > > + LOCATION, which is attempting to match on WHICH. */ > > s/WHICH/WORD/ ? :-) Fixed. > > > + > > +static VEC (char_ptr) * > > +collect_explicit_location_matches (struct event_location *location, > > + enum explicit_location_match_type what, > > + const char *word) > > +{ > > + VEC (char_ptr) *matches = NULL; > > + struct explicit_location *explicit = EVENT_LOCATION_EXPLICIT (location); > > + > > + switch (what) > > + { > > + case MATCH_SOURCE: > > + { > > + const char *text = (explicit->source_filename == NULL > > + ? "" : explicit->source_filename); > > + > > + matches = make_source_files_completion_list (text, word); > > + } > > + break; > > + > > + case MATCH_FUNCTION: > > + { > > + const char *text = (explicit->function_name == NULL > > + ? "" : explicit->function_name); > > + > > + if (explicit->source_filename != NULL) > > + matches > > + = make_file_symbol_completion_list (text, word, > > + explicit->source_filename); > > Wrap if/else/etc clauses longer than one line in { }. Done. > > + else > > + matches = make_symbol_completion_list (text, word); > > + } > > + break; > > + > > + case MATCH_LABEL: > > + /* Not supported. */ > > + break; > > + > > + default: > > + gdb_assert_not_reached (_("unhandled which_explicit")); > > "unhandled explicit_location_match_type" ? Better. Fixed. > > > + } > > + > > + return matches; > > +} > > + > > +/* A convenience macro to (safely) back up P to the previous > > + word. */ > > + > > +#define BACKUP(P,S) \ > > I'd prefer this be a function. Done. > > +static VEC (char_ptr) * > > +explicit_location_completer (struct cmd_list_element *ignore, > > + struct event_location *location, > > + const char *text, const char *word) > > +{ > > + const char *p; > > + VEC (char_ptr) *matches = NULL; > > + > > + /* Find the beginning of the word. This is necessary because > > + we need to know if we are completing an option name or value. We > > + don't get the leading '-' from the completer. */ > > + p = word; > > + BACKUP (p, text); > > + > > + if (*p == '-') > > + { > > + size_t len = strlen (word); > > + > > + /* Completing on option name. */ > > + if (strncmp (word, "source", len) == 0) > > + VEC_safe_push (char_ptr, matches, xstrdup ("source")); > > + else if (strncmp (word, "function", len) == 0) > > + VEC_safe_push (char_ptr, matches, xstrdup ("function")); > > + else if (*word == 'l') > > + { > > + if (strncmp (word, "line", len) == 0) > > + VEC_safe_push (char_ptr, matches, xstrdup ("line")); > > + if (strncmp (word, "label", len) == 0) > > + VEC_safe_push (char_ptr, matches, xstrdup ("label")); > > + } > > Keeping this style (special casing multiple words that start with > the same letter) over time adds a bit more complexity than necessary. > Although "suboptimal" a series of if's and no else's would be simpler. Done. > btw, What happens if one tries to complete on "-li"? Um.. It works? :-) > Where does "word" point in this case? > [Should all the strncmp's use p+1 instead of word?] Yes, indeed it should! I've adjusted the patch. [snip] > > + /* Now gather matches */ > > + matches = collect_explicit_location_matches (location, what, new_word); > > + } > > + > > + return matches; > > +} > > I didn't parse this logic with a fine-toothed comb. > I'm assuming it's correct (for now at least). "I find your lack of faith disturbing." [obligatory Star Wars sarcasm] Believe me when I say that I hope it is correct, too. I don't need more bugs to fix! > > + > > +/* A completer for locations. */ > > + > > +VEC (char_ptr) * > > +location_completer (struct cmd_list_element *ignore, > > + const char *text, const char *word) > > +{ > > + VEC (char_ptr) *matches = NULL; > > + const char *copy = text; > > + struct event_location *location; > > + > > + location = string_to_explicit_location (©, current_language, 1); > > + if (location != NULL) > > + { > > + matches = explicit_location_completer (ignore, location, text, word); > > + delete_event_location (location); > > Do we know if there are absolutely zero occasions in which > explicit_location_completer can throw an exception? > Or do we need to free location via a cleanup? Probably safer to use a cleanup anyway. Fixed. > > @@ -668,16 +847,6 @@ complete_line_internal (const char *text, > > rl_completer_word_break_characters = > > gdb_completer_file_name_break_characters; > > } > > - else if (c->completer == location_completer) > > - { > > - /* Commands which complete on locations want to > > - see the entire argument. */ > > - for (p = word; > > - p > tmp_command > > - && p[-1] != ' ' && p[-1] != '\t'; > > - p--) > > - ; > > - } > > Removing this code is sure nice to see. :-) I didn't see the need for this kinda of specialization. IMO, it should have gone into the completer function anyway. > > @@ -379,7 +377,7 @@ linespec_lexer_lex_number (linespec_parser *parser, linespec_token *tokenp) > > /* Does P represent one of the keywords? If so, return > > the keyword. If not, return NULL. */ > > > > -static const char * > > +const char * > > linespec_lexer_lex_keyword (const char *p) > > { > > int i; > > @@ -405,7 +403,7 @@ linespec_lexer_lex_keyword (const char *p) > > /* Does STRING represent an Ada operator? If so, return the length > > of the decoded operator name. If not, return 0. */ > > Duplicate function comment with what's in linespec.h. > [Probably others as well. Either remove the comment from linespec.h > or replace this one with "See linespec.h." > I know current thinking is to put the comments in the header, > but I don't insist on it.] I've fixed all of these, adding "See description in foo.h." > > @@ -1547,6 +1546,9 @@ linespec_parse_line_offset (const char *string) > > ++string; > > } > > > > + if (!isdigit (*string)) > > + error (_("malformed line offset: \"%s\""), start); > > + > > This is independent of this patch, right? > If so, PLEASE don't move it to another patch on my account. > Just checking. > [For this patch I don't want to be too pedantic!] Not strictly, no. It is presently not possible to trigger this error via the current linespec parser. > > diff --git a/gdb/location.c b/gdb/location.c > > index 86d4232..cfcbc46 100644 > > --- a/gdb/location.c > > +++ b/gdb/location.c > > @@ -379,6 +379,214 @@ event_location_to_string (struct event_location *location) > > return location->as_string; > > } > > > > +/* 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') > > This is one big "if" that consumes most of the function. > Seems like it would simplify things to just early exit if *start == '\0'. Done. > > > + { > > + /* 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); > > extra space after , Fixed. > > + > > +/* Attempt to convert the input string in *ARGP into an explicit location. > > + ARGP is advanced past any processed input. Returns an event_location > > *ARGP is advanced past ... > Fixed. > > + (malloc'd) if an explicit location was successfully found in *ARGP, > > + NULL otherwise. > > + > > + IF !DONT_THROW, this function may call error() if *ARGP looks like > > + properly formed input, e.g., if it is called with missing argument > > + parameters or invalid options. If DONT_THROW is non-zero, this function > > extra space after invalid > Fixed. > > + will not throw any exceptions. */ > > + > > +struct event_location * > > +string_to_explicit_location (const char **argp, > > + const struct language_defn *language, > > + int dont_throw) > > +{ > > + /* It is assumed that input beginning with '-' and a non-digit > > + character is an explicit location. */ > > + if (argp != NULL && *argp != NULL) > > The function comment doesn't indicate that argp can be NULL. > Can it? > If it can be NULL I'd just early exit. Right, argp cannot be NULL, but there's no harm guarding against it. Likewise *argp cannot be NULL (for an explicit location). Only linespec locations may do that. I've added an early exit for this. > Also, this is another large "if" that covers most of > the function. I rarely find these easier to read than > those that early exit and remove the extra level of > indentation (especially as the size of the function grows). > > > + { > > + const char *copy, *orig; > > + > > + orig = copy = *argp; > > + if ((*argp[0] == '-' && isalpha ((*argp)[1]))) > > Is this another early exit opportunity? Done. > > + { > > + char *s, *str; > > + struct cleanup *cleanup; > > + struct event_location *location; > > + > > + location = new_explicit_location (NULL); > > + cleanup = make_cleanup_delete_event_location (location); > > + > > + /* Process option/argument pairs. dprintf_command > > + requires that processing stop on ','. */ > > + while ((*argp)[0] != '\0' && (*argp)[0] != ',') > > + { > > + int len; > > + char *opt, *oarg; > > + const char *start; > > + struct cleanup *inner; > > + > > + /* If *ARGP starts with a keyword, stop processing > > + options. */ > > + if (linespec_lexer_lex_keyword (*argp) != NULL) > > + break; > > + > > + /* Mark the start of the string in case we need to rewind. */ > > + start = *argp; > > + > > + /* Get the option string. */ > > + opt = explicit_location_lex_one (argp, language); > > + inner = make_cleanup (xfree, opt); > > + > > + /* Skip any whitespace. */ > > One can argue this comment and the one below (and any others) > are unnecessary. In this particular case I don't mind them, > but I'm sure others will so might as well delete them. Done. > > + *argp = skip_spaces_const (*argp); > > + > > + /* Get the argument string. */ > > + oarg = explicit_location_lex_one (argp, language); > > + > > + /* Skip any whitespace. */ > > + *argp = skip_spaces_const (*argp); > > + > > + /* Use the length of the option to allow abbreviations. */ > > + len = strlen (opt); > > Is "-" by itself flagged as an error? > [e.g., if the user (mis)typed "- source mumble"] It is not an error, no, but it is not a valid explicit location. "-" by itself (as a linespec) *is* valid (or at least it is currently). I've added a test for this [and fixed a bug]. > > > + > > + /* All options have a required argument. */ > > It would be good to add a comment here saying the test to > ensure oarg is non-NULL is done later. > The reader at this point will be wondering why we're assigning > oarg which may be NULL even though it's a required argument. Done. > > + if (strncmp (opt, "-source", len) == 0) > > + EVENT_LOCATION_EXPLICIT (location)->source_filename = oarg; > > + else if (strncmp (opt, "-function", len) == 0) > > + EVENT_LOCATION_EXPLICIT (location)->function_name = oarg; > > + else if (strncmp (opt, "-line", len) == 0) > > + { > > + if (oarg != NULL) > > + { > > + volatile struct gdb_exception e; > > + > > + TRY_CATCH (e, RETURN_MASK_ERROR) > > + { > > + EVENT_LOCATION_EXPLICIT (location)->line_offset > > + = linespec_parse_line_offset (oarg); > > + } > > + > > + xfree (oarg); > > + if (e.reason < 0 && !dont_throw) > > + throw_exception (e); > > + } > > + } > > + else if (strncmp (opt, "-label", len) == 0) > > + EVENT_LOCATION_EXPLICIT (location)->label_name = oarg; > > + /* Only emit an "invalid argument" error for options > > + that look like option strings. */ > > + else if (opt[0] == '-' && !isdigit (opt[1])) > > + { > > + xfree (oarg); > > + if (!dont_throw) > > + { > > + error (_("invalid explicit location argument, \"%s\""), > > + opt); > > + } > > + } > > + else > > + { > > Still need to xfree (oarg) here? Yeah, there was a little mix-up. Fixed. > > > + /* Trailing garbage. This will be handled by > > + one of the callers. */ > > The caller still gets a location object, and yet we've rewound. > This is a bit confusing. Can you clarify? > After more testing, I have decided to handle this slightly differently. In this case, we reset argp (to where it was before we attempt to find the current option/value pair) and immediately return the found location (if any). This now passes all my tests, and I think it will make more sense. > Also, do we need to do_cleanups (inner) before breaking out > of the while loop? Yes. > > @@ -414,10 +622,24 @@ string_to_event_location (char **stringp, > > } > > else > > { > > - /* Everything else is a linespec. */ > > - location = new_linespec_location (*stringp); > > - if (*stringp != NULL) > > - *stringp += strlen (*stringp); > > + const char *arg, *orig; > > + > > + /* Next, try an explicit location. */ > > + orig = arg = *stringp; > > + location = string_to_explicit_location (&arg, language, 0); > > + if (location != NULL) > > + { > > + /* It was a valid explicit location. Advance STRINGP to > > + the end of input. */ > > + *stringp += arg - orig; > > + } > > + else > > + { > > + /* Everything else is a linespec. */ > > + location = new_linespec_location (*stringp); > > + if (*stringp != NULL) > > + *stringp += strlen (*stringp); > > Having just read that dprintf requires explicit location parsing > to stop on ",", it's odd to see linespecs consume the entire line. > IOW, the different handling between the above "*stringp += arg - orig" > and this "*stringp += strlen (*stringp)" is confusing. > Can you add a clarifying comment to "*stringp += strlen (*stringp)" > explaining why it's done that way? I've added a simple comment explaining that we need to "let the linespec parser sort it out." While almost a comical comment, it is true. Something could be done to clean this up, but I'd rather not add to what is already an enormously complex patchset. But say the word, and I will attempt it. > > @@ -215,4 +215,19 @@ extern struct event_location * > > > > extern int event_location_empty_p (const struct event_location *location); > > > > +/* Attempt to convert the input string in *ARGP into an explicit location. > > + ARGP is advanced past any processed input. Returns an event_location > > + (malloc'd) if an explicit location was successfully found in *ARGP, > > + NULL otherwise. > > + > > + IF !DONT_THROW, this function may call error() if *ARGP looks like > > + properly formed input, e.g., if it is called with missing argument > > + parameters or invalid options. If DONT_THROW is non-zero, this function > > + will not throw any exceptions. */ > > Duplicate function comment. Removed. > > + > > +extern struct event_location * > > + string_to_explicit_location (const char **argp, > > + const struct language_defn *langauge, > > + int dont_throw); > > + > > #endif /* LOCATIONS_H */ > > I didn't review the attached testcases, I was at a good stopping point > and needed a break. :-) I hear you! Thank you for persevering! Updated patch attached. Keith Changes since last revision: - collect_explicit_location_matches: WHICH -> WORD wrap multi-line stmts in {} Change gdb_assert_not_reached text - location_completer: use a cleanup for deleting location. - explicit_lcoation_lex_one: early return for "" input removed extra space in string string_to_explicit_locaiton: advanced past *ARGP removed extra space in comment. add early escape remove unused variables orig & copy & s & str. found another problem with -line and oarg check at end of loop -- oarg was already freed! - fixed duplicate function comments - tring_to_explicit_location needs to return an explicit location. The trailing garbage is handled by init_breakpoint_sal. find_thread_and_condition in create_breakpoint. If we found something that looks like an explicit location, we return it and stop processing the string when we do. We *can't* know what else may be around -- and more is permitted. - remove duplicate headers from linespec.c - *string = '\0' OKAY for linespec_parse_basic_offset. gdb/ChangeLog: * completer.c: Include location.h. (enum match_type): New enum. (location_completer): Rename to ... (linespec_completer): ... this. (collect_explicit_location_matches): New function. (explicit_location_completer): New function. (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): Export. (is_ada_operator): Ditto. (find_toplevel_char): Ditto. (linespec_parse_line_offset): Ditto. Issue error if STRING is not numerical. (gdb_get_linespec_parser_quote_characters): New function. * linespec.h (linespec_parse_line_offset): Declare. (location_completer): Declare. (get_gdb_linespec_parser_quote_characters): Declare. (is_ada_operator): Declare. (find_toplevel_char): 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.base/save-bp.exp: Add tests for address locations and explicit locations, pending and not-pending, with and without conditions, including resolved pending breakpoints. * 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. --- gdb/completer.c | 211 +++++++++++++++-- gdb/linespec.c | 30 ++ gdb/linespec.h | 30 ++ gdb/location.c | 219 +++++++++++++++++ gdb/location.h | 15 + gdb/testsuite/gdb.base/save-bp.exp | 23 ++ gdb/testsuite/gdb.linespec/3explicit.c | 28 ++ gdb/testsuite/gdb.linespec/cpexplicit.cc | 63 +++++ gdb/testsuite/gdb.linespec/cpexplicit.exp | 104 ++++++++ gdb/testsuite/gdb.linespec/explicit.c | 56 ++++ gdb/testsuite/gdb.linespec/explicit.exp | 366 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.linespec/explicit2.c | 24 ++ gdb/testsuite/gdb.linespec/ls-errs.exp | 45 +++- 13 files changed, 1167 insertions(+), 47 deletions(-) create mode 100644 gdb/testsuite/gdb.linespec/3explicit.c create mode 100644 gdb/testsuite/gdb.linespec/cpexplicit.cc create mode 100644 gdb/testsuite/gdb.linespec/cpexplicit.exp create mode 100644 gdb/testsuite/gdb.linespec/explicit.c create mode 100644 gdb/testsuite/gdb.linespec/explicit.exp create mode 100644 gdb/testsuite/gdb.linespec/explicit2.c diff --git a/gdb/completer.c b/gdb/completer.c index 94f70a9..a00d3cf 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -25,6 +25,7 @@ #include "gdb_assert.h" #include "exceptions.h" #include "gdb_signals.h" +#include "location.h" #include "cli/cli-decode.h" @@ -41,6 +42,21 @@ #include "completer.h" +/* An enumeration of the various things a user might + attempt to complete for a location. */ + +enum explicit_location_match_type +{ + /* The filename of a source file. */ + MATCH_SOURCE, + + /* The name of a function or method. */ + MATCH_FUNCTION, + + /* The name of a label. */ + MATCH_LABEL +}; + /* Prototypes for local functions. */ static char *line_completion_function (const char *text, int matches, @@ -173,7 +189,7 @@ filename_completer (struct cmd_list_element *ignore, return return_val; } -/* Complete on locations, which might be of two possible forms: +/* Complete on linespecs, which might be of two possible forms: file:line or @@ -182,9 +198,9 @@ filename_completer (struct cmd_list_element *ignore, This is intended to be used in commands that set breakpoints etc. */ -VEC (char_ptr) * -location_completer (struct cmd_list_element *ignore, - const char *text, const char *word) +static VEC (char_ptr) * +linespec_location_completer (struct cmd_list_element *ignore, + const char *text, const char *word) { int n_syms, n_files, ix; VEC (char_ptr) *fn_list = NULL; @@ -331,6 +347,175 @@ location_completer (struct cmd_list_element *ignore, return list; } +/* A helper function to collect explicit location matches for the given + LOCATION, which is attempting to match on WORD. */ + +static VEC (char_ptr) * +collect_explicit_location_matches (struct event_location *location, + enum explicit_location_match_type what, + const char *word) +{ + VEC (char_ptr) *matches = NULL; + struct explicit_location *explicit = get_explicit_location (location); + + switch (what) + { + case MATCH_SOURCE: + { + const char *text = (explicit->source_filename == NULL + ? "" : explicit->source_filename); + + matches = make_source_files_completion_list (text, word); + } + break; + + case MATCH_FUNCTION: + { + const char *text = (explicit->function_name == NULL + ? "" : explicit->function_name); + + if (explicit->source_filename != NULL) + { + matches + = make_file_symbol_completion_list (text, word, + explicit->source_filename); + } + else + matches = make_symbol_completion_list (text, word); + } + break; + + case MATCH_LABEL: + /* Not supported. */ + break; + + default: + gdb_assert_not_reached (_("unhandled explicit_location_match_type")); + } + + return matches; +} + +/* A convenience macro to (safely) back up P to the previous + word. */ + +static const char * +backup_text_ptr (const char *p, const char *text) +{ + while (p > text && isspace (*p)) + --p; + for (; p > text && !isspace (p[-1]); --p) + ; + + return p; +} + +static VEC (char_ptr) * +explicit_location_completer (struct cmd_list_element *ignore, + struct event_location *location, + const char *text, const char *word) +{ + const char *p; + VEC (char_ptr) *matches = NULL; + + /* Find the beginning of the word. This is necessary because + we need to know if we are completing an option name or value. We + don't get the leading '-' from the completer. */ + p = backup_text_ptr (word, text); + + if (*p == '-') + { + size_t len = strlen (word); + + /* Completing on option name. */ + + /* Skip over the '-'. */ + ++p; + + if (strncmp (p, "source", len) == 0) + VEC_safe_push (char_ptr, matches, xstrdup ("source")); + if (strncmp (p, "function", len) == 0) + VEC_safe_push (char_ptr, matches, xstrdup ("function")); + if (strncmp (p, "line", len) == 0) + VEC_safe_push (char_ptr, matches, xstrdup ("line")); + if (strncmp (p, "label", len) == 0) + VEC_safe_push (char_ptr, matches, xstrdup ("label")); + } + else + { + /* Completing on value (or unknown). Get the previous word to see what + the user is completing on. */ + size_t len, offset; + const char *new_word, *end; + enum explicit_location_match_type what; + struct explicit_location *explicit = get_explicit_location (location); + + /* Backup P to the previous word, which should be the option + the user is attempting to complete. */ + offset = word - p; + end = --p; + p = backup_text_ptr (p, text); + len = end - p; + + if (strncmp (p, "-source", len) == 0) + { + what = MATCH_SOURCE; + new_word = explicit->source_filename + offset; + } + else if (strncmp (p, "-function", len) == 0) + { + what = MATCH_FUNCTION; + new_word = explicit->function_name + offset; + } + else if (strncmp (p, "-label", len) == 0) + { + what = MATCH_LABEL; + new_word = explicit->label_name + offset; + } + else + { + /* The user isn't completing on any valid option name, + e.g., "break -source foo.c [tab]". */ + return NULL; + } + + /* Now gather matches */ + matches = collect_explicit_location_matches (location, what, new_word); + } + + return matches; +} + +/* A completer for locations. */ + +VEC (char_ptr) * +location_completer (struct cmd_list_element *ignore, + const char *text, const char *word) +{ + VEC (char_ptr) *matches = NULL; + const char *copy = text; + struct event_location *location; + + location = string_to_explicit_location (©, current_language, 1); + if (location != NULL) + { + struct cleanup *cleanup; + + cleanup = make_cleanup_delete_event_location (location); + matches = explicit_location_completer (ignore, location, text, word); + do_cleanups (cleanup); + } + else + { + /* This is an address or linespec location. + Right now both of these are handled by the (old) linespec + completer. */ + matches = linespec_location_completer (ignore, text, word); + } + + return matches; +} + /* Helper for expression_completer which recursively adds field and method names from TYPE, a struct or union type, to the array OUTPUT. */ @@ -668,16 +853,6 @@ complete_line_internal (const char *text, rl_completer_word_break_characters = gdb_completer_file_name_break_characters; } - else if (c->completer == location_completer) - { - /* Commands which complete on locations want to - see the entire argument. */ - for (p = word; - p > tmp_command - && p[-1] != ' ' && p[-1] != '\t'; - p--) - ; - } if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } @@ -743,14 +918,6 @@ complete_line_internal (const char *text, rl_completer_word_break_characters = gdb_completer_file_name_break_characters; } - else if (c->completer == location_completer) - { - for (p = word; - p > tmp_command - && p[-1] != ' ' && p[-1] != '\t'; - p--) - ; - } if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } diff --git a/gdb/linespec.c b/gdb/linespec.c index a529457..95916df 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -328,8 +328,6 @@ static int compare_symbols (const void *a, const void *b); static int compare_msymbols (const void *a, const void *b); -static const char *find_toplevel_char (const char *s, char c); - /* Permitted quote characters for the parser. This is different from the completer's quote characters to allow backward compatibility with the previous parser. */ @@ -376,10 +374,9 @@ linespec_lexer_lex_number (linespec_parser *parser, linespec_token *tokenp) return 1; } -/* Does P represent one of the keywords? If so, return - the keyword. If not, return NULL. */ +/* See description in linespec.h. */ -static const char * +const char * linespec_lexer_lex_keyword (const char *p) { int i; @@ -402,10 +399,9 @@ linespec_lexer_lex_keyword (const char *p) return NULL; } -/* Does STRING represent an Ada operator? If so, return the length - of the decoded operator name. If not, return 0. */ +/* See description in linespec.h. */ -static int +int is_ada_operator (const char *string) { const struct ada_opname_map *mapping; @@ -1118,7 +1114,7 @@ find_methods (struct type *t, const char *name, strings. Also, ignore the char within a template name, like a ',' within foo. */ -static const char * +const char * find_toplevel_char (const char *s, char c) { int quoted = 0; /* zero if we're not in quotes; @@ -1529,11 +1525,12 @@ source_file_not_found_error (const char *name) throw_error (NOT_FOUND_ERROR, _("No source file named %s."), name); } -/* Parse and return a line offset in STRING. */ +/* See description in linespec.h. */ -static struct line_offset +struct line_offset linespec_parse_line_offset (const char *string) { + const char *start = string; struct line_offset line_offset = {0, LINE_OFFSET_NONE}; if (*string == '+') @@ -1547,6 +1544,9 @@ linespec_parse_line_offset (const char *string) ++string; } + if (*string != '\0' && !isdigit (*string)) + error (_("malformed line offset: \"%s\""), start); + /* Right now, we only allow base 10 for offsets. */ line_offset.offset = atoi (string); return line_offset; @@ -3799,3 +3799,11 @@ make_cleanup_destroy_linespec_result (struct linespec_result *ls) { return make_cleanup (cleanup_linespec_result, ls); } + +/* Return the quote characters permitted by the linespec parser. */ + +const char * +get_gdb_linespec_parser_quote_characters (void) +{ + return linespec_quote_characters; +} diff --git a/gdb/linespec.h b/gdb/linespec.h index df5820a..87a678d 100644 --- a/gdb/linespec.h +++ b/gdb/linespec.h @@ -152,6 +152,36 @@ extern struct symtabs_and_lines decode_line_with_current_source (char *, int); extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int); +/* Parse a line offset from STRING. */ + +extern struct line_offset linespec_parse_line_offset (const char *string); + +/* A completion handler for locations. */ + +VEC (char_ptr) *location_completer (struct cmd_list_element *ignore, + const char *text, const char *word); + +/* Return the quote characters permitted by the linespec parser. */ + +extern const char *get_gdb_linespec_parser_quote_characters (void); + +/* Does STRING represent an Ada operator? If so, return the length + of the decoded operator name. If not, return 0. */ + +extern int is_ada_operator (const char *string); + +/* Find an instance of the character C in the string S that is outside + of all parenthesis pairs, single-quoted strings, and double-quoted + strings. Also, ignore the char within a template name, like a ',' + within foo. */ + +extern const char *find_toplevel_char (const char *s, char c); + +/* Does P represent one of the keywords? If so, return + the keyword. If not, return NULL. */ + +extern const char *linespec_lexer_lex_keyword (const char *p); + /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR, advancing EXP_PTR past any parsed text. */ diff --git a/gdb/location.c b/gdb/location.c index 9faad40..30ae181 100644 --- a/gdb/location.c +++ b/gdb/location.c @@ -434,6 +434,202 @@ 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. */ + if (*start == '-' || *start == '+') + *inp = skip_to_space_const (*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 ','. */ + *inp = start; + while ((*inp)[0] + && (*inp)[0] != ',' + && !(isspace ((*inp)[0]) + && ((*inp)[1] == '-' + || 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); + } + } + + if (*inp - start > 0) + return savestring (start, *inp - start); + + return NULL; +} + +/* See description in location.h. */ + +struct event_location * +string_to_explicit_location (const char **argp, + const struct language_defn *language, + int dont_throw) +{ + struct cleanup *cleanup; + struct event_location *location; + + /* It is assumed that input beginning with '-' and a non-digit + character is an explicit location. */ + if (argp == NULL + || *argp == '\0' + || *argp[0] != '-' + || !isalpha ((*argp)[1])) + return NULL; + + location = new_explicit_location (NULL); + cleanup = make_cleanup_delete_event_location (location); + + /* Process option/argument pairs. dprintf_command + requires that processing stop on ','. */ + while ((*argp)[0] != '\0' && (*argp)[0] != ',') + { + int len; + char *opt, *oarg; + const char *start; + struct cleanup *inner; + + /* If *ARGP starts with a keyword, stop processing + options. */ + if (linespec_lexer_lex_keyword (*argp) != NULL) + break; + + /* Mark the start of the string in case we need to rewind. */ + start = *argp; + + /* Get the option string. */ + opt = explicit_location_lex_one (argp, language); + inner = make_cleanup (xfree, opt); + + *argp = skip_spaces_const (*argp); + + /* Get the argument string. */ + oarg = explicit_location_lex_one (argp, language); + + *argp = skip_spaces_const (*argp); + + /* Use the length of the option to allow abbreviations. */ + len = strlen (opt); + + /* All options have a required argument. Checking for this required + argument is deferred until later. */ + if (strncmp (opt, "-source", len) == 0) + EL_EXPLICIT (location)->source_filename = oarg; + else if (strncmp (opt, "-function", len) == 0) + EL_EXPLICIT (location)->function_name = oarg; + else if (strncmp (opt, "-line", len) == 0) + { + if (oarg != NULL) + { + volatile struct gdb_exception e; + + TRY_CATCH (e, RETURN_MASK_ERROR) + { + EL_EXPLICIT (location)->line_offset + = linespec_parse_line_offset (oarg); + } + + xfree (oarg); + if (e.reason < 0 && !dont_throw) + throw_exception (e); + do_cleanups (inner); + continue; + } + } + else if (strncmp (opt, "-label", len) == 0) + EL_EXPLICIT (location)->label_name = oarg; + /* Only emit an "invalid argument" error for options + that look like option strings. */ + else if (opt[0] == '-' && !isdigit (opt[1])) + { + xfree (oarg); + if (!dont_throw) + error (_("invalid explicit location argument, \"%s\""), opt); + } + else + { + /* End of the explicit location specification. + Stop parsing and return whatever explicit location was + parsed. */ + *argp = start; + xfree (oarg); + do_cleanups (inner); + discard_cleanups (cleanup); + return location; + } + + /* It's a little lame to error after the fact, but in this + case, it provides a much better user experience to issue + the "invalid argument" error before any missing + argument error. */ + if (oarg == NULL && !dont_throw) + error (_("missing argument for \"%s\""), opt); + + /* The option/argument pari was successfully processed; + oarg belongs to the explicit location, and opt should + be freed. */ + do_cleanups (inner); + } + + /* One special error check: If a source filename was given + without offset, function, or label, issue an error. */ + if (EL_EXPLICIT (location)->source_filename != NULL + && EL_EXPLICIT (location)->function_name == NULL + && EL_EXPLICIT (location)->label_name == NULL + && (EL_EXPLICIT (location)->line_offset.sign == LINE_OFFSET_UNKNOWN) + && !dont_throw) + { + error (_("Source filename requires function, label, or " + "line offset.")); + } + + discard_cleanups (cleanup); + return location; +} + /* See description in location.h. */ struct event_location * @@ -466,10 +662,25 @@ string_to_event_location (char **stringp, } else { - /* Everything else is a linespec. */ - location = new_linespec_location (*stringp); - if (*stringp != NULL) - *stringp += strlen (*stringp); + const char *arg, *orig; + + /* Next, try an explicit location. */ + orig = arg = *stringp; + location = string_to_explicit_location (&arg, language, 0); + if (location != NULL) + { + /* It was a valid explicit location. Advance STRINGP to + the end of input. */ + *stringp += arg - orig; + } + else + { + /* Everything else is a linespec and left for the linespec + parser to sort out. */ + location = new_linespec_location (*stringp); + if (*stringp != NULL) + *stringp += strlen (*stringp); + } } } diff --git a/gdb/location.h b/gdb/location.h index 0adb0c5..0c30675 100644 --- a/gdb/location.h +++ b/gdb/location.h @@ -212,6 +212,21 @@ extern struct event_location * string_to_event_location (char **argp, const struct language_defn *langauge); +/* Attempt to convert the input string in *ARGP into an explicit location. + ARGP is advanced past any processed input. Returns an event_location + (malloc'd) if an explicit location was successfully found in *ARGP, + NULL otherwise. + + IF !DONT_THROW, this function may call error() if *ARGP looks like + properly formed input, e.g., if it is called with missing argument + parameters or invalid options. If DONT_THROW is non-zero, this function + will not throw any exceptions. */ + +extern struct event_location * + string_to_explicit_location (const char **argp, + const struct language_defn *langauge, + int dont_throw); + /* A convenience function for testing for unset locations. */ extern int event_location_empty_p (const struct event_location *location); diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp index ba98633..5fab7cf 100644 --- a/gdb/testsuite/gdb.base/save-bp.exp +++ b/gdb/testsuite/gdb.base/save-bp.exp @@ -27,6 +27,8 @@ if ![runto_main] { # does not interfere with our testing. delete_breakpoints +set savefile [standard_output_file "bps"] + # Insert a bunch of breakpoints... The goal is to create breakpoints # that we are going to try to save in a file and then reload. So # try to create a good variety of them. @@ -45,6 +47,8 @@ set loc_bp5 [gdb_get_line_number "with commands"] gdb_breakpoint ${srcfile}:${loc_bp5} gdb_test "commands\nsilent\nend" "End with.*" "add breakpoint commands" +gdb_breakpoint "*break_me" + gdb_test "dprintf ${srcfile}:${loc_bp5},\"At foo entry\\n\"" "Dprintf .*" # Now, save the breakpoints into a file... @@ -72,5 +76,20 @@ gdb_test "source $bps" "" "source bps" # Now, verify that all breakpoints have been created correctly... set bp_row_start "\[0-9\]+ +breakpoint +keep +y +0x\[0-9a-f\]+ +in" set dprintf_row_start "\[0-9\]+ +dprintf +keep +y +0x\[0-9a-f\]+ +in" -gdb_test "info break" \ - " *Num +Type +Disp +Enb +Address +What\r\n$bp_row_start break_me at .*$srcfile:\[0-9\]+\r\n$bp_row_start main at .*$srcfile:$loc_bp2\r\n$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1\r\n\[ \t]+stop only in thread 1\r\n$bp_row_start main at .*$srcfile:$loc_bp4\r\n\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?\r\n$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent\r\n$dprintf_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+printf.*" + +set row_list \ + [list \ + " *Num +Type +Disp +Enb +Address +What" \ + "$bp_row_start break_me at .*$srcfile:\[0-9\]+" \ + "$bp_row_start main at .*$srcfile:$loc_bp2" \ + "$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1" \ + "\[ \t]+stop only in thread 1" \ + "$bp_row_start main at .*$srcfile:$loc_bp4" \ + "\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?" \ + "$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent" \ + "$bp_row_start break_me at .*$srcfile:\[0-9\]+" \ + "$dprintf_row_start main at .*$srcfile:$loc_bp5" \ + "\[ \t\]+printf.*"] +set expected [join $row_list {\r\n}] +verbose -log "Expecting: $expected" +gdb_test "info break" $expected diff --git a/gdb/testsuite/gdb.linespec/3explicit.c b/gdb/testsuite/gdb.linespec/3explicit.c new file mode 100644 index 0000000..12bf277 --- /dev/null +++ b/gdb/testsuite/gdb.linespec/3explicit.c @@ -0,0 +1,28 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 . */ + +static int +myfunction4 (int arg) +{ + return arg + 2; +} + +int +myfunction3 (int arg) +{ + return myfunction4 (arg); +} diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.cc b/gdb/testsuite/gdb.linespec/cpexplicit.cc new file mode 100644 index 0000000..42d50c7 --- /dev/null +++ b/gdb/testsuite/gdb.linespec/cpexplicit.cc @@ -0,0 +1,63 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2012-2013 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 . */ + +class myclass +{ +public: + static int myfunction (int arg) /* entry location */ + { + int i, j, r; + + j = 0; /* myfunction location */ + r = arg; + + top: + ++j; /* top location */ + + if (j == 10) + goto done; + + for (i = 0; i < 10; ++i) + { + r += i; + if (j % 2) + goto top; + } + + done: + return r; + } + + int operator, (const myclass& c) { return 0; } /* operator location */ +}; + +int +main (void) +{ + int i, j; + + /* Call the test function repeatedly, enough times for all our tests + without running forever if something goes wrong. */ + myclass c, d; + for (i = 0, j = 0; i < 1000; ++i) + { + j += myclass::myfunction (0); + j += (c,d); + } + + return j; +} diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.exp b/gdb/testsuite/gdb.linespec/cpexplicit.exp new file mode 100644 index 0000000..2bb9291 --- /dev/null +++ b/gdb/testsuite/gdb.linespec/cpexplicit.exp @@ -0,0 +1,104 @@ +# Copyright 2012-2013 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 linespecs + +if {[skip_cplus_tests]} { + unsupported "skipping C++ tests" + return +} + +standard_testfile .cc +set exefile $testfile + +if {[prepare_for_testing $testfile $exefile $srcfile \ + {c++ debug nowarnings}]} { + return -1 +} + +# Test the given (explicit) LINESPEC which should cause gdb to break +# at LOCATION. +proc test_breakpoint {linespec location} { + + # Delete all breakpoints, set a new breakpoint at LINESPEC, + # and attempt to run to it. + delete_breakpoints + gdb_breakpoint $linespec + gdb_continue_to_breakpoint $linespec $location +} + +# 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 +} + +# Make sure variables are not already in use +unset -nocomplain lineno location linespecs + +# Some locations used in this test +set lineno(normal) [gdb_get_line_number "myfunction location" $srcfile] +set lineno(entry) [gdb_get_line_number "entry location" $srcfile] +set lineno(top) [gdb_get_line_number "top location" $srcfile] +set lineno(operator) [gdb_get_line_number "operator location" $srcfile] +foreach v [array names lineno] { + set location($v) ".*[string_to_regexp "$srcfile:$lineno($v)"].*" +} + +# A list of explicit linespecs and the corresponding location +set linespecs(linespecs) {} +set linespecs(location) {} + +add linespecs "-source $srcfile -function myclass::myfunction" $location(normal) +add linespecs "-source $srcfile -function myclass::myfunction -label top" \ + $location(top) + +# This isn't implemented yet; -line is silently ignored. +add linespecs \ + "-source $srcfile -function myclass::myfunction -label top -line 3" \ + $location(top) +add linespecs "-source $srcfile -line $lineno(top)" $location(top) +add linespecs "-function myclass::myfunction" $location(normal) +add linespecs "-function myclass::myfunction -label top" $location(top) + +# These are also not yet supported; -line is silently ignored. +add linespecs "-function myclass::myfunction -line 3" $location(normal) +add linespecs "-function myclass::myfunction -label top -line 3" $location(top) +add linespecs "-line 3" $location(normal) +add linespecs "-function myclass::operator," $location(operator) +add linespecs "-function 'myclass::operator,'" $location(operator) +add linespecs "-function \"myclass::operator,\"" $location(operator) + +# Fire up gdb. +if {![runto_main]} { + return -1 +} + +# Test explicit linespecs, with and without conditions. +foreach linespec $linespecs(linespecs) loc_pattern $linespecs(locations) { + # Test the linespec + test_breakpoint $linespec $loc_pattern +} + +# Special (orphaned) dprintf cases. +gdb_test "dprintf -function myclass::operator,,\"hello\"" \ + "Dprintf .*$srcfile, line $lineno(operator)\\." +gdb_test "dprintf -function 'myclass::operator,',\"hello\"" \ + "Dprintf .*$srcfile, line $lineno(operator)\\." +gdb_test "dprintf -function \"myclass::operator,\",\"hello\"" \ + "Dprintf .*$srcfile, line $lineno(operator)\\." diff --git a/gdb/testsuite/gdb.linespec/explicit.c b/gdb/testsuite/gdb.linespec/explicit.c new file mode 100644 index 0000000..4e1c635 --- /dev/null +++ b/gdb/testsuite/gdb.linespec/explicit.c @@ -0,0 +1,56 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2012-2013 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 . */ + +extern int myfunction2 (int arg); + +static int +myfunction (int arg) +{ + int i, j, r; + + j = 0; /* myfunction location */ + r = arg; + + top: + ++j; /* top location */ + + if (j == 10) + goto done; + + for (i = 0; i < 10; ++i) + { + r += i; + if (j % 2) + goto top; + } + + done: + return r; +} + +int +main (void) +{ + int i, j; + + /* Call the test function repeatedly, enough times for all our tests + without running forever if something goes wrong. */ + for (i = 0, j = 0; i < 1000; ++i) + j += myfunction (0); + + return myfunction2 (j); +} diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp new file mode 100644 index 0000000..854e16f --- /dev/null +++ b/gdb/testsuite/gdb.linespec/explicit.exp @@ -0,0 +1,366 @@ +# Copyright 2012-2014 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 +} + +# 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 +} + +# Make sure variables are not already in use +unset -nocomplain all_arguments lineno location linespecs + +# A list of all explicit linespec arguments. +set all_arguments {"source" "function" "label" "line"} + +# Some locations used in this test +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. +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 +} + +# 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 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 + } + } + } +} + +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" + } +} + +# NOTE: We don't bother testing more elaborate combinations of options, +# such as "-func main -sour 3ex\t" (main is defined in explicit.c). The +# completer cannot handle these yet. + +# Test pending explicit breakpoints +gdb_exit +gdb_start + +set tst "pending invalid conditional explicit breakpoint" +if {![gdb_breakpoint "-func myfunction if foofoofoo == 1" \ + allow-pending]} { + fail "set $tst" +} else { + gdb_test "info break" ".*PENDING.*myfunction if foofoofoo == 1.*" $tst +} + +gdb_exit +gdb_start + +set tst "pending valid conditional explicit breakpoint" +if {![gdb_breakpoint "-func myfunction if arg == 0" \ + allow-pending]} { + fail "set $tst" +} else { + gdb_test "info break" ".*PENDING.*myfunction if arg == 0" $tst + + gdb_load [standard_output_file $exefile] + gdb_test "info break" \ + ".*in myfunction at .*$srcfile:.*stop only if arg == 0.*" \ + "$tst resolved" +} + + +# Test interaction of condition command and explicit linespec conditons. +gdb_exit +gdb_start +gdb_load [standard_output_file $exefile] + +set tst "condition_command overrides explicit linespec condition" +if {![runto main]} { + fail $tst +} else { + if {![gdb_breakpoint "-func myfunction if arg == 1"]} { + fail "set breakpoint with condition 'arg == 1'" + } else { + gdb_test_no_output "cond 2 arg == 0" \ + "set new breakpoint condition for explicit linespec" + + gdb_continue_to_breakpoint $tst $location(normal) + } +} + +gdb_test "cond 2" [string_to_regexp "Breakpoint 2 now unconditional."] \ + "clear condition for explicit breakpoint" +set tst "info break of cleared condition of explicit breakpoint" +gdb_test_multiple "info break" $tst { + -re ".*in myfunction at .*$srcfile:.*stop only if arg == 0.*" { + fail $tst + } + -re ".*in myfunction at .*$srcfile:.*$gdb_prompt $" { + pass $tst + } +} + +unset -nocomplain lineno tst diff --git a/gdb/testsuite/gdb.linespec/explicit2.c b/gdb/testsuite/gdb.linespec/explicit2.c new file mode 100644 index 0000000..218cccb --- /dev/null +++ b/gdb/testsuite/gdb.linespec/explicit2.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 . */ + +extern int myfunction3 (int arg); + +int +myfunction2 (int arg) +{ + return myfunction3 (arg); +} diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 86056c5..949bae6 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -46,11 +46,16 @@ array set error_messages { invalid_var_or_func_f \ "Undefined convenience variable or function \"%s\" not defined in \"%s\"." invalid_label "No label \"%s\" defined in function \"%s\"." + invalid_parm "invalid linespec argument, \"%s\"" invalid_offset "No line %d in the current file." invalid_offset_f "No line %d in file \"%s\"." + malformed_line_offset "malformed line offset: \"%s\"" + source_incomplete \ + "Source filename requires function, label, or line offset." unexpected "malformed linespec error: unexpected %s" unexpected_opt "malformed linespec error: unexpected %s, \"%s\"" unmatched_quote "unmatched quote" + garbage "Garbage '%s' at end of command" } # Some commonly used whitespace tests around ':'. @@ -77,6 +82,7 @@ foreach x $invalid_offsets { incr offset 16 } test_break $x invalid_offset $offset + test_break "-line $x" invalid_offset $offset } # Test offsets with trailing tokens w/ and w/o spaces. @@ -88,13 +94,17 @@ foreach x $spaces { foreach x {1 +1 +100 -10} { test_break "3 $x" unexpected_opt "number" $x + test_break "-line 3 $x" garbage $x test_break "+10 $x" unexpected_opt "number" $x + test_break "-line +10 $x" garbage $x test_break "-10 $x" unexpected_opt "number" $x + test_break "-line -10 $x" garbage $x } -test_break "3 foo" unexpected_opt "string" "foo" -test_break "+10 foo" unexpected_opt "string" "foo" -test_break "-10 foo" unexpected_opt "string" "foo" +foreach x {3 +10 -10} { + test_break "$x foo" unexpected_opt "string" "foo" + test_break "-line $x foo" garbage "foo" +} # Test invalid linespecs starting with filename. foreach x [list "this_file_doesn't_exist.c" \ @@ -110,6 +120,13 @@ foreach x [list "this_file_doesn't_exist.c" \ # Remove any quoting from FILENAME for the error message. test_break "$x:3" invalid_file [string trim $x \"'] } +foreach x [list "this_file_doesn't_exist.c" \ + "this file has spaces.c" \ + "file::colons.c" \ + "'file::colons.c'"] { + test_break "-source $x -line 3" \ + invalid_file [string trim $x \"'] +} # Test unmatched quotes. foreach x {"\"src-file.c'" "'src-file.c"} { @@ -120,7 +137,11 @@ test_break $srcfile invalid_function $srcfile foreach x {"foo" " foo" " foo "} { # Trim any leading/trailing whitespace for error messages. test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile + test_break "-source $srcfile -function $x" \ + invalid_function_f [string trim $x] $srcfile test_break "$srcfile:main:$x" invalid_label [string trim $x] "main" + test_break "-source $srcfile -function main -label $x" \ + invalid_label [string trim $x] "main" } foreach x $spaces { @@ -130,20 +151,26 @@ foreach x $spaces { test_break "${srcfile}::" invalid_function "${srcfile}::" test_break "$srcfile:3 1" unexpected_opt "number" "1" +test_break "-source $srcfile -line 3 1" garbage "1" test_break "$srcfile:3 +100" unexpected_opt "number" "+100" +test_break "-source $srcfile -line 3 +100" garbage "+100" test_break "$srcfile:3 -100" unexpected_opt "number" "-100" test_break "$srcfile:3 foo" unexpected_opt "string" "foo" +test_break "-source $srcfile -line 3 foo" garbage "foo" foreach x $invalid_offsets { test_break "$srcfile:$x" invalid_offset_f $x $srcfile test_break "\"$srcfile:$x\"" invalid_offset_f $x $srcfile test_break "'$srcfile:$x'" invalid_offset_f $x $srcfile + test_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile } +test_break "-source $srcfile -line -x" malformed_line_offset "-x" # Test invalid filespecs starting with function. foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \ "foo 0" "foo +10" "foo -10" "foo +100" "foo -100"} { test_break $x invalid_function $x + test_break "-function \"$x\"" invalid_function $x } foreach x $spaces { @@ -152,13 +179,12 @@ foreach x $spaces { test_break "main:here${x}" unexpected "end of input" } -test_break "main 3" invalid_function "main 3" -test_break "main +100" invalid_function "main +100" -test_break "main -100" invalid_function "main -100" -test_break "main foo" invalid_function "main foo" - foreach x {"3" "+100" "-100" "foo"} { + test_break "main 3" invalid_function "main 3" + test_break "-function \"main $x\"" invalid_function "main $x" test_break "main:here $x" invalid_label "here $x" "main" + test_break "-function main -label \"here $x\"" \ + invalid_label "here $x" "main" } foreach x {"if" "task" "thread"} { @@ -175,3 +201,6 @@ test_break "'main.c'+3" unexpected_opt "number" "+3" set x {$zippo} test_break $x invalid_var_or_func $x test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile + +# Explicit linespec-specific tests +test_break "-source $srcfile" source_incomplete