From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129453 invoked by alias); 17 May 2015 20:54:51 -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 129442 invoked by uid 89); 17 May 2015 20:54:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f53.google.com Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 17 May 2015 20:54:46 +0000 Received: by pabts4 with SMTP id ts4so121063848pab.3 for ; Sun, 17 May 2015 13:54:45 -0700 (PDT) X-Received: by 10.70.90.129 with SMTP id bw1mr38391768pdb.85.1431896084947; Sun, 17 May 2015 13:54:44 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by mx.google.com with ESMTPSA id hj11sm7879218pbd.33.2015.05.17.13.54.43 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 17 May 2015 13:54:44 -0700 (PDT) From: Doug Evans To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v4 2/9] Explicit locations: introduce new struct event_location-based API References: <20150507180523.19629.77846.stgit@valrhona.uglyboxes.com> <20150507180549.19629.87819.stgit@valrhona.uglyboxes.com> Date: Sun, 17 May 2015 20:54:00 -0000 In-Reply-To: <20150507180549.19629.87819.stgit@valrhona.uglyboxes.com> (Keith Seitz's message of "Thu, 07 May 2015 11:05:49 -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-05/txt/msg00448.txt.bz2 Keith Seitz writes: > This patch introduces the new breakpoint/"linespec" API based on > a new struct event_location. This API currently only supports > traditional linespecs, maintaining the status quo of the code base. > Future patches will add additional functionality for other location > types such as address locations. Hi. Just a couple of nits and a couple of questions. I wrote down a question for myself below. I haven't read the entire series yet. > > gdb/ChangeLog: > > * Makefile.in (SFILES): Add location.c. > (HFILES_NO_SRCDIR): Add location.h. > (COMMON_OBS): Add location.o. > * linespec.c (linespec_lex_to_end): New function. > * linespec.c (linespec_lex_to_end): Declare. typo: linespec.h > * location.c: New file. > * location.h: New file. >... > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index 95104ef..0a51564 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -851,7 +851,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \ > inline-frame.c \ > interps.c \ > jv-exp.y jv-lang.c jv-valprint.c jv-typeprint.c jv-varobj.c \ > - language.c linespec.c minidebug.c \ > + language.c linespec.c location.c minidebug.c \ > m2-exp.y m2-lang.c m2-typeprint.c m2-valprint.c \ > macrotab.c macroexp.c macrocmd.c macroscope.c main.c maint.c \ > mdebugread.c memattr.c mem-break.c minsyms.c mipsread.c memory-map.c \ > @@ -937,7 +937,7 @@ mi/mi-out.h mi/mi-main.h mi/mi-common.h mi/mi-cmds.h linux-nat.h \ > complaints.h gdb_proc_service.h gdb_regex.h xtensa-tdep.h inf-loop.h \ > common/gdb_wait.h common/gdb_assert.h solib.h ppc-tdep.h cp-support.h glibc-tdep.h \ > interps.h auxv.h gdbcmd.h tramp-frame.h mipsnbsd-tdep.h \ > -amd64-linux-tdep.h linespec.h i387-tdep.h mn10300-tdep.h \ > +amd64-linux-tdep.h linespec.h location.h i387-tdep.h mn10300-tdep.h \ > sparc64-tdep.h monitor.h ppcobsd-tdep.h srec.h \ > coff-pe-read.h parser-defs.h gdb_ptrace.h mips-linux-tdep.h \ > m68k-tdep.h spu-tdep.h jv-lang.h environ.h amd64-tdep.h \ > @@ -1021,7 +1021,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ > source.o value.o eval.o valops.o valarith.o valprint.o printcmd.o \ > block.o symtab.o psymtab.o symfile.o symfile-debug.o symmisc.o \ > linespec.o dictionary.o \ > - infcall.o \ > + location.o infcall.o \ > infcmd.o infrun.o \ > expprint.o environ.o stack.o thread.o \ > exceptions.o \ > diff --git a/gdb/linespec.c b/gdb/linespec.c > index d2089b5..a480be1 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -2435,6 +2435,61 @@ linespec_parser_delete (void *arg) > linespec_state_destructor (PARSER_STATE (parser)); > } > > +/* See description in linespec.h. */ > + > +void > +linespec_lex_to_end (char **stringp) > +{ > + linespec_parser parser; > + struct cleanup *cleanup; > + linespec_token token; > + volatile struct gdb_exception e; > + const char *orig; > + > + if (stringp == NULL || *stringp == NULL) > + return; > + > + linespec_parser_new (&parser, 0, current_language, NULL, 0, NULL); > + cleanup = make_cleanup (linespec_parser_delete, &parser); > + parser.lexer.saved_arg = *stringp; > + PARSER_STREAM (&parser) = orig = *stringp; > + > + TRY > + { > + do > + { > + /* Stop before any comma tokens; we need it to keep it > + as the next token in the string. */ > + token = linespec_lexer_peek_token (&parser); > + if (token.type == LSTOKEN_COMMA) > + break; > + > + /* For addresses advance the parser stream past > + any parsed input and stop lexing. */ > + if (token.type == LSTOKEN_STRING > + && *LS_TOKEN_STOKEN (token).ptr == '*') > + { > + const char *arg; > + > + arg = *stringp; > + (void) linespec_expression_to_pc (&arg); > + PARSER_STREAM (&parser) = arg; > + break; > + } > + > + token = linespec_lexer_consume_token (&parser); > + } > + while (token.type != LSTOKEN_EOI && token.type != LSTOKEN_KEYWORD); > + } > + CATCH (e, RETURN_MASK_ERROR) > + { I'm guessing I'm missing something here. Is the intent to ignore errors here? > + } > + END_CATCH > + > + *stringp += PARSER_STREAM (&parser) - orig; > + do_cleanups (cleanup); > +} > + > /* See linespec.h. */ > > void > diff --git a/gdb/linespec.h b/gdb/linespec.h > index 7e66024..77ec46d 100644 > --- a/gdb/linespec.h > +++ b/gdb/linespec.h > @@ -156,4 +156,9 @@ extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int); > the keyword. If not, return NULL. */ > > extern const char *linespec_lexer_lex_keyword (const char *p); > + > +/* Find the end of the (first) linespec pointed to by *STRINGP. > + STRINGP will be advanced to this point. */ > + > +extern void linespec_lex_to_end (char **stringp); > #endif /* defined (LINESPEC_H) */ > diff --git a/gdb/location.c b/gdb/location.c > new file mode 100644 > index 0000000..39e09c1 > --- /dev/null > +++ b/gdb/location.c > @@ -0,0 +1,234 @@ > +/* Data structures and API for event locations in GDB. > + Copyright (C) 2013-2015 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + 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 . */ > + > +#include "defs.h" > +#include "gdb_assert.h" > +#include "location.h" > +#include "symtab.h" > +#include "language.h" > +#include "linespec.h" > +#include "cli/cli-utils.h" > +#include "probe.h" > + > +#include > +#include > + > +/* An event location used to set a stop event in the inferior. > + This structure is an amalgam of the various ways > + to specify where a stop event should be set. */ > + > +struct event_location > +{ > + /* The type of this breakpoint specification. */ > + enum event_location_type type; > +#define EL_TYPE(PTR) (PTR)->type > + > + union > + { > + /* A generic "this is a string specification" for a location. > + This representation is used by both "normal" linespecs and > + probes. */ > + char *addr_string; > +#define EL_LINESPEC(PTR) ((PTR)->u.addr_string) > + } u; > + > + /* Cached string representation of this location. This is used, e.g., to > + save stop event locations to file. Malloc'd. */ > + char *as_string; > +#define EL_STRING(PTR) ((PTR)->as_string) > +}; > + > +/* See description in location.h. */ > + > +enum event_location_type > +event_location_type (const struct event_location *location) > +{ > + return EL_TYPE (location); > +} > + > +/* See description in location.h. */ > + > +struct event_location * > +new_linespec_location (char **linespec) > +{ > + struct event_location *location; > + > + location = XCNEW (struct event_location); > + EL_TYPE (location) = LINESPEC_LOCATION; > + if (*linespec != NULL) > + { > + char *p; > + char *orig = *linespec; > + > + linespec_lex_to_end (linespec); > + p = remove_trailing_whitespace (orig, *linespec); > + if ((p - orig) > 0) > + EL_LINESPEC (location) = savestring (orig, p - orig); > + } > + return location; > +} > + > +/* See description in location.h. */ > + > +const char * > +get_linespec_location (const struct event_location *location) > +{ > + gdb_assert (EL_TYPE (location) == LINESPEC_LOCATION); > + return EL_LINESPEC (location); > +} > + > +/* See description in location.h. */ > + > +struct event_location * > +copy_event_location (const struct event_location *src) > +{ > + struct event_location *dst; > + > + dst = XCNEW (struct event_location); > + EL_TYPE (dst) = EL_TYPE (src); > + if (EL_STRING (src) != NULL) > + EL_STRING (dst) = xstrdup (EL_STRING (src)); > + > + switch (EL_TYPE (src)) > + { > + case LINESPEC_LOCATION: > + if (EL_LINESPEC (src) != NULL) > + EL_LINESPEC (dst) = xstrdup (EL_LINESPEC (src)); > + break; > + > + default: > + gdb_assert_not_reached ("unknown event location type"); > + } > + > + return dst; > +} > + > +/* A cleanup function for struct event_location. */ > + > +static void > +delete_event_location_cleanup (void *data) > +{ > + struct event_location *location = (struct event_location *) data; > + > + delete_event_location (location); > +} > + > +/* See description in location.h. */ > + > +struct cleanup * > +make_cleanup_delete_event_location (struct event_location *location) > +{ > + return make_cleanup (delete_event_location_cleanup, location); > +} > + > +/* See description in location.h. */ > + > +void > +delete_event_location (struct event_location *location) > +{ > + if (location != NULL) > + { > + xfree (EL_STRING (location)); > + > + switch (EL_TYPE (location)) > + { > + case LINESPEC_LOCATION: > + xfree (EL_LINESPEC (location)); > + break; > + > + default: > + gdb_assert_not_reached ("unknown event location type"); > + } > + > + xfree (location); > + } > +} > + > +/* See description in location.h. */ > + > +char * > +event_location_to_string_const (const struct event_location *location) > +{ > + char *result = NULL; > + > + if (EL_STRING (location) != NULL) > + return xstrdup (EL_STRING (location)); > + > + switch (EL_TYPE (location)) > + { > + case LINESPEC_LOCATION: > + if (EL_LINESPEC (location) != NULL) > + result = xstrdup (EL_LINESPEC (location)); > + break; > + > + default: > + gdb_assert_not_reached ("unknown event location type"); > + } > + > + return result; > +} > + > +/* See description in location.h. */ > + > +const char * > +event_location_to_string (struct event_location *location) > +{ > + /* Cache a copy of the string representation. */ > + if (EL_STRING (location) == NULL) > + EL_STRING (location) = event_location_to_string_const (location); > + > + return EL_STRING (location); > +} > + > +/* See description in location.h. */ > + > +struct event_location * > +string_to_event_location (char **stringp, > + const struct language_defn *language) > +{ > + struct event_location *location; > + > + location = new_linespec_location (stringp); > + return location; > +} > + > +/* See description in location.h. */ > + > +int > +event_location_empty_p (const struct event_location *location) > +{ > + switch (EL_TYPE (location)) > + { > + case LINESPEC_LOCATION: > + /* Linespecs are never "empty." (NULL is a valid linespec) */ > + return 0; > + > + default: > + gdb_assert_not_reached ("unknown event location type"); > + } > +} > + > +/* See description in location.h. */ > + > +void > +set_event_location_string (struct event_location *location, > + const char *string) > +{ > + xfree (EL_STRING (location)); > + EL_STRING (location) = string == NULL ? NULL : xstrdup (string); > +} > diff --git a/gdb/location.h b/gdb/location.h > new file mode 100644 > index 0000000..992f21e > --- /dev/null > +++ b/gdb/location.h > @@ -0,0 +1,113 @@ > +/* Data structures and API for event locations in GDB. > + Copyright (C) 2013-2015 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + 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 . */ > + > +#ifndef LOCATIONS_H > +#define LOCATIONS_H 1 > + > +struct language_defn; > +struct event_location; > + > +/* An enumeration of the various ways to specify a stop event > + location (used with create_breakpoint). */ > + > +enum event_location_type > +{ > + /* A traditional linespec. */ > + LINESPEC_LOCATION > +}; > + > +/* Return the type of the given event location. */ > + > +extern enum event_location_type > + event_location_type (const struct event_location *); > + > +/* Return a string representation of the LOCATION. > + This function may return NULL for unspecified linespecs, > + e.g, LOCATION_LINESPEC and addr_string is NULL. > + > + The result is cached in LOCATION. */ > + > +extern const char * > + event_location_to_string (struct event_location *location); > + > +/* A const version of event_location_to_string that will not cache > + the resulting string representation. The result is malloc'd > + and must be freed by the caller. */ > + > +extern char * > + event_location_to_string_const (const struct event_location *location); Note to self: Do we need both non-const and const versions? [e.g., treat cached value as mutable in c++ sense?] > + > +/* Create a new linespec location. The return result is malloc'd > + and should be freed with delete_event_location. */ > + > +extern struct event_location * > + new_linespec_location (char **linespec); > + > +/* Return the linespec location (a string) of the given event_location > + (which must be of type LINESPEC_LOCATION). */ > + > +extern const char * > + get_linespec_location (const struct event_location *location); > + > +/* Free an event location and any associated data. */ > + > +extern void delete_event_location (struct event_location *location); > + > +/* Make a cleanup to free LOCATION. */ > + > +extern struct cleanup * > + make_cleanup_delete_event_location (struct event_location *location); > + > +/* Return a copy of the given SRC location. */ > + > +extern struct event_location * > + copy_event_location (const struct event_location *src); > + > +/* Allocate and "copy" the opaque struct event_location. This is used > + when decoding locations which must parse their inputs. The return result > + should be freed. */ > + > +extern struct event_location * > + copy_event_location_tmp (const struct event_location *src); This function doesn't exist in this patch. > + > +/* Attempt to convert the input string in *ARGP into an event location. > + ARGP is advanced past any processed input. Returns a event_location nit: an event_location > + (malloc'd) if an event location was successfully found in *ARGP, > + NULL otherwise. > + > + This function may call error() if *ARGP looks like properly formed, > + but invalid, input, e.g., if it is called with missing argument parameters > + or invalid options. > + > + The return result must be freed with delete_event_location. */ > + > +extern struct event_location * > + string_to_event_location (char **argp, > + const struct language_defn *langauge); > + > +/* A convenience function for testing for unset locations. */ > + > +extern int event_location_empty_p (const struct event_location *location); > + > +/* Set the location's string representation. If STRING is NULL, clear > + the string representation. */ > + > +extern void > + set_event_location_string (struct event_location *location, > + const char *string); > +#endif /* LOCATIONS_H */