public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA 4/9] Explicit locations v2 - Add address locations
@ 2014-05-08 18:01 Keith Seitz
  2014-05-30 18:18 ` Keith Seitz
  0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2014-05-08 18:01 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

Hi,

Hi,

Currently address locations ("*EXPR") are handled by the linespec parser.

This patch adds a new address location type to the API introduced in 
previous patches and moves the handling of this form from the linespec 
parser into the location API function string_to_event_location.

 From the command-writer's POV, nothing is required to support this. 
Once again, this is a elaboration of the location API to support 
existing features.

This patch also converts any hard-coded linespecs of the form "*EXPR" 
into proper address locations.

Keith

ChangeLog
2014-05-08  Keith Seitz  <keiths@redhat.com>

	* breakpoint.c (create_thread_event_breakpoint): Convert
	linespec to address location.
	(init_breakpoint_sal): Likewise.
	* linespec.c (canonicalize_linespec): Canonicalize expressions
	into address locations.
	(convert_address_location_to_sals): New function; contents moved
	from ...
	(convert_linespc_to_sals): ... here.
	(parse_linespec): Remove handling of address locations.
	(event_location_to_sals): Handle EVENT_LOCATION_ADDRESS.
	(linespec_expression_to_pc): Export.
	* linespec.h (linespec_expression_to_pc): Add declaration.
	* location.c (copy_event_location): Handle address locations.
	(delete_event_location): Likewise.
	(event_location_to_string): Likewise.
	(string_to_event_location): Likewise.
	(event_location_empty_p): Likewise.
	* location.h (enum event_location_type): Add EVENT_LOCATION_ADDRESS.
	(struct event_location.u) <address>: New member.
	(EVENT_LOCATION_ADDRESS): Define accessor macro.
	* python/py-finishbreakpoint.c (bpfinishpy_init): Remove local
	variable `finish_pc'.
	Convert linespec to address location.
	* spu-tdep.c (spu_catch_start): Convert linespec to address
	location.

[-- Attachment #2: explicit-address-locations.patch --]
[-- Type: text/x-patch, Size: 11456 bytes --]

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index de0f65a..f509d7a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7544,9 +7544,8 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b->enable_state = bp_enabled;
   /* location has to be used or breakpoint_re_set will delete me.  */
-  b->location = new_event_location (EVENT_LOCATION_LINESPEC);
-  EVENT_LOCATION_LINESPEC (b->location)
-    = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
+  b->location = new_event_location (EVENT_LOCATION_ADDRESS);
+  EVENT_LOCATION_ADDRESS (b->location) = b->loc->address;
 
   update_global_location_list_nothrow (1);
 
@@ -9430,11 +9429,10 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
     b->location = location;
   else
     {
-      b->location = new_event_location (EVENT_LOCATION_LINESPEC);
-      EVENT_LOCATION_LINESPEC (b->location)
-	= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
-      EVENT_LOCATION_SAVE_SPEC (b->location)
-	= xstrdup (EVENT_LOCATION_LINESPEC (b->location));
+      b->location = new_event_location (EVENT_LOCATION_ADDRESS);
+      EVENT_LOCATION_ADDRESS (b->location) = b->loc->address;
+      EVENT_LOCATION_SAVE_SPEC (b->location) =
+	xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
    }
   b->filter = filter;
 }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 717ea68..07a84b5 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -317,7 +317,7 @@ static void iterate_over_file_blocks (struct symtab *symtab,
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
+CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -1774,9 +1774,8 @@ canonicalize_linespec (struct linespec_state *state, linespec_p ls)
   if (ls->expression != NULL)
     {
       state->canonical->location
-	= new_event_location (EVENT_LOCATION_LINESPEC);
-      EVENT_LOCATION_LINESPEC (state->canonical->location)
-	= xstrdup (ls->expression);
+	= new_event_location (EVENT_LOCATION_ADDRESS);
+      EVENT_LOCATION_ADDRESS (state->canonical->location) = ls->expr_pc;
       EVENT_LOCATION_SAVE_SPEC (state->canonical->location)
 	= xstrdup (ls->expression);
     }
@@ -1997,6 +1996,30 @@ create_sals_line_offset (struct linespec_state *self,
   return values;
 }
 
+/* Convert the given ADDRESS into SaLs.  */
+
+static struct symtabs_and_lines
+convert_address_location_to_sals (struct linespec_state *self,
+				  CORE_ADDR address)
+{
+  struct symtab_and_line sal;
+  struct symtabs_and_lines sals = {NULL, 0};
+
+  sal = find_pc_line (address, 0);
+  sal.pc = address;
+  sal.section = find_pc_overlay (address);
+  sal.explicit_pc = 1;
+  add_sal_to_sals (self, &sals, &sal, core_addr_to_string (address), 1);
+
+  if (self->canonical != NULL)
+    {
+      self->canonical->location = new_event_location (EVENT_LOCATION_ADDRESS);
+      EVENT_LOCATION_ADDRESS (self->canonical->location) = address;
+    }
+
+  return sals;
+}
+
 /* Create and return SALs from the linespec LS.  */
 
 static struct symtabs_and_lines
@@ -2006,14 +2029,8 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 
   if (ls->expression != NULL)
     {
-      struct symtab_and_line sal;
-
       /* We have an expression.  No other attribute is allowed.  */
-      sal = find_pc_line (ls->expr_pc, 0);
-      sal.pc = ls->expr_pc;
-      sal.section = find_pc_overlay (ls->expr_pc);
-      sal.explicit_pc = 1;
-      add_sal_to_sals (state, &sals, &sal, ls->expression, 1);
+      sals = convert_address_location_to_sals (state, ls->expr_pc);
     }
   else if (ls->labels.label_symbols != NULL)
     {
@@ -2212,33 +2229,7 @@ parse_linespec (linespec_parser *parser, const char **argptr)
   token = linespec_lexer_lex_one (parser);
 
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
-  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
-    {
-      char *expr;
-      const char *copy;
-
-      /* User specified an expression, *EXPR.  */
-      copy = expr = copy_token_string (token);
-      cleanup = make_cleanup (xfree, expr);
-      PARSER_RESULT (parser)->expr_pc = linespec_expression_to_pc (&copy);
-      discard_cleanups (cleanup);
-      PARSER_RESULT (parser)->expression = expr;
-
-      /* This is a little hacky/tricky.  If linespec_expression_to_pc
-	 did not evaluate the entire token, then we must find the
-	 string COPY inside the original token buffer.  */
-      if (*copy != '\0')
-	{
-	  PARSER_STREAM (parser) = strstr (parser->lexer.saved_arg, copy);
-	  gdb_assert (PARSER_STREAM (parser) != NULL);
-	}
-
-      /* Consume the token.  */
-      linespec_lexer_consume_token (parser);
-
-      goto convert_to_sals;
-    }
-  else if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
+  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
     {
       char *var;
 
@@ -2460,6 +2451,12 @@ event_location_to_sals (linespec_parser *parser,
       }
       break;
 
+    case EVENT_LOCATION_ADDRESS:
+      result
+	= convert_address_location_to_sals (PARSER_STATE (parser),
+					    EVENT_LOCATION_ADDRESS (location));
+      break;
+
     default:
       gdb_assert_not_reached ("unhandled event location type");
     }
@@ -2669,7 +2666,7 @@ initialize_defaults (struct symtab **default_symtab, int *default_line)
 /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
    advancing EXP_PTR past any parsed text.  */
 
-static CORE_ADDR
+CORE_ADDR
 linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 658f2a1..df5820a 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -152,4 +152,8 @@ extern struct symtabs_and_lines decode_line_with_current_source (char *, int);
 
 extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int);
 
+/* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
+   advancing EXP_PTR past any parsed text.  */
+
+extern CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 #endif /* defined (LINESPEC_H) */
diff --git a/gdb/location.c b/gdb/location.c
index 3f50bed..22e7496 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -68,6 +68,10 @@ copy_event_location (const struct event_location *src)
       EVENT_LOCATION_LINESPEC (dst) = xstrdup (EVENT_LOCATION_LINESPEC (src));
       break;
 
+    case EVENT_LOCATION_ADDRESS:
+      EVENT_LOCATION_ADDRESS (dst) = EVENT_LOCATION_ADDRESS (src);
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -93,6 +97,10 @@ delete_event_location (void *data)
 	  xfree (EVENT_LOCATION_LINESPEC (location));
 	  break;
 
+	case EVENT_LOCATION_ADDRESS:
+	  /* Nothing to do.  */
+	  break;
+
 	default:
 	  gdb_assert_not_reached ("unknown event location type");
 	}
@@ -116,6 +124,10 @@ event_location_to_string (const struct event_location *location)
       result = EVENT_LOCATION_LINESPEC (location);
       break;
 
+    case EVENT_LOCATION_ADDRESS:
+      result = EVENT_LOCATION_SAVE_SPEC (location);
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -133,11 +145,26 @@ string_to_event_location (char **stringp,
 {
   struct event_location *location;
 
-  location = new_event_location (EVENT_LOCATION_LINESPEC);
-  if (*stringp != NULL)
+  /* First, check if the string is an address location.  */
+  if (*stringp != NULL && **stringp == '*')
     {
-      EVENT_LOCATION_LINESPEC (location) = xstrdup (*stringp);
-      *stringp += strlen (*stringp);
+      const char *arg, *orig;
+
+      orig = arg = *stringp;
+      location = new_event_location (EVENT_LOCATION_ADDRESS);
+      EVENT_LOCATION_SAVE_SPEC (location) = xstrdup (arg);
+      EVENT_LOCATION_ADDRESS (location) = linespec_expression_to_pc (&arg);
+      *stringp += arg - orig;
+    }
+  else
+    {
+      /* Everything else is a linespec.  */
+      location = new_event_location (EVENT_LOCATION_LINESPEC);
+      if (*stringp != NULL)
+	{
+	  EVENT_LOCATION_LINESPEC (location) = xstrdup (*stringp);
+	  *stringp += strlen (*stringp);
+	}
     }
 
   return location;
@@ -153,6 +180,9 @@ event_location_empty_p (const struct event_location *location)
     case EVENT_LOCATION_LINESPEC:
       return EVENT_LOCATION_LINESPEC (location) == NULL;
 
+    case EVENT_LOCATION_ADDRESS:
+      return 0;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
diff --git a/gdb/location.h b/gdb/location.h
index 83fc3a4..8575daf 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -27,7 +27,10 @@ struct language_defn;
 enum event_location_type
 {
   /* A traditional linespec.  */
-  EVENT_LOCATION_LINESPEC
+  EVENT_LOCATION_LINESPEC,
+
+  /* An address in the inferior.  */
+  EVENT_LOCATION_ADDRESS
 };
 
 /* An event location used to set a stop event in the inferior.
@@ -47,6 +50,10 @@ struct event_location
        probes.  */
     char *addr_string;
 #define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
+
+    /* An address in the inferior.  */
+    CORE_ADDR address;
+#define EVENT_LOCATION_ADDRESS(S) ((S)->u.address)
   } u;
 
   /* A string representation of how this location may be
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index b4db5ec..864b043 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -167,9 +167,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   struct frame_id frame_id;
   PyObject *internal = NULL;
   int internal_bp = 0;
-  CORE_ADDR finish_pc, pc;
+  CORE_ADDR pc;
   volatile struct gdb_exception except;
-  char small_buf[100];
   struct symbol *function;
 
   if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
@@ -291,10 +290,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
       struct event_location location;
 
       /* Set a breakpoint on the return address.  */
-      initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
-      finish_pc = get_frame_pc (prev_frame);
-      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc));
-      EVENT_LOCATION_LINESPEC (&location) = small_buf;
+      initialize_event_location (&location, EVENT_LOCATION_ADDRESS);
+      EVENT_LOCATION_ADDRESS (&location) = get_frame_pc (prev_frame);
       create_breakpoint (python_gdbarch,
                          &location, NULL, thread, NULL,
                          0,
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 43b4fec..2ced594 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2001,8 +2001,8 @@ spu_catch_start (struct objfile *objfile)
 
   /* Use a numerical address for the set_breakpoint command to avoid having
      the breakpoint re-set incorrectly.  */
-  initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
-  EVENT_LOCATION_LINESPEC (&location) = xstrdup (core_addr_to_string (pc));
+  initialize_event_location (&location, EVENT_LOCATION_ADDRESS);
+  EVENT_LOCATION_ADDRESS (&location) = pc;
   create_breakpoint (get_objfile_arch (objfile), &location,
 		     NULL /* cond_string */, -1 /* thread */,
 		     NULL /* extra_string */,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA 4/9] Explicit locations v2 - Add address locations
  2014-05-08 18:01 [RFA 4/9] Explicit locations v2 - Add address locations Keith Seitz
@ 2014-05-30 18:18 ` Keith Seitz
  2014-06-08 21:58   ` Doug Evans
  0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2014-05-30 18:18 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

This is an update of this patch based on changes from reviews of 
previous patches.

Keith

Changes since last version:
   - remove SAVE_SPEC stuff
   - canonicalize_linespec: remove expression stuff
   - ditto convert_linespec_to_sals
   - event_location_to_string_const: compute return value
     (aka remove SAVE_SPEC)
   - new_address_location

ChangeLog
2014-05-29  Keith Seitz  <keiths@redhat.com>

	* breakpoint.c (create_thread_event_breakpoint): Convert
	linespec to address location.
	(init_breakpoint_sal): Likewise.
	* linespec.c (canonicalize_linespec): Do not handle address
	locations here.
	(convert_address_location_to_sals): New function; contents moved
	from ...
	(convert_linespc_to_sals): ... here.
	(parse_linespec): Remove address locations from linespec grammar.
	Remove handling of address locations.
	(event_location_to_sals): Handle ADDRESS_LOCATION.
	(linespec_expression_to_pc): Export.
	* linespec.h (linespec_expression_to_pc): Add declaration.
	* location.c (copy_event_location): Handle address locations.
	(delete_event_location): Likewise.
	(event_location_to_string): Likewise.
	(string_to_event_location): Likewise.
	(event_location_empty_p): Likewise.
	* location.h (enum event_location_type): Add EVENT_LOCATION_ADDRESS.
	(struct event_location.u) <address>: New member.
	(ADDRESS_LOCATION): Define accessor macro.
	* python/py-finishbreakpoint.c (bpfinishpy_init): Remove local
	variable `finish_pc'.
	Convert linespec to address location.
	* spu-tdep.c (spu_catch_start): Convert linespec to address
	location.


[-- Attachment #2: explicit-address-locations-2.patch --]
[-- Type: text/x-patch, Size: 15203 bytes --]

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c4727da..ef60f44 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7538,9 +7538,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b->enable_state = bp_enabled;
   /* location has to be used or breakpoint_re_set will delete me.  */
-  b->location = new_linespec_location (NULL);
-  EVENT_LOCATION_LINESPEC (b->location)
-    = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
+  b->location = new_address_location (b->loc->address);
 
   update_global_location_list_nothrow (1);
 
@@ -9423,11 +9421,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
   if (location != NULL)
     b->location = location;
   else
-    {
-      b->location = new_linespec_location (NULL);
-      EVENT_LOCATION_LINESPEC (b->location)
-	= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
-   }
+    b->location = new_address_location (b->loc->address);
   b->filter = filter;
 }
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 92482b4..8ea0c7b 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -317,7 +317,7 @@ static void iterate_over_file_blocks (struct symtab *symtab,
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
+CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -1766,77 +1766,68 @@ linespec_parse_basic (linespec_parser *parser)
 static void
 canonicalize_linespec (struct linespec_state *state, linespec_p ls)
 {
+  struct ui_file *buf;
+  int need_colon = 0;
+
   /* If canonicalization was not requested, no need to do anything.  */
   if (!state->canonical)
     return;
 
-  /* Shortcut expressions, which can only appear by themselves.  */
-  if (ls->expression != NULL)
-    {
-      state->canonical->location
-	= new_linespec_location (ls->expression);
-    }
-  else
-    {
-      struct ui_file *buf;
-      int need_colon = 0;
+  buf = mem_fileopen ();
 
-      buf = mem_fileopen ();
+  state->canonical->location = new_linespec_location (NULL);
 
-      state->canonical->location = new_linespec_location (NULL);
+  if (ls->source_filename)
+    {
+      fputs_unfiltered (ls->source_filename, buf);
+      need_colon = 1;
+    }
 
-      if (ls->source_filename)
-	{
-	  fputs_unfiltered (ls->source_filename, buf);
-	  need_colon = 1;
-	}
+  if (ls->function_name)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
+      fputs_unfiltered (ls->function_name, buf);
+      need_colon = 1;
+    }
 
-      if (ls->function_name)
-	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-	  fputs_unfiltered (ls->function_name, buf);
-	  need_colon = 1;
-	}
+  if (ls->label_name)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
 
-      if (ls->label_name)
+      if (ls->function_name == NULL)
 	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-
-	  if (ls->function_name == NULL)
-	    {
-	      struct symbol *s;
-
-	      /* No function was specified, so add the symbol name.  */
-	      gdb_assert (ls->labels.function_symbols != NULL
-			  && (VEC_length (symbolp, ls->labels.function_symbols)
-			      == 1));
-	      s = VEC_index (symbolp, ls->labels.function_symbols, 0);
-	      fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf);
-	      fputc_unfiltered (':', buf);
-	    }
-
-	  fputs_unfiltered (ls->label_name, buf);
-	  need_colon = 1;
-	  state->canonical->special_display = 1;
+	  struct symbol *s;
+
+	  /* No function was specified, so add the symbol name.  */
+	  gdb_assert (ls->labels.function_symbols != NULL
+		      && (VEC_length (symbolp, ls->labels.function_symbols)
+			  == 1));
+	  s = VEC_index (symbolp, ls->labels.function_symbols, 0);
+	  fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf);
+	  fputc_unfiltered (':', buf);
 	}
 
-      if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN)
-	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-	  fprintf_filtered (buf, "%s%d",
-			    (ls->line_offset.sign == LINE_OFFSET_NONE ? ""
-			     : (ls->line_offset.sign
-				== LINE_OFFSET_PLUS ? "+" : "-")),
-			    ls->line_offset.offset);
-	}
+      fputs_unfiltered (ls->label_name, buf);
+      need_colon = 1;
+      state->canonical->special_display = 1;
+    }
 
-      EVENT_LOCATION_LINESPEC (state->canonical->location)
-	= ui_file_xstrdup (buf, NULL);
-      ui_file_delete (buf);
+  if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
+      fprintf_filtered (buf, "%s%d",
+			(ls->line_offset.sign == LINE_OFFSET_NONE ? ""
+			 : (ls->line_offset.sign
+			    == LINE_OFFSET_PLUS ? "+" : "-")),
+			ls->line_offset.offset);
     }
+
+  EVENT_LOCATION_LINESPEC (state->canonical->location)
+    = ui_file_xstrdup (buf, NULL);
+  ui_file_delete (buf);
 }
 
 /* Given a line offset in LS, construct the relevant SALs.  */
@@ -1990,6 +1981,27 @@ create_sals_line_offset (struct linespec_state *self,
   return values;
 }
 
+/* Convert the given ADDRESS into SaLs.  */
+
+static struct symtabs_and_lines
+convert_address_location_to_sals (struct linespec_state *self,
+				  CORE_ADDR address)
+{
+  struct symtab_and_line sal;
+  struct symtabs_and_lines sals = {NULL, 0};
+
+  sal = find_pc_line (address, 0);
+  sal.pc = address;
+  sal.section = find_pc_overlay (address);
+  sal.explicit_pc = 1;
+  add_sal_to_sals (self, &sals, &sal, core_addr_to_string (address), 1);
+
+  if (self->canonical != NULL)
+    self->canonical->location = new_address_location (address);
+
+  return sals;
+}
+
 /* Create and return SALs from the linespec LS.  */
 
 static struct symtabs_and_lines
@@ -1997,18 +2009,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 {
   struct symtabs_and_lines sals = {NULL, 0};
 
-  if (ls->expression != NULL)
-    {
-      struct symtab_and_line sal;
-
-      /* We have an expression.  No other attribute is allowed.  */
-      sal = find_pc_line (ls->expr_pc, 0);
-      sal.pc = ls->expr_pc;
-      sal.section = find_pc_overlay (ls->expr_pc);
-      sal.explicit_pc = 1;
-      add_sal_to_sals (state, &sals, &sal, ls->expression, 1);
-    }
-  else if (ls->labels.label_symbols != NULL)
+  if (ls->labels.label_symbols != NULL)
     {
       /* We have just a bunch of functions/methods or labels.  */
       int i;
@@ -2108,8 +2109,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 
    The basic grammar of linespecs:
 
-   linespec -> expr_spec | var_spec | basic_spec
-   expr_spec -> '*' STRING
+   linespec -> var_spec | basic_spec
    var_spec -> '$' (STRING | NUMBER)
 
    basic_spec -> file_offset_spec | function_spec | label_spec
@@ -2205,33 +2205,7 @@ parse_linespec (linespec_parser *parser, const char **argptr)
   token = linespec_lexer_lex_one (parser);
 
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
-  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
-    {
-      char *expr;
-      const char *copy;
-
-      /* User specified an expression, *EXPR.  */
-      copy = expr = copy_token_string (token);
-      cleanup = make_cleanup (xfree, expr);
-      PARSER_RESULT (parser)->expr_pc = linespec_expression_to_pc (&copy);
-      discard_cleanups (cleanup);
-      PARSER_RESULT (parser)->expression = expr;
-
-      /* This is a little hacky/tricky.  If linespec_expression_to_pc
-	 did not evaluate the entire token, then we must find the
-	 string COPY inside the original token buffer.  */
-      if (*copy != '\0')
-	{
-	  PARSER_STREAM (parser) = strstr (parser->lexer.saved_arg, copy);
-	  gdb_assert (PARSER_STREAM (parser) != NULL);
-	}
-
-      /* Consume the token.  */
-      linespec_lexer_consume_token (parser);
-
-      goto convert_to_sals;
-    }
-  else if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
+  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
     {
       char *var;
 
@@ -2453,6 +2427,12 @@ event_location_to_sals (linespec_parser *parser,
       }
       break;
 
+    case ADDRESS_LOCATION:
+      result
+	= convert_address_location_to_sals (PARSER_STATE (parser),
+					    EVENT_LOCATION_ADDRESS (location));
+      break;
+
     default:
       gdb_assert_not_reached ("unhandled event location type");
     }
@@ -2647,7 +2627,7 @@ initialize_defaults (struct symtab **default_symtab, int *default_line)
 /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
    advancing EXP_PTR past any parsed text.  */
 
-static CORE_ADDR
+CORE_ADDR
 linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 658f2a1..df5820a 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -152,4 +152,8 @@ extern struct symtabs_and_lines decode_line_with_current_source (char *, int);
 
 extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int);
 
+/* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
+   advancing EXP_PTR past any parsed text.  */
+
+extern CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 #endif /* defined (LINESPEC_H) */
diff --git a/gdb/location.c b/gdb/location.c
index 57900bf..3c430db 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -53,6 +53,20 @@ new_linespec_location (const char *linespec)
   return location;
 }
 
+/* Create a new address location.  The return result is malloc'd
+   and should be freed with delete_event_location.  */
+
+struct event_location *
+new_address_location (CORE_ADDR addr)
+{
+  struct event_location *location;
+
+  location = XNEW (struct event_location);
+  initialize_event_location (location, ADDRESS_LOCATION);
+  EVENT_LOCATION_ADDRESS (location) = addr;
+  return location;
+}
+
 /* Return a copy of the given SRC location.  */
 
 struct event_location *
@@ -71,6 +85,10 @@ copy_event_location (const struct event_location *src)
       EVENT_LOCATION_LINESPEC (dst) = xstrdup (EVENT_LOCATION_LINESPEC (src));
       break;
 
+    case ADDRESS_LOCATION:
+      EVENT_LOCATION_ADDRESS (dst) = EVENT_LOCATION_ADDRESS (src);
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -111,6 +129,10 @@ delete_event_location (struct event_location *location)
 	  xfree (EVENT_LOCATION_LINESPEC (location));
 	  break;
 
+	case ADDRESS_LOCATION:
+	  /* Nothing to do.  */
+	  break;
+
 	default:
 	  gdb_assert_not_reached ("unknown event location type");
 	}
@@ -138,6 +160,12 @@ event_location_to_string_const (const struct event_location *location)
 	result = xstrdup (EVENT_LOCATION_LINESPEC (location));
       break;
 
+    case ADDRESS_LOCATION:
+      result
+	= xstrprintf ("*%s",
+		      core_addr_to_string (EVENT_LOCATION_ADDRESS (location)));
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -171,9 +199,24 @@ string_to_event_location (char **stringp,
 {
   struct event_location *location;
 
-  location = new_linespec_location (*stringp);
-  if (*stringp != NULL)
-    *stringp += strlen (*stringp);
+  /* First, check if the string is an address location.  */
+  if (*stringp != NULL && **stringp == '*')
+    {
+      const char *arg, *orig;
+      CORE_ADDR addr;
+
+      orig = arg = *stringp;
+      addr = linespec_expression_to_pc (&arg);
+      location = new_address_location (addr);
+      *stringp += arg - orig;
+    }
+  else
+    {
+      /* Everything else is a linespec.  */
+      location = new_linespec_location (*stringp);
+      if (*stringp != NULL)
+	*stringp += strlen (*stringp);
+    }
 
   return location;
 }
@@ -188,6 +231,9 @@ event_location_empty_p (const struct event_location *location)
     case LINESPEC_LOCATION:
       return EVENT_LOCATION_LINESPEC (location) == NULL;
 
+    case ADDRESS_LOCATION:
+      return 0;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
diff --git a/gdb/location.h b/gdb/location.h
index 418e271..1daefb6 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -27,7 +27,10 @@ struct language_defn;
 enum event_location_type
 {
   /* A traditional linespec.  */
-  LINESPEC_LOCATION
+  LINESPEC_LOCATION,
+
+  /* An address in the inferior.  */
+  ADDRESS_LOCATION
 };
 
 /* An event location used to set a stop event in the inferior.
@@ -47,6 +50,10 @@ struct event_location
        probes.  */
     char *addr_string;
 #define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
+
+    /* An address in the inferior.  */
+    CORE_ADDR address;
+#define EVENT_LOCATION_ADDRESS(S) ((S)->u.address)
   } u;
 
   /* Cached string representation of this location.  This is used to
@@ -85,6 +92,12 @@ extern struct cleanup *
 extern struct event_location *
   new_linespec_location (const char *linespec);
 
+/* Create a new address location.  The return result is malloc'd
+   and should be freed with delete_event_location.  */
+
+extern struct event_location *
+  new_address_location (CORE_ADDR addr);
+
 /* Return a copy of the given SRC location.  */
 
 extern struct event_location *
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index c29037a..c97bb0a 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -167,9 +167,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   struct frame_id frame_id;
   PyObject *internal = NULL;
   int internal_bp = 0;
-  CORE_ADDR finish_pc, pc;
+  CORE_ADDR pc;
   volatile struct gdb_exception except;
-  char small_buf[100];
   struct symbol *function;
 
   if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
@@ -291,10 +290,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
       struct event_location location;
 
       /* Set a breakpoint on the return address.  */
-      initialize_event_location (&location, LINESPEC_LOCATION);
-      finish_pc = get_frame_pc (prev_frame);
-      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc));
-      EVENT_LOCATION_LINESPEC (&location) = small_buf;
+      initialize_event_location (&location, ADDRESS_LOCATION);
+      EVENT_LOCATION_ADDRESS (&location) = get_frame_pc (prev_frame);
       create_breakpoint (python_gdbarch,
                          &location, NULL, thread, NULL,
                          0,
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index fcfe066..4ff742f 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2001,8 +2001,8 @@ spu_catch_start (struct objfile *objfile)
 
   /* Use a numerical address for the set_breakpoint command to avoid having
      the breakpoint re-set incorrectly.  */
-  initialize_event_location (&location, LINESPEC_LOCATION);
-  EVENT_LOCATION_LINESPEC (&location) = xstrdup (core_addr_to_string (pc));
+  initialize_event_location (&location, ADDRESS_LOCATION);
+  EVENT_LOCATION_ADDRESS (&location) = pc;
   create_breakpoint (get_objfile_arch (objfile), &location,
 		     NULL /* cond_string */, -1 /* thread */,
 		     NULL /* extra_string */,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA 4/9] Explicit locations v2 - Add address locations
  2014-05-30 18:18 ` Keith Seitz
@ 2014-06-08 21:58   ` Doug Evans
  2014-06-09 18:12     ` Keith Seitz
  0 siblings, 1 reply; 6+ messages in thread
From: Doug Evans @ 2014-06-08 21:58 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On Fri, May 30, 2014 at 11:18 AM, Keith Seitz <keiths@redhat.com> wrote:
> This is an update of this patch based on changes from reviews of previous
> patches.
>
> Keith
>
> Changes since last version:
>   - remove SAVE_SPEC stuff
>   - canonicalize_linespec: remove expression stuff
>   - ditto convert_linespec_to_sals
>   - event_location_to_string_const: compute return value
>     (aka remove SAVE_SPEC)
>   - new_address_location
>
> ChangeLog
> 2014-05-29  Keith Seitz  <keiths@redhat.com>
>
>
>         * breakpoint.c (create_thread_event_breakpoint): Convert
>         linespec to address location.
>         (init_breakpoint_sal): Likewise.
>         * linespec.c (canonicalize_linespec): Do not handle address
>         locations here.
>
>         (convert_address_location_to_sals): New function; contents moved
>         from ...
>         (convert_linespc_to_sals): ... here.
>         (parse_linespec): Remove address locations from linespec grammar.
>
>         Remove handling of address locations.
>         (event_location_to_sals): Handle ADDRESS_LOCATION.
>
>         (linespec_expression_to_pc): Export.
>         * linespec.h (linespec_expression_to_pc): Add declaration.
>         * location.c (copy_event_location): Handle address locations.
>         (delete_event_location): Likewise.
>         (event_location_to_string): Likewise.
>         (string_to_event_location): Likewise.
>         (event_location_empty_p): Likewise.
>         * location.h (enum event_location_type): Add EVENT_LOCATION_ADDRESS.
>         (struct event_location.u) <address>: New member.
>         (ADDRESS_LOCATION): Define accessor macro.
>
>         * python/py-finishbreakpoint.c (bpfinishpy_init): Remove local
>         variable `finish_pc'.
>         Convert linespec to address location.
>         * spu-tdep.c (spu_catch_start): Convert linespec to address
>         location.
>

Hi.
I've given the series another read through.
Not as detailed a read through as I want to, but that'll have to wait for a bit.

I do have one comment that I thought I'd forward on now though ...

Maybe I'm asking for too much in a C-based world,
but IWBN if code like this was done differently.
It's not a blocker, but I would like your thoughts.

@@ -291,10 +290,8 @@ bpfinishpy_init (PyObject *self, PyObject *args,
PyObject *kwargs)
       struct event_location location;

       /* Set a breakpoint on the return address.  */
-      initialize_event_location (&location, LINESPEC_LOCATION);
-      finish_pc = get_frame_pc (prev_frame);
-      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc));
-      EVENT_LOCATION_LINESPEC (&location) = small_buf;
+      initialize_event_location (&location, ADDRESS_LOCATION);
+      EVENT_LOCATION_ADDRESS (&location) = get_frame_pc (prev_frame);
       create_breakpoint (python_gdbarch,
                          &location, NULL, thread, NULL,
                          0

[apologies if there's a cut-n-paste error there - I hope that comes through ok]

There's still too much implementation detail exposed here for my
taste, but this is C so we have to compromise.  In this case,
bpfinishpy_init has to take on the responsibility of knowing what
needs to be done to properly initialize an ADDRESS_LOCATION - we can't
call a constructor because the object is on the stack.
I think there are a few cases in the patch set like this, so IWBN, if
possible, to do something better here.

I can think of two solutions.  Maybe there's another even better one.

1) Have a "placement new"-like constructor akin to the existing
new_foo_location constructors we have that takes a pointer to the
object to initialize.
2) Always use the existing new_foo_location constructors and free the
object when done.

Thoughts?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA 4/9] Explicit locations v2 - Add address locations
  2014-06-08 21:58   ` Doug Evans
@ 2014-06-09 18:12     ` Keith Seitz
  2014-08-02 19:16       ` Doug Evans
  2014-09-03 19:32       ` Keith Seitz
  0 siblings, 2 replies; 6+ messages in thread
From: Keith Seitz @ 2014-06-09 18:12 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches@sourceware.org ml

On 06/08/2014 02:58 PM, Doug Evans wrote:

> I've given the series another read through.
> Not as detailed a read through as I want to, but that'll have to wait for a bit.
>

I really appreciate your efforts here. I owe you a beverage of your 
choice should we ever meet in person again!

> @@ -291,10 +290,8 @@ bpfinishpy_init (PyObject *self, PyObject *args,
> PyObject *kwargs)
>         struct event_location location;
>
>         /* Set a breakpoint on the return address.  */
> -      initialize_event_location (&location, LINESPEC_LOCATION);
> -      finish_pc = get_frame_pc (prev_frame);
> -      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc));
> -      EVENT_LOCATION_LINESPEC (&location) = small_buf;
> +      initialize_event_location (&location, ADDRESS_LOCATION);
> +      EVENT_LOCATION_ADDRESS (&location) = get_frame_pc (prev_frame);
>         create_breakpoint (python_gdbarch,
>                            &location, NULL, thread, NULL,
>                            0
> I can think of two solutions.  Maybe there's another even better one.
>
> 1) Have a "placement new"-like constructor akin to the existing
> new_foo_location constructors we have that takes a pointer to the
> object to initialize.
> 2) Always use the existing new_foo_location constructors and free the
> object when done.

My purpose on this was efficiency. I haven't a clue how often a python 
script could call this, so I opted to handle memory allocations a little 
more tightly. I have no real objections to either solution, but I guess 
I would opt for #2 since that both exists and is already widely used.

Do you want me to submit an update to the series for this, or shall I 
wait for more feedback?

Keith

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA 4/9] Explicit locations v2 - Add address locations
  2014-06-09 18:12     ` Keith Seitz
@ 2014-08-02 19:16       ` Doug Evans
  2014-09-03 19:32       ` Keith Seitz
  1 sibling, 0 replies; 6+ messages in thread
From: Doug Evans @ 2014-08-02 19:16 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On Mon, Jun 9, 2014 at 11:12 AM, Keith Seitz <keiths@redhat.com> wrote:
> On 06/08/2014 02:58 PM, Doug Evans wrote:
>
>> I've given the series another read through.
>> Not as detailed a read through as I want to, but that'll have to wait for
>> a bit.
>>
>
> I really appreciate your efforts here. I owe you a beverage of your choice
> should we ever meet in person again!

Got Guinness? :-)

>> @@ -291,10 +290,8 @@ bpfinishpy_init (PyObject *self, PyObject *args,
>> PyObject *kwargs)
>>         struct event_location location;
>>
>>         /* Set a breakpoint on the return address.  */
>> -      initialize_event_location (&location, LINESPEC_LOCATION);
>> -      finish_pc = get_frame_pc (prev_frame);
>> -      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string
>> (finish_pc));
>> -      EVENT_LOCATION_LINESPEC (&location) = small_buf;
>> +      initialize_event_location (&location, ADDRESS_LOCATION);
>> +      EVENT_LOCATION_ADDRESS (&location) = get_frame_pc (prev_frame);
>>         create_breakpoint (python_gdbarch,
>>                            &location, NULL, thread, NULL,
>>                            0
>> I can think of two solutions.  Maybe there's another even better one.
>>
>> 1) Have a "placement new"-like constructor akin to the existing
>> new_foo_location constructors we have that takes a pointer to the
>> object to initialize.
>> 2) Always use the existing new_foo_location constructors and free the
>> object when done.
>
>
> My purpose on this was efficiency. I haven't a clue how often a python
> script could call this, so I opted to handle memory allocations a little
> more tightly. I have no real objections to either solution, but I guess I
> would opt for #2 since that both exists and is already widely used.
>
> Do you want me to submit an update to the series for this, or shall I wait
> for more feedback?

When we last left our story I was about to leave for holidays, was
gone for most of June and half of July.  Vacation time is over, alas,
back to work ... :-)

No need to submit an updated series until I get any remaining comments
out for this series.
Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA 4/9] Explicit locations v2 - Add address locations
  2014-06-09 18:12     ` Keith Seitz
  2014-08-02 19:16       ` Doug Evans
@ 2014-09-03 19:32       ` Keith Seitz
  1 sibling, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2014-09-03 19:32 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On 06/09/2014 11:12 AM, Keith Seitz wrote:
> On 06/08/2014 02:58 PM, Doug Evans wrote:
>> I can think of two solutions.  Maybe there's another even better one.
>>
>> 1) Have a "placement new"-like constructor akin to the existing
>> new_foo_location constructors we have that takes a pointer to the
>> object to initialize.
>> 2) Always use the existing new_foo_location constructors and free the
>> object when done.
>

As I mentioned, I've implemented #2.

Keith

Changes since last revision:
   - Use functions instead of EVENT_LOCATION_* macros.
   - new_address_location/get_address_location.


[-- Attachment #2: explicit-address-locations.patch --]
[-- Type: text/x-patch, Size: 17006 bytes --]

gdb/ChangeLog:

	* breakpoint.c (create_thread_event_breakpoint): Convert
	linespec to address location.
	(init_breakpoint_sal): Likewise.
	* linespec.c (canonicalize_linespec): Do not handle address
	locations here.
	(convert_address_location_to_sals): New function; contents moved
	from ...
	(convert_linespc_to_sals): ... here.
	(parse_linespec): Remove address locations from linespec grammar.
	Remove handling of address locations.
	(event_location_to_sals): Handle ADDRESS_LOCATION.
	(linespec_expression_to_pc): Export.
	* linespec.h (linespec_expression_to_pc): Add declaration.
	* location.c (struct event_location.u) <address>: New member.
	(copy_event_location): Handle address locations.
	(delete_event_location): Likewise.
	(event_location_to_string): Likewise.
	(string_to_event_location): Likewise.
	(event_location_empty_p): Likewise.
	* location.h (enum event_location_type): Add ADDRESS_LOCATION.
	(new_address_location, get_address_location): New functions.
	* python/py-finishbreakpoint.c (bpfinishpy_init): Remove local
	variable `finish_pc'.
	Convert linespec to address location.
	* spu-tdep.c (spu_catch_start): Convert linespec to address
	location.
---
 gdb/breakpoint.c                 |   13 ---
 gdb/linespec.c                   |  177 +++++++++++++++++---------------------
 gdb/linespec.h                   |    4 +
 gdb/location.c                   |   64 +++++++++++++-
 gdb/location.h                   |   17 +++-
 gdb/python/py-finishbreakpoint.c |    7 --
 gdb/spu-tdep.c                   |    2 
 7 files changed, 164 insertions(+), 120 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 00046b0..395e0f4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7531,7 +7531,6 @@ delete_std_terminate_breakpoint (void)
 struct breakpoint *
 create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 {
-  char *tmp;
   struct breakpoint *b;
 
   b = create_internal_breakpoint (gdbarch, address, bp_thread_event,
@@ -7539,9 +7538,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b->enable_state = bp_enabled;
   /* location has to be used or breakpoint_re_set will delete me.  */
-  tmp = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
-  b->location = new_linespec_location (tmp);
-  xfree (tmp);
+  b->location = new_address_location (b->loc->address);
 
   update_global_location_list_nothrow (1);
 
@@ -9424,13 +9421,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
   if (location != NULL)
     b->location = location;
   else
-    {
-      char *tmp = xstrprintf ("*%s",
-			      paddress (b->loc->gdbarch, b->loc->address));
-
-      b->location = new_linespec_location (tmp);
-      xfree (tmp);
-   }
+    b->location = new_address_location (b->loc->address);
   b->filter = filter;
 }
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 6c2467e..7048d02 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -317,7 +317,7 @@ static void iterate_over_file_blocks (struct symtab *symtab,
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
+CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 
 static struct symtabs_and_lines decode_objc (struct linespec_state *self,
 					     linespec_p ls,
@@ -1767,77 +1767,67 @@ static void
 canonicalize_linespec (struct linespec_state *state, linespec_p ls)
 {
   char *tmp;
+  struct ui_file *buf;
+  int need_colon = 0;
 
   /* If canonicalization was not requested, no need to do anything.  */
   if (!state->canonical)
     return;
 
-  /* Shortcut expressions, which can only appear by themselves.  */
-  if (ls->expression != NULL)
+  buf = mem_fileopen ();
+
+  if (ls->source_filename)
     {
-      state->canonical->location
-	= new_linespec_location (ls->expression);
+      fputs_unfiltered (ls->source_filename, buf);
+      need_colon = 1;
     }
-  else
-    {
-      struct ui_file *buf;
-      int need_colon = 0;
 
-      buf = mem_fileopen ();
-
-      if (ls->source_filename)
-	{
-	  fputs_unfiltered (ls->source_filename, buf);
-	  need_colon = 1;
-	}
+  if (ls->function_name)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
+      fputs_unfiltered (ls->function_name, buf);
+      need_colon = 1;
+    }
 
-      if (ls->function_name)
-	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-	  fputs_unfiltered (ls->function_name, buf);
-	  need_colon = 1;
-	}
+  if (ls->label_name)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
 
-      if (ls->label_name)
+      if (ls->function_name == NULL)
 	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-
-	  if (ls->function_name == NULL)
-	    {
-	      struct symbol *s;
-
-	      /* No function was specified, so add the symbol name.  */
-	      gdb_assert (ls->labels.function_symbols != NULL
-			  && (VEC_length (symbolp, ls->labels.function_symbols)
-			      == 1));
-	      s = VEC_index (symbolp, ls->labels.function_symbols, 0);
-	      fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf);
-	      fputc_unfiltered (':', buf);
-	    }
-
-	  fputs_unfiltered (ls->label_name, buf);
-	  need_colon = 1;
-	  state->canonical->special_display = 1;
+	  struct symbol *s;
+
+	  /* No function was specified, so add the symbol name.  */
+	  gdb_assert (ls->labels.function_symbols != NULL
+		      && (VEC_length (symbolp, ls->labels.function_symbols)
+			  == 1));
+	  s = VEC_index (symbolp, ls->labels.function_symbols, 0);
+	  fputs_unfiltered (SYMBOL_NATURAL_NAME (s), buf);
+	  fputc_unfiltered (':', buf);
 	}
 
-      if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN)
-	{
-	  if (need_colon)
-	    fputc_unfiltered (':', buf);
-	  fprintf_filtered (buf, "%s%d",
-			    (ls->line_offset.sign == LINE_OFFSET_NONE ? ""
-			     : (ls->line_offset.sign
-				== LINE_OFFSET_PLUS ? "+" : "-")),
-			    ls->line_offset.offset);
-	}
+      fputs_unfiltered (ls->label_name, buf);
+      need_colon = 1;
+      state->canonical->special_display = 1;
+    }
 
-      tmp = ui_file_xstrdup (buf, NULL);
-      state->canonical->location = new_linespec_location (tmp);
-      xfree (tmp);
-      ui_file_delete (buf);
+  if (ls->line_offset.sign != LINE_OFFSET_UNKNOWN)
+    {
+      if (need_colon)
+	fputc_unfiltered (':', buf);
+      fprintf_filtered (buf, "%s%d",
+			(ls->line_offset.sign == LINE_OFFSET_NONE ? ""
+			 : (ls->line_offset.sign
+			    == LINE_OFFSET_PLUS ? "+" : "-")),
+			ls->line_offset.offset);
     }
+
+  tmp = ui_file_xstrdup (buf, NULL);
+  state->canonical->location = new_linespec_location (tmp);
+  xfree (tmp);
+  ui_file_delete (buf);
 }
 
 /* Given a line offset in LS, construct the relevant SALs.  */
@@ -1991,6 +1981,27 @@ create_sals_line_offset (struct linespec_state *self,
   return values;
 }
 
+/* Convert the given ADDRESS into SaLs.  */
+
+static struct symtabs_and_lines
+convert_address_location_to_sals (struct linespec_state *self,
+				  CORE_ADDR address)
+{
+  struct symtab_and_line sal;
+  struct symtabs_and_lines sals = {NULL, 0};
+
+  sal = find_pc_line (address, 0);
+  sal.pc = address;
+  sal.section = find_pc_overlay (address);
+  sal.explicit_pc = 1;
+  add_sal_to_sals (self, &sals, &sal, core_addr_to_string (address), 1);
+
+  if (self->canonical != NULL)
+    self->canonical->location = new_address_location (address);
+
+  return sals;
+}
+
 /* Create and return SALs from the linespec LS.  */
 
 static struct symtabs_and_lines
@@ -1998,18 +2009,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 {
   struct symtabs_and_lines sals = {NULL, 0};
 
-  if (ls->expression != NULL)
-    {
-      struct symtab_and_line sal;
-
-      /* We have an expression.  No other attribute is allowed.  */
-      sal = find_pc_line (ls->expr_pc, 0);
-      sal.pc = ls->expr_pc;
-      sal.section = find_pc_overlay (ls->expr_pc);
-      sal.explicit_pc = 1;
-      add_sal_to_sals (state, &sals, &sal, ls->expression, 1);
-    }
-  else if (ls->labels.label_symbols != NULL)
+  if (ls->labels.label_symbols != NULL)
     {
       /* We have just a bunch of functions/methods or labels.  */
       int i;
@@ -2109,8 +2109,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 
    The basic grammar of linespecs:
 
-   linespec -> expr_spec | var_spec | basic_spec
-   expr_spec -> '*' STRING
+   linespec -> var_spec | basic_spec
    var_spec -> '$' (STRING | NUMBER)
 
    basic_spec -> file_offset_spec | function_spec | label_spec
@@ -2206,33 +2205,7 @@ parse_linespec (linespec_parser *parser, const char **argptr)
   token = linespec_lexer_lex_one (parser);
 
   /* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER.  */
-  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '*')
-    {
-      char *expr;
-      const char *copy;
-
-      /* User specified an expression, *EXPR.  */
-      copy = expr = copy_token_string (token);
-      cleanup = make_cleanup (xfree, expr);
-      PARSER_RESULT (parser)->expr_pc = linespec_expression_to_pc (&copy);
-      discard_cleanups (cleanup);
-      PARSER_RESULT (parser)->expression = expr;
-
-      /* This is a little hacky/tricky.  If linespec_expression_to_pc
-	 did not evaluate the entire token, then we must find the
-	 string COPY inside the original token buffer.  */
-      if (*copy != '\0')
-	{
-	  PARSER_STREAM (parser) = strstr (parser->lexer.saved_arg, copy);
-	  gdb_assert (PARSER_STREAM (parser) != NULL);
-	}
-
-      /* Consume the token.  */
-      linespec_lexer_consume_token (parser);
-
-      goto convert_to_sals;
-    }
-  else if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
+  if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
     {
       char *var;
 
@@ -2454,6 +2427,12 @@ event_location_to_sals (linespec_parser *parser,
       }
       break;
 
+    case ADDRESS_LOCATION:
+      result
+	= convert_address_location_to_sals (PARSER_STATE (parser),
+					    get_address_location (location));
+      break;
+
     default:
       gdb_assert_not_reached ("unhandled event location type");
     }
@@ -2648,7 +2627,7 @@ initialize_defaults (struct symtab **default_symtab, int *default_line)
 /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
    advancing EXP_PTR past any parsed text.  */
 
-static CORE_ADDR
+CORE_ADDR
 linespec_expression_to_pc (const char **exp_ptr)
 {
   if (current_program_space->executing_startup)
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 658f2a1..df5820a 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -152,4 +152,8 @@ extern struct symtabs_and_lines decode_line_with_current_source (char *, int);
 
 extern struct symtabs_and_lines decode_line_with_last_displayed (char *, int);
 
+/* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR,
+   advancing EXP_PTR past any parsed text.  */
+
+extern CORE_ADDR linespec_expression_to_pc (const char **exp_ptr);
 #endif /* defined (LINESPEC_H) */
diff --git a/gdb/location.c b/gdb/location.c
index 8111d7c..9e86107 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -45,6 +45,10 @@ struct event_location
        probes.  */
     char *addr_string;
 #define EL_LINESPEC(PTR) ((PTR)->u.addr_string)
+
+    /* An address in the inferior.  */
+    CORE_ADDR address;
+#define EL_ADDRESS(PTR) (PTR)->u.address
   } u;
 
   /* Cached string representation of this location.  This is used, e.g., to
@@ -87,6 +91,28 @@ get_linespec_location (struct event_location *location)
 /* See description in location.h.  */
 
 struct event_location *
+new_address_location (CORE_ADDR addr)
+{
+  struct event_location *location;
+
+  location = XCNEW (struct event_location);
+  EL_TYPE (location) = ADDRESS_LOCATION;
+  EL_ADDRESS (location) = addr;
+  return location;
+}
+
+/* See description in location.h.  */
+
+CORE_ADDR
+get_address_location (struct event_location *location)
+{
+  gdb_assert (EL_TYPE (location) == ADDRESS_LOCATION);
+  return EL_ADDRESS (location);
+}
+
+/* See description in location.h.  */
+
+struct event_location *
 copy_event_location (const struct event_location *src)
 {
   struct event_location *dst;
@@ -103,6 +129,10 @@ copy_event_location (const struct event_location *src)
 	EL_LINESPEC (dst) = xstrdup (EL_LINESPEC (src));
       break;
 
+    case ADDRESS_LOCATION:
+      EL_ADDRESS (dst) = EL_ADDRESS (src);
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -143,6 +173,10 @@ delete_event_location (struct event_location *location)
 	  xfree (EL_LINESPEC (location));
 	  break;
 
+	case ADDRESS_LOCATION:
+	  /* Nothing to do.  */
+	  break;
+
 	default:
 	  gdb_assert_not_reached ("unknown event location type");
 	}
@@ -179,6 +213,12 @@ event_location_to_string_const (const struct event_location *location)
 	result = xstrdup (EL_LINESPEC (location));
       break;
 
+    case ADDRESS_LOCATION:
+      result
+	= xstrprintf ("*%s",
+		      core_addr_to_string (EL_ADDRESS (location)));
+      break;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
@@ -206,9 +246,24 @@ string_to_event_location (char **stringp,
 {
   struct event_location *location;
 
-  location = new_linespec_location (*stringp);
-  if (*stringp != NULL)
-    *stringp += strlen (*stringp);
+  /* First, check if the string is an address location.  */
+  if (*stringp != NULL && **stringp == '*')
+    {
+      const char *arg, *orig;
+      CORE_ADDR addr;
+
+      orig = arg = *stringp;
+      addr = linespec_expression_to_pc (&arg);
+      location = new_address_location (addr);
+      *stringp += arg - orig;
+    }
+  else
+    {
+      /* Everything else is a linespec.  */
+      location = new_linespec_location (*stringp);
+      if (*stringp != NULL)
+	*stringp += strlen (*stringp);
+    }
 
   return location;
 }
@@ -224,6 +279,9 @@ event_location_empty_p (const struct event_location *location)
       /* Linespecs are never "empty."  (NULL is a valid linespec)  */
       return 0;
 
+    case ADDRESS_LOCATION:
+      return 0;
+
     default:
       gdb_assert_not_reached ("unknown event location type");
     }
diff --git a/gdb/location.h b/gdb/location.h
index 705c83b..14aaffa 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -28,7 +28,10 @@ struct event_location;
 enum event_location_type
 {
   /* A traditional linespec.  */
-  LINESPEC_LOCATION
+  LINESPEC_LOCATION,
+
+  /* An address in the inferior.  */
+  ADDRESS_LOCATION
 };
 
 /* Return the type of the given event location.  */
@@ -64,6 +67,18 @@ extern struct event_location *
 extern char *
   get_linespec_location (struct event_location *location);
 
+/* Create a new address location.  The return result is malloc'd
+   and should be freed with delete_event_location.  */
+
+extern struct event_location *
+  new_address_location (CORE_ADDR addr);
+
+/* Return the address location (a CORE_ADDR) of the given event_location
+   (which must be of type ADDRESS_LOCATION).  */
+
+extern CORE_ADDR
+  get_address_location (struct event_location *location);
+
 /* Free an event location and any associated data.  */
 
 extern void delete_event_location (struct event_location *location);
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index d08e13a..f52b1db 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -167,9 +167,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   struct frame_id frame_id;
   PyObject *internal = NULL;
   int internal_bp = 0;
-  CORE_ADDR finish_pc, pc;
+  CORE_ADDR pc;
   volatile struct gdb_exception except;
-  char small_buf[100];
   struct symbol *function;
 
   if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
@@ -292,9 +291,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
       struct cleanup *back_to;
 
       /* Set a breakpoint on the return address.  */
-      finish_pc = get_frame_pc (prev_frame);
-      xsnprintf (small_buf, sizeof (small_buf), "*%s", hex_string (finish_pc));
-      location = new_linespec_location (small_buf);
+      location = new_address_location (get_frame_pc (prev_frame));
       back_to = make_cleanup_delete_event_location (location);
       create_breakpoint (python_gdbarch,
                          location, NULL, thread, NULL,
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index e20fea3..e566e9f 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2002,7 +2002,7 @@ spu_catch_start (struct objfile *objfile)
 
   /* Use a numerical address for the set_breakpoint command to avoid having
      the breakpoint re-set incorrectly.  */
-  location = new_linespec_location (core_addr_to_string (pc));
+  location = new_address_location (pc);
   back_to = make_cleanup_delete_event_location (location);
   create_breakpoint (get_objfile_arch (objfile), location,
 		     NULL /* cond_string */, -1 /* thread */,

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-03 19:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-08 18:01 [RFA 4/9] Explicit locations v2 - Add address locations Keith Seitz
2014-05-30 18:18 ` Keith Seitz
2014-06-08 21:58   ` Doug Evans
2014-06-09 18:12     ` Keith Seitz
2014-08-02 19:16       ` Doug Evans
2014-09-03 19:32       ` Keith Seitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).