[v6,2/9] Explicit locations: introduce new struct event_location-based API

Message ID 20150805232913.21646.96976.stgit@valrhona.uglyboxes.com
State New, archived
Headers

Commit Message

Keith Seitz Aug. 5, 2015, 11:29 p.m. UTC
  Differences in this revision:

1. Fixed typos
2. Changed error-handling in linespec_lex_to_end.
3. Remove unused references to copy_event_location_tmp.

--

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.

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.h (linespec_lex_to_end): Declare.
	* location.c: New file.
	* location.h: New file.
---
 gdb/Makefile.in |    6 +
 gdb/linespec.c  |   48 +++++++++++
 gdb/linespec.h  |    5 +
 gdb/location.c  |  234 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/location.h  |  106 +++++++++++++++++++++++++
 5 files changed, 396 insertions(+), 3 deletions(-)
 create mode 100644 gdb/location.c
 create mode 100644 gdb/location.h
  

Comments

Doug Evans Aug. 10, 2015, 5:33 p.m. UTC | #1
Keith Seitz <keiths@redhat.com> writes:
> Differences in this revision:
>
> 1. Fixed typos
> 2. Changed error-handling in linespec_lex_to_end.
> 3. Remove unused references to copy_event_location_tmp.
>
> --
>
> 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.
>
> 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.h (linespec_lex_to_end): Declare.
> 	* location.c: New file.
> 	* location.h: New file.

Hi.
I'm going to try to minimize the number of requested changes.
I'd really like to get this checked in,
and I'm sure you would too. :-)

This patch is ok as is, though there is one change I would make.
This change *can* be deferred for later, but feel free to make it now.

The requested change is: Remove event_location_to_string_const
from location.h and make it static in location.c.
I didn't check any intermediate state of applying the patch series,
but after I apply all the patches event_location_to_string_const
is only used in location.c.

Later (not now, let's get this sucker checked in, unless of course
you really want to), let's make event_location_to_string take a
const struct event_location *, and make the cached copy
"mutable in the c++ sense".
  
Keith Seitz Aug. 10, 2015, 6:05 p.m. UTC | #2
On 08/10/2015 10:33 AM, Doug Evans wrote:
> Later (not now, let's get this sucker checked in, unless of course
> you really want to), let's make event_location_to_string take a
> const struct event_location *, and make the cached copy
> "mutable in the c++ sense".

Bah. You got me on that one. This was used as part of the
pending-location hack ("append extra_string to the location") in
create_breakpoint:

      if (extra_string != NULL)
        {
          char *new = xstrprintf ("%s %s",
                                  event_location_to_string_const (location),
                                  extra_string);

          set_event_location_string (b->location, new);
          xfree (new);
        }

... which was removed in this revision, and I forgot to check if it was
being used at all anymore.

Answer: No, it is not used anymore. I could just remove it entirely.
Which is what I've done in my local copy. [Do you want me to repost this?]

Thanks!

Keith
  
Doug Evans Aug. 10, 2015, 7:58 p.m. UTC | #3
Keith Seitz <keiths@redhat.com> writes:
> On 08/10/2015 10:33 AM, Doug Evans wrote:
>> Later (not now, let's get this sucker checked in, unless of course
>> you really want to), let's make event_location_to_string take a
>> const struct event_location *, and make the cached copy
>> "mutable in the c++ sense".
>
> Bah. You got me on that one. This was used as part of the
> pending-location hack ("append extra_string to the location") in
> create_breakpoint:
>
>       if (extra_string != NULL)
>         {
>           char *new = xstrprintf ("%s %s",
>                                   event_location_to_string_const (location),
>                                   extra_string);
>
>           set_event_location_string (b->location, new);
>           xfree (new);
>         }
>
> ... which was removed in this revision, and I forgot to check if it was
> being used at all anymore.
>
> Answer: No, it is not used anymore. I could just remove it entirely.
> Which is what I've done in my local copy. [Do you want me to repost this?]
>
> Thanks!
>
> Keith

Well, event_location_to_string_const is used internally in one location
in the v6 patch set, but if you've removed it completely that's fine by me.

As for reposting, in this particular case (2/9) I suppose you should,
but only for informational sake. No need for waiting for another review.
  

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 4080ba4..981b078 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -854,7 +854,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 \
@@ -940,7 +940,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 \
@@ -1023,7 +1023,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 2fe845f..5c4ed3f 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2435,6 +2435,54 @@  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;
+
+  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);
+
+  *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 <http://www.gnu.org/licenses/>.  */
+
+#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 <ctype.h>
+#include <string.h>
+
+/* 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..0e31c0a
--- /dev/null
+++ b/gdb/location.h
@@ -0,0 +1,106 @@ 
+/* 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 <http://www.gnu.org/licenses/>.  */
+
+#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);
+
+/* 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);
+
+/* Attempt to convert the input string in *ARGP into an event_location.
+   ARGP is advanced past any processed input.  Returns 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 */