This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 3/9] Explicit locations v2 - Locations refactor
- From: Doug Evans <dje at google dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Mon, 12 May 2014 17:37:41 -0700
- Subject: Re: [RFA 3/9] Explicit locations v2 - Locations refactor
- Authentication-results: sourceware.org; auth=none
- References: <536BC61D dot 30700 at redhat dot com>
Hi.
While I think I may have more comments for this part,
I need to send this out now.
I'll return to it later if I think of something.
I'll try to get to the other patches in the series this week.
Thanks!
Keith Seitz writes:
> Hi,
>
> This patch is the "big one." It changes the breakpoint methods and
> auxiliary functions (like decode_line_full) to take a "location" (struct
> event_location) instead of an address string.
>
> Unfortunately, this includes using the new location API (from the
> previous patch) to keep everything working regression-free, so it is a
> bit larger than I would have liked, but a lot of it is "simple"
> boilerplate for creating event locations in the various breakpoint commands.
>
> Once again, this patch only supports linespec locations. The following
> patches will introduce other location types (address, probe, explicit).
>
> This patch introduces no new tests. The current test suite does a
> superlative job of testing the status quo.
>
> Keith
>
> ChangeLog
> 2014-05-08 Keith Seitz <keiths@redhat.com>
>
> * ax-gdb.c: Include location.h.
> (agent_command_1)) Use linespec location instead of address
> string.
> * break-catch-throw.c: Include location.h
> (re_set_exception_catchpoint): Use linespec location instead
> of address string.
> * breakpoint.c: Include location.h.
> (create_overlay_event_breakpoint): Use linespec location
> location instead of address string.
s/location//
> (create_longjmp_master_breakpoint): Likewise.
> (create_std_terminate_master_breakpoint): Likewise.
> (create_exception_master_breakpoint): Likewise.
> (update_breakpoints_after_exec): Likewise.
> (print_breakpoint_location): Use locations and
> event_location_to_string.
> (print_one_breakpoint_location): Likewise.
> (init_raw_breakpoint_without_location): Initialize
> b->location.
> (create_thread_event_breakpoint): Use linespec location instead of
> address string.
> (init_breakpoint_sal): Likewise.
> Only save extra_string if it is non-NULL and not the empty string.
> Use event_location_to_string instead of `addr_string'.
> Constify `p'.
> Use skip_spaces_const/skip_space_const instead of non-const versions.
> Copy the location into the breakpoint.
> If LOCATION is NULL, save the breakpoint address as a linespec location
> instead of an address string.
> (create_breakpoint_sal): Change `addr_string' parameter to a struct
> event_location. All uses updated.
> (create_breakpoints_sal): Likewise for local variable `addr_string'.
> (parse_breakpoint_sals): Use locations instead of address strings.
> Gaurd against NULL address string when not using the default
Guard
> breakpoint.
> (create_breakpoint): Change `arg' to a struct event_location and
> rename.
> Remove `copy_arg' and `addr_start'.
> Save the SAVE_SPEC of the location for pending breakpoints.
> For non-linespec locations, pass `extra_string' to
> find_condition_and_thread.
> Copy the location inlto the breakpoint instead of an address string.
into
> If the breakpoint location's SAVE_SPEC is not set and there is a
> string representation, save it as the SAVE_SPEC.
> (break_command_1): Use string_to_event_location and pass this to
> create_breakpoint instead of an address string.
> Check against `arg_cp' for a probe linespec.
> (dprintf_command): Use string_to_event_location and pass this to
> create_breakpoint instead of an address string.
> (print_recreate_ranged_breakpoint): Use the location's SAVE_SPEC for
> recreating the breakpoint.
> (break_range_command): Use locations instead of address strings.
> (until_break_command): Likewise.
> (init_ada_exception_breakpoint): Likewise.
> (say_where): Likewise.
> (base_breakpoint_dtor): Delete `location' and `location_range_end' of
> the breakpoint.
> (base_breakpoint_create_sals_from_location): Use struct event_location
> instead of address string.
> Remove `addr_start' and `copy_arg' parameters.
> (base_breakpoint_decode_location): Use struct event_location instead of
> address string.
> (bkpt_re_set): Use locations instead of address strings.
> (bkpt_print_recreate): Use the location's SAVE_SPEC instead of
> an address string.
> (bkpt_create_sals_from_location): Use struct event_location instead of
> address string.
> (bkpt_probe_create_sals_from_location): Likewise.
> (bkpt_probe_decode_location): Use struct event_location instead of
> address string.
> (tracepoint_print_recreate): Use the location's SAVE_SPEC for
> recreating the tracepoint.
> (tracepoint_create_sals_from_location): Use struct event_location
> instead of address string.
> (tracepoint_decode_location): Likewise.
> (tracepoint_probe_decode_location): Likewise.
> (strace_marker_create_sals_from_location): Likewise.
> (strace_marker_create_breakpoints_sal): Likewise.
> (strace_marker_decode_location): Likewise.
> (location_to_sals): Likewise.
> For non-linespec locations, pass `extra_string' to
> find_condition_and_thread.
> When a pending breakpoint has been resolved, set a
> new SAVE_SPEC for the location.
> (breakpoint_re_set_default): Use locations instead of address
> strings.
> (create_sals_from_location_default): Use locations instead of
> address strings.
> (decode_location_default): Use locations instead of address strings.
> (trace_command): Use locations instead of address strings.
> (ftrace_command): Likewise.
> (strace_command): Likewise.
> (create_tracepoint_from_upload): Likewise.
> * breakpoint.h (struct breakpoint_ops) <create_sals_from_location>:
> Use struct event_location instead of address string.
> Update all uses.
> <decode_location>: Likewise.
> (struct breakpoint) <addr_string>: Change to struct event_location
> and rename `location'.
> <addr_string_range_end>: Change to struct event_location and reanme
rename
> `location_range_end'.
> (create_breakpoint): Change `arg' parameter to struct event_location
> and rename `location'.
> * cli/cli-cmds.c: Include location.h.
> (edit_command): Use locations instead of address strings.
> (list_command): Likewise.
> * elfread.c: Include location.h.
> (elf_gnu_ifunc_resolver_return_stop): Use event_location_to_string.
> * linespec.c: Include location.h.
> (canonicalize_linespec): Save a linespec location into `canonical'.
> Save a SAVE_SPEC into `canonical'.
> (linespec_parser_new): Initialize `parser'.
> (event_location_to_sals): New function.
> (decode_line_full): Change `argptr' to a struct event_location and
> rename it `location'.
> Use locations instead of address strings.
> Call event_location_to_sals instead of parse_linespec.
> (decode_line_1): Likewise.
> (decode_line_with_current_source): Use locations instead of
> address strings.
> (decode_line_with_last_displayed): Likewise.
> (decode_objc): Likewise.
> (destroy_linespec_result): Delete the linespec result's location
> instead of freeing the address string.
> * linespec.h (struct linespec_result) <addr_string>: Change to
> struct event_location and rename to ...
> <location>: ... this.
> (decode_line_1): Change `argptr' to struct event_location.
> All callers updated.
> (decode_line_full): Likewise.
> * mi/mi-cmd-break.c: Include langauge.h, location.h, and linespec.h.
language.h
> (mi_cmd_break_insert_1): Use locations instead of address strings.
> * probe.c: Include location.h.
> (parse_probes): Change `argptr' to struct event_location.
> Make local variable `arg_start' const.
> Use event locations instead of address strings.
> * probe.h (parse_probes): Change `argptr' to struct event_location.
> * python/py-breakpoint.c: Include location.h.
> (bppy_get_location): Constify local variable `str'.
> Use event_location_to_string.
> (bppy_init): Use locations instead of address strings.
> * python/py-finishbreakpoint.c: Include location.h.
> (bpfinishpy_init): Remove local variable `addr_str'.
> Use locations instead of address strings.
> * python/python.c: Include location.h.
> (gdbpy_decode_line): Use locations instead of address strings.
> * remote.c: Include location.h.
> (remote_download_tracepoint): Use locations instead of address
> strings.
> * spu-tdep.c: Include location.h.
> (spu_catch_start): Remove local variable `buf'.
> Use locations instead of address strings.
> * tracepoint.c: Include location.h.
> (scope_info): Use locations instead of address strings.
> (encode_source_string): Constify parameter `src'.
> * tracepoint.h (encode_source_string): Likewise.
> diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
> index b77716d..256187c 100644
> --- a/gdb/ax-gdb.c
> +++ b/gdb/ax-gdb.c
> @@ -41,6 +41,7 @@
> #include "arch-utils.h"
> #include "cli/cli-utils.h"
> #include "linespec.h"
> +#include "location.h"
> #include "objfiles.h"
>
> #include "valprint.h"
> @@ -2643,14 +2644,19 @@ agent_command_1 (char *exp, int eval)
> int ix;
> struct linespec_sals *iter;
> struct cleanup *old_chain;
> + struct event_location *location, copy_location;
>
> exp = skip_spaces (exp);
> init_linespec_result (&canonical);
> - decode_line_full (&exp, DECODE_LINE_FUNFIRSTLINE,
> + location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (location) = exp;
> + copy_location = *location;
As I'm reading this the ownership of the location string is murky.
I'm not sure how many event locations there'll be but I wonder
if it would be preferable to have a constructor for each kind,
and then the constructor for linespec locations can xstrdup
the string.
Plus you're making a copy with "copy_location = *location",
but locations have a (pseudo-)copy-constructor
(e.g., copy_event_location will xstrdup the linespec string).
> + old_chain = make_cleanup (delete_event_location, location);
> + decode_line_full (©_location, DECODE_LINE_FUNFIRSTLINE,
> (struct symtab *) NULL, 0, &canonical,
> NULL, NULL);
> - old_chain = make_cleanup_destroy_linespec_result (&canonical);
> - exp = skip_spaces (exp);
> + make_cleanup_destroy_linespec_result (&canonical);
> + exp = skip_spaces (EVENT_LOCATION_LINESPEC (©_location));
> if (exp[0] == ',')
> {
> exp++;
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index d0dacae..31cce47 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -36,6 +36,7 @@
> #include "cp-abi.h"
> #include "gdb_regex.h"
> #include "cp-support.h"
> +#include "location.h"
>
> /* Enums for exception-handling support. */
> enum exception_event_kind
> @@ -211,9 +212,12 @@ re_set_exception_catchpoint (struct breakpoint *self)
> /* We first try to use the probe interface. */
> TRY_CATCH (e, RETURN_MASK_ERROR)
> {
> - char *spec = ASTRDUP (exception_functions[kind].probe);
> + struct event_location location;
>
> - sals = parse_probes (&spec, NULL);
> + initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (&location)
> + = ASTRDUP (exception_functions[kind].probe);
Another case of murky ownership of the location string.
I see that it could work since one isn't going to call delete_event_location
here (which would xfree an ASTRDUP's string).
[And with c++ we could write code without having to worry about such things.]
I guess I don't have *that* strong an opinion on needing to change things here.
> + sals = parse_probes (&location, NULL);
> }
>
> if (e.reason < 0)
> @@ -224,9 +228,12 @@ re_set_exception_catchpoint (struct breakpoint *self)
> catchpoint mode. */
> TRY_CATCH (ex, RETURN_MASK_ERROR)
> {
> - char *spec = ASTRDUP (exception_functions[kind].function);
> + struct event_location location;
>
> - self->ops->decode_location (self, &spec, &sals);
> + initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (&location)
> + = ASTRDUP (exception_functions[kind].function);
> + self->ops->decode_location (self, &location, &sals);
> }
>
> /* NOT_FOUND_ERROR just means the breakpoint will be pending, so
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 3f089de..de0f65a 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -68,6 +68,7 @@
> #include "skip.h"
> #include "ax-gdb.h"
> #include "dummy-frame.h"
> +#include "location.h"
>
> #include "format.h"
>
> @@ -111,10 +112,10 @@ static int breakpoint_re_set_one (void *);
>
> static void breakpoint_re_set_default (struct breakpoint *);
>
> -static void create_sals_from_location_default (char **,
> - struct linespec_result *,
> - enum bptype, char *,
> - char **);
> +static void
> + create_sals_from_location_default (struct event_location *location,
> + struct linespec_result *canonical,
> + enum bptype type_wanted);
>
> static void create_breakpoints_sal_default (struct gdbarch *,
> struct linespec_result *,
> @@ -124,8 +125,9 @@ static void create_breakpoints_sal_default (struct gdbarch *,
> const struct breakpoint_ops *,
> int, int, int, unsigned);
>
> -static void decode_location_default (struct breakpoint *, char **,
> - struct symtabs_and_lines *);
> +static void decode_location_default (struct breakpoint *b,
> + struct event_location *location,
> + struct symtabs_and_lines *sals);
>
> static void clear_command (char *, int);
>
> @@ -3303,7 +3305,8 @@ create_overlay_event_breakpoint (void)
> b = create_internal_breakpoint (get_objfile_arch (objfile), addr,
> bp_overlay_event,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup (func_name);
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location) = xstrdup (func_name);
>
> if (overlay_debugging == ovly_auto)
> {
> @@ -3384,7 +3387,9 @@ create_longjmp_master_breakpoint (void)
> objfile),
> bp_longjmp_master,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup ("-probe-stap libc:longjmp");
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location)
> + = xstrdup ("-probe-stap libc:longjmp");
> b->enable_state = bp_disabled;
> }
>
> @@ -3421,7 +3426,8 @@ create_longjmp_master_breakpoint (void)
> addr = BMSYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]);
> b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup (func_name);
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
struct breakpoint now has "loc" and "location".
Yeah, I think we need some naming changes.
[not sure what to though]
> + EVENT_LOCATION_LINESPEC (b->location) = xstrdup (func_name);
> b->enable_state = bp_disabled;
> }
> }
> @@ -3477,7 +3483,8 @@ create_std_terminate_master_breakpoint (void)
> b = create_internal_breakpoint (get_objfile_arch (objfile), addr,
> bp_std_terminate_master,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup (func_name);
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location) = xstrdup (func_name);
I like the idea of a linespec-specific constructor the more I read this
pattern of new_event_location followed by setting EVENT_LOCATION_LINESPEC.
E.g.,
b->location = new_linespec_event_location (func_name);
or some such.
> b->enable_state = bp_disabled;
> }
> }
> @@ -3547,7 +3554,9 @@ create_exception_master_breakpoint (void)
> objfile),
> bp_exception_master,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup ("-probe-stap libgcc:unwind");
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location)
> + = xstrdup ("-probe-stap libgcc:unwind");
> b->enable_state = bp_disabled;
> }
>
> @@ -3580,7 +3589,8 @@ create_exception_master_breakpoint (void)
> ¤t_target);
> b = create_internal_breakpoint (gdbarch, addr, bp_exception_master,
> &internal_breakpoint_ops);
> - b->addr_string = xstrdup (func_name);
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location) = xstrdup (func_name);
> b->enable_state = bp_disabled;
> }
>
> @@ -3694,7 +3704,7 @@ update_breakpoints_after_exec (void)
> /* Without a symbolic address, we have little hope of the
> pre-exec() address meaning the same thing in the post-exec()
> a.out. */
> - if (b->addr_string == NULL)
> + if (event_location_empty_p (b->location))
> {
> delete_breakpoint (b);
> continue;
> @@ -5919,7 +5929,8 @@ print_breakpoint_location (struct breakpoint *b,
> set_current_program_space (loc->pspace);
>
> if (b->display_canonical)
> - ui_out_field_string (uiout, "what", b->addr_string);
> + ui_out_field_string (uiout, "what",
> + event_location_to_string (b->location));
> else if (loc && loc->symtab)
> {
> struct symbol *sym
> @@ -5955,7 +5966,8 @@ print_breakpoint_location (struct breakpoint *b,
> do_cleanups (stb_chain);
> }
> else
> - ui_out_field_string (uiout, "pending", b->addr_string);
> + ui_out_field_string (uiout, "pending",
> + event_location_to_string (b->location));
>
> if (loc && is_breakpoint (b)
> && breakpoint_condition_evaluation_mode () == condition_evaluation_target
> @@ -6422,8 +6434,10 @@ print_one_breakpoint_location (struct breakpoint *b,
>
> ui_out_field_string (uiout, "original-location", w->exp_string);
> }
> - else if (b->addr_string)
> - ui_out_field_string (uiout, "original-location", b->addr_string);
> + else if (b->location != NULL
> + && event_location_to_string (b->location) != NULL)
> + ui_out_field_string (uiout, "original-location",
> + event_location_to_string (b->location));
> }
> }
>
> @@ -7184,6 +7198,7 @@ init_raw_breakpoint_without_location (struct breakpoint *b,
> b->condition_not_parsed = 0;
> b->py_bp_object = NULL;
> b->related_breakpoint = b;
> + b->location = NULL;
> }
>
> /* Helper to set_raw_breakpoint below. Creates a breakpoint
> @@ -7528,8 +7543,9 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
> &internal_breakpoint_ops);
>
> b->enable_state = bp_enabled;
> - /* addr_string has to be used or breakpoint_re_set will delete me. */
> - b->addr_string
> + /* 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));
Using a linespec-specific constructor would be harder here though.
[absent c++]
Still, it feels warranted (i.e., delete new_event_location in favor
of constructors for each kind).
If using xstrprintf to get *address appears often enough one could
write a simpler wrapper.
Haven't seen the rest of the patchset though.
>
> update_global_location_list_nothrow (1);
> @@ -9269,13 +9285,14 @@ update_dprintf_commands (char *args, int from_tty,
> }
> }
>
> -/* Create a breakpoint with SAL as location. Use ADDR_STRING
> - as textual description of the location, and COND_STRING
> +/* Create a breakpoint with SAL as location. Use LOCATION
> + as a description of the location, and COND_STRING
> as condition expression. */
>
> static void
> init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
> - struct symtabs_and_lines sals, char *addr_string,
> + struct symtabs_and_lines sals,
> + struct event_location *location,
> char *filter, char *cond_string,
> char *extra_string,
> enum bptype type, enum bpdisp disposition,
> @@ -9324,7 +9341,10 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
> b->task = task;
>
> b->cond_string = cond_string;
> - b->extra_string = extra_string;
> + if (extra_string != NULL && *extra_string != '\0')
> + b->extra_string = extra_string;
> + else
> + b->extra_string = NULL;
> b->ignore_count = ignore_count;
> b->enable_state = enabled ? bp_enabled : bp_disabled;
> b->disposition = disposition;
> @@ -9341,13 +9361,13 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
> {
> /* We already know the marker exists, otherwise, we
> wouldn't see a sal for it. */
> - char *p = &addr_string[3];
> - char *endp;
> + const char *p = &event_location_to_string (b->location)[3];
> + const char *endp;
> char *marker_str;
>
> - p = skip_spaces (p);
> + p = skip_spaces_const (p);
>
> - endp = skip_to_space (p);
> + endp = skip_to_space_const (p);
>
> marker_str = savestring (p, endp - p);
> t->static_trace_marker_id = marker_str;
> @@ -9406,19 +9426,23 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
> }
>
> b->display_canonical = display_canonical;
> - if (addr_string)
> - b->addr_string = addr_string;
> + if (location != NULL)
> + b->location = location;
> else
> - /* addr_string has to be used or breakpoint_re_set will delete
> - me. */
> - b->addr_string
> - = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
> + {
> + 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->filter = filter;
> }
>
> static void
> create_breakpoint_sal (struct gdbarch *gdbarch,
> - struct symtabs_and_lines sals, char *addr_string,
> + struct symtabs_and_lines sals,
> + struct event_location *location,
> char *filter, char *cond_string,
> char *extra_string,
> enum bptype type, enum bpdisp disposition,
> @@ -9443,7 +9467,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
> old_chain = make_cleanup (xfree, b);
>
> init_breakpoint_sal (b, gdbarch,
> - sals, addr_string,
> + sals, location,
> filter, cond_string, extra_string,
> type, disposition,
> thread, task, ignore_count,
> @@ -9487,17 +9511,17 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
>
> for (i = 0; VEC_iterate (linespec_sals, canonical->sals, i, lsal); ++i)
> {
> - /* Note that 'addr_string' can be NULL in the case of a plain
> + /* Note that 'location' can be NULL in the case of a plain
> 'break', without arguments. */
> - char *addr_string = (canonical->addr_string
> - ? xstrdup (canonical->addr_string)
> - : NULL);
> + struct event_location *location
> + = (canonical->location != NULL
> + ? copy_event_location (canonical->location) : NULL);
> char *filter_string = lsal->canonical ? xstrdup (lsal->canonical) : NULL;
> - struct cleanup *inner = make_cleanup (xfree, addr_string);
> + struct cleanup *inner = make_cleanup (delete_event_location, location);
>
> make_cleanup (xfree, filter_string);
> create_breakpoint_sal (gdbarch, lsal->sals,
> - addr_string,
> + location,
> filter_string,
> cond_string, extra_string,
> type, disposition,
> @@ -9508,22 +9532,25 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
> }
> }
>
> -/* Parse ADDRESS which is assumed to be a SAL specification possibly
> +/* Parse LOCATION which is assumed to be a SAL specification possibly
> followed by conditionals. On return, SALS contains an array of SAL
> - addresses found. ADDR_STRING contains a vector of (canonical)
> - address strings. ADDRESS points to the end of the SAL.
> + addresses found. LOCATION points to the end of the SAL (for
> + linespec locations).
>
> The array and the line spec strings are allocated on the heap, it is
> the caller's responsibility to free them. */
>
> static void
> -parse_breakpoint_sals (char **address,
> +parse_breakpoint_sals (struct event_location *location,
> struct linespec_result *canonical)
> {
> + const char *address = event_location_to_string (location);
> +
> /* If no arg given, or if first arg is 'if ', use the default
> breakpoint. */
> - if ((*address) == NULL
> - || (strncmp ((*address), "if", 2) == 0 && isspace ((*address)[2])))
> + if (EVENT_LOCATION_TYPE (location) == EVENT_LOCATION_LINESPEC
> + && (address == NULL
> + || (strncmp (address, "if", 2) == 0 && isspace (address[2]))))
> {
> /* The last displayed codepoint, if it's valid, is our default breakpoint
> address. */
> @@ -9576,14 +9603,15 @@ parse_breakpoint_sals (char **address,
> may have a '+' or '-' succeeded by a '['. */
> if (last_displayed_sal_is_valid ()
> && (!cursal.symtab
> - || ((strchr ("+-", (*address)[0]) != NULL)
> - && ((*address)[1] != '['))))
> - decode_line_full (address, DECODE_LINE_FUNFIRSTLINE,
> + || (address != NULL
> + && strchr ("+-", address[0]) != NULL
> + && address[1] != '[')))
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE,
> get_last_displayed_symtab (),
> get_last_displayed_line (),
> canonical, NULL, NULL);
> else
> - decode_line_full (address, DECODE_LINE_FUNFIRSTLINE,
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE,
> cursal.symtab, cursal.line, canonical, NULL, NULL);
> }
> }
> @@ -9775,20 +9803,30 @@ decode_static_tracepoint_spec (char **arg_p)
> return sals;
> }
>
> -/* Set a breakpoint. This function is shared between CLI and MI
> - functions for setting a breakpoint. This function has two major
> - modes of operations, selected by the PARSE_ARG parameter. If
> - non-zero, the function will parse ARG, extracting location,
> - condition, thread and extra string. Otherwise, ARG is just the
> - breakpoint's location, with condition, thread, and extra string
> - specified by the COND_STRING, THREAD and EXTRA_STRING parameters.
> +/* Set a breakpoint. This function is shared between CLI and MI functions
> + for setting a breakpoint at LOCATION.
> +
> + This function has two major modes of operations, selected by the PARSE_ARG
> + parameter.
> +
> + If PARSE_ARG is zero, LOCATION is just the breakpoint's location,
> + with condition, thread, and extra string specified by the COND_STRING,
> + THREAD, and EXTRA_STRING parameters.
> +
> + If PARSE_ARG is non-zero and LOCATION is a linespec location,
> + this function will attempt to extract the location, condition, thread,
> + and extra string from the linespec stored in LOCATION.
> + For non-linespec locations EXTRA_STRING is parsed for condition, thread,
> + and extra string.
> +
> If INTERNAL is non-zero, the breakpoint number will be allocated
> - from the internal breakpoint count. Returns true if any breakpoint
> - was created; false otherwise. */
> + from the internal breakpoint count.
> +
> + Returns true if any breakpoint was created; false otherwise. */
Not a problem introduced by your patch of course, so don't change anything,
but boy this function's API feels overloaded (parse_arg == 0, != 0).
>
> int
> create_breakpoint (struct gdbarch *gdbarch,
> - char *arg, char *cond_string,
> + const struct event_location *location, char *cond_string,
> int thread, char *extra_string,
> int parse_arg,
> int tempflag, enum bptype type_wanted,
> @@ -9799,23 +9837,22 @@ create_breakpoint (struct gdbarch *gdbarch,
> unsigned flags)
> {
> volatile struct gdb_exception e;
> - char *copy_arg = NULL;
> - char *addr_start = arg;
> struct linespec_result canonical;
> struct cleanup *old_chain;
> struct cleanup *bkpt_chain = NULL;
> int pending = 0;
> int task = 0;
> int prev_bkpt_count = breakpoint_count;
> + struct event_location copy_location;
>
> gdb_assert (ops != NULL);
>
> init_linespec_result (&canonical);
>
> + copy_location = *location;
> TRY_CATCH (e, RETURN_MASK_ALL)
> {
> - ops->create_sals_from_location (&arg, &canonical, type_wanted,
> - addr_start, ©_arg);
> + ops->create_sals_from_location (©_location, &canonical, type_wanted);
> }
>
> /* If caller is interested in rc value from parse, set value. */
> @@ -9852,8 +9889,7 @@ create_breakpoint (struct gdbarch *gdbarch,
> {
> struct linespec_sals lsal;
>
> - copy_arg = xstrdup (addr_start);
> - lsal.canonical = xstrdup (copy_arg);
> + lsal.canonical = xstrdup (event_location_to_string (location));
> lsal.sals.nelts = 1;
> lsal.sals.sals = XNEW (struct symtab_and_line);
> init_sal (&lsal.sals.sals[0]);
> @@ -9907,11 +9943,20 @@ create_breakpoint (struct gdbarch *gdbarch,
> if (parse_arg)
> {
> char *rest;
> + const char *arg;
> struct linespec_sals *lsal;
>
> lsal = VEC_index (linespec_sals, canonical.sals, 0);
>
> - /* Here we only parse 'arg' to separate condition
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg = EVENT_LOCATION_LINESPEC (©_location);
> + else
> + {
> + arg = extra_string;
> + extra_string = NULL;
> + }
> +
> + /* Here we only parse the location to separate condition
> from thread number, so parsing in context of first
> sal is OK. When setting the breakpoint we'll
> re-parse it in context of each sal. */
> @@ -9927,17 +9972,19 @@ create_breakpoint (struct gdbarch *gdbarch,
> }
> else
> {
> - if (*arg != '\0')
> - error (_("Garbage '%s' at end of location"), arg);
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC
> + && *EVENT_LOCATION_LINESPEC (©_location) != '\0')
> + error (_("Garbage '%s' at end of location"),
> + EVENT_LOCATION_LINESPEC (©_location));
>
> /* Create a private copy of condition string. */
> - if (cond_string)
> + if (cond_string != NULL)
Some would flag the good though gratuitous change here as verboten.
I don't mind them too much in such a large patch.
> {
> cond_string = xstrdup (cond_string);
> make_cleanup (xfree, cond_string);
> }
> /* Create a private copy of any extra string. */
> - if (extra_string)
> + if (extra_string != NULL)
> {
> extra_string = xstrdup (extra_string);
> make_cleanup (xfree, extra_string);
> @@ -9954,8 +10001,6 @@ create_breakpoint (struct gdbarch *gdbarch,
> {
> struct breakpoint *b;
>
> - make_cleanup (xfree, copy_arg);
> -
> if (is_tracepoint_type (type_wanted))
> {
> struct tracepoint *t;
> @@ -9967,8 +10012,18 @@ create_breakpoint (struct gdbarch *gdbarch,
> b = XNEW (struct breakpoint);
>
> init_raw_breakpoint_without_location (b, gdbarch, type_wanted, ops);
> + b->location = copy_event_location (location);
> +
> + /* If the location has a string representation,
> + save it to the breakpoint's location save spec, since this
> + may be used to save the breakpoint to a file. */
> + if (EVENT_LOCATION_SAVE_SPEC (b->location) == NULL
> + && event_location_to_string (b->location) != NULL)
> + {
> + EVENT_LOCATION_SAVE_SPEC (b->location)
> + = xstrdup (event_location_to_string (b->location));
> + }
This feels a bit clumsy.
How about adding a location.c API function to set save_spec?
if (EVENT_LOCATION_SAVE_SPEC (b->location) == NULL)
set_event_location_save_spec (b->location);
or some such?
>
> - b->addr_string = copy_arg;
> if (parse_arg)
> b->cond_string = NULL;
> else
> @@ -9981,7 +10036,13 @@ create_breakpoint (struct gdbarch *gdbarch,
> }
> b->cond_string = cond_string;
> }
> - b->extra_string = NULL;
> + if (extra_string != NULL && *extra_string != '\0')
> + {
> + b->extra_string = xstrdup (extra_string);
> + make_cleanup (xfree, b->extra_string);
The cleanup here looks odd.
> + }
> + else
> + b->extra_string = NULL;
> b->ignore_count = ignore_count;
> b->disposition = tempflag ? disp_del : disp_donttouch;
> b->condition_not_parsed = 1;
> @@ -10028,16 +10089,21 @@ break_command_1 (char *arg, int flag, int from_tty)
> : bp_breakpoint);
> struct breakpoint_ops *ops;
> const char *arg_cp = arg;
> + struct event_location *location;
> + struct cleanup *cleanup;
> +
> + location = string_to_event_location (&arg, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
>
> /* Matching breakpoints on probes. */
> - if (arg && probe_linespec_to_ops (&arg_cp) != NULL)
> + if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
> ops = &bkpt_probe_breakpoint_ops;
> else
> ops = &bkpt_breakpoint_ops;
>
> create_breakpoint (get_current_arch (),
> - arg,
> - NULL, 0, NULL, 1 /* parse arg */,
> + location,
> + NULL, 0, arg, 1 /* parse arg */,
> tempflag, type_wanted,
> 0 /* Ignore count */,
> pending_break_support,
> @@ -10046,6 +10112,7 @@ break_command_1 (char *arg, int flag, int from_tty)
> 1 /* enabled */,
> 0 /* internal */,
> 0);
> + do_cleanups (cleanup);
> }
>
> /* Helper function for break_command_1 and disassemble_command. */
> @@ -10210,9 +10277,15 @@ stopat_command (char *arg, int from_tty)
> static void
> dprintf_command (char *arg, int from_tty)
> {
> + struct event_location *location;
> + struct cleanup *cleanup;
> +
> + location = string_to_event_location (&arg, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
> +
> create_breakpoint (get_current_arch (),
> - arg,
> - NULL, 0, NULL, 1 /* parse arg */,
> + location,
> + NULL, 0, arg, 1 /* parse arg */,
> 0, bp_dprintf,
> 0 /* Ignore count */,
> pending_break_support,
> @@ -10221,6 +10294,7 @@ dprintf_command (char *arg, int from_tty)
> 1 /* enabled */,
> 0 /* internal */,
> 0);
> + do_cleanups (cleanup);
> }
>
> static void
> @@ -10365,8 +10439,9 @@ print_mention_ranged_breakpoint (struct breakpoint *b)
> static void
> print_recreate_ranged_breakpoint (struct breakpoint *b, struct ui_file *fp)
> {
> - fprintf_unfiltered (fp, "break-range %s, %s", b->addr_string,
> - b->addr_string_range_end);
> + fprintf_unfiltered (fp, "break-range %s, %s",
> + EVENT_LOCATION_SAVE_SPEC (b->location),
> + EVENT_LOCATION_SAVE_SPEC (b->location_range_end));
> print_recreate_thread (b, fp);
> }
>
> @@ -10417,6 +10492,7 @@ break_range_command (char *arg, int from_tty)
> struct symtab_and_line sal_start, sal_end;
> struct cleanup *cleanup_bkpt;
> struct linespec_sals *lsal_start, *lsal_end;
> + struct event_location *start_location, *end_location, copy_location;
>
> /* We don't support software ranged breakpoints. */
> if (target_ranged_break_num_registers () < 0)
> @@ -10436,9 +10512,13 @@ break_range_command (char *arg, int from_tty)
> init_linespec_result (&canonical_start);
>
> arg_start = arg;
> - parse_breakpoint_sals (&arg, &canonical_start);
> -
> - cleanup_bkpt = make_cleanup_destroy_linespec_result (&canonical_start);
> + start_location = string_to_event_location (&arg, current_language);
> + cleanup_bkpt = make_cleanup (delete_event_location, start_location);
> + copy_location = *start_location;
> + parse_breakpoint_sals (©_location, &canonical_start);
> + make_cleanup_destroy_linespec_result (&canonical_start);
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg = EVENT_LOCATION_LINESPEC (©_location);
>
> if (arg[0] != ',')
> error (_("Too few arguments."));
> @@ -10468,7 +10548,10 @@ break_range_command (char *arg, int from_tty)
> symtab and line as the default symtab and line for the end of the
> range. This makes it possible to have ranges like "foo.c:27, +14",
> where +14 means 14 lines from the start location. */
> - decode_line_full (&arg, DECODE_LINE_FUNFIRSTLINE,
> + end_location = string_to_event_location (&arg, current_language);
> + make_cleanup (delete_event_location, end_location);
> + copy_location = *end_location;
Why is copy_location is needed here?
[and elsewhere too I guess - I see this pattern used a lot,
and I'm just wondering if there's a simplification possible here, dunno]
> + decode_line_full (©_location, DECODE_LINE_FUNFIRSTLINE,
> sal_start.symtab, sal_start.line,
> &canonical_end, NULL, NULL);
>
> @@ -10483,8 +10566,6 @@ break_range_command (char *arg, int from_tty)
> error (_("Cannot create a ranged breakpoint with multiple locations."));
>
> sal_end = lsal_end->sals.sals[0];
> - addr_string_end = savestring (arg_start, arg - arg_start);
> - make_cleanup (xfree, addr_string_end);
>
> end = find_breakpoint_range_end (sal_end);
> if (sal_start.pc > end)
> @@ -10511,8 +10592,8 @@ break_range_command (char *arg, int from_tty)
> set_breakpoint_count (breakpoint_count + 1);
> b->number = breakpoint_count;
> b->disposition = disp_donttouch;
> - b->addr_string = xstrdup (addr_string_start);
> - b->addr_string_range_end = xstrdup (addr_string_end);
> + b->location = copy_event_location (start_location);
> + b->location_range_end = copy_event_location (end_location);
> b->loc->length = length;
>
> do_cleanups (cleanup_bkpt);
> @@ -11632,26 +11713,34 @@ until_break_command (char *arg, int from_tty, int anywhere)
> struct frame_id caller_frame_id;
> struct breakpoint *breakpoint;
> struct breakpoint *breakpoint2 = NULL;
> - struct cleanup *old_chain;
> + struct cleanup *old_chain, *cleanup;
> int thread;
> struct thread_info *tp;
> + struct event_location *location, copy_location;
>
> clear_proceed_status ();
>
> /* Set a breakpoint where the user wants it and at return from
> this function. */
>
> + location = string_to_event_location (&arg, current_language);
> + copy_location = *location;
> + cleanup = make_cleanup (delete_event_location, location);
> +
> if (last_displayed_sal_is_valid ())
> - sals = decode_line_1 (&arg, DECODE_LINE_FUNFIRSTLINE,
> + sals = decode_line_1 (©_location, DECODE_LINE_FUNFIRSTLINE,
> get_last_displayed_symtab (),
> get_last_displayed_line ());
> else
> - sals = decode_line_1 (&arg, DECODE_LINE_FUNFIRSTLINE,
> + sals = decode_line_1 (©_location, DECODE_LINE_FUNFIRSTLINE,
> (struct symtab *) NULL, 0);
>
> if (sals.nelts != 1)
> error (_("Couldn't get information on specified line."));
>
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg = EVENT_LOCATION_LINESPEC (©_location);
A short comment explaining why setting arg here is necessary would help
me the reader. Looking at breakpoint.c I see it checking *arg for whether
there is junk at the end of the arguments, so now I'm wondering if this
part is correct.
> +
> sal = sals.sals[0];
> xfree (sals.sals); /* malloc'd, so freed. */
>
> @@ -11732,6 +11821,8 @@ until_break_command (char *arg, int from_tty, int anywhere)
> }
> else
> do_cleanups (old_chain);
> +
> + do_cleanups (cleanup);
> }
>
> /* This function attempts to parse an optional "if <cond>" clause
> @@ -11887,7 +11978,8 @@ init_ada_exception_breakpoint (struct breakpoint *b,
>
> b->enable_state = enabled ? bp_enabled : bp_disabled;
> b->disposition = tempflag ? disp_del : disp_donttouch;
> - b->addr_string = addr_string;
> + b->location = string_to_event_location (&addr_string,
> + language_def (language_ada));
> b->language = language_ada;
> }
>
> @@ -12853,7 +12945,8 @@ say_where (struct breakpoint *b)
> single string. */
> if (b->loc == NULL)
> {
> - printf_filtered (_(" (%s) pending."), b->addr_string);
> + printf_filtered (_(" (%s) pending."),
> + event_location_to_string (b->location));
> }
> else
> {
> @@ -12875,7 +12968,8 @@ say_where (struct breakpoint *b)
> /* This is not ideal, but each location may have a
> different file name, and this at least reflects the
> real situation somewhat. */
> - printf_filtered (": %s.", b->addr_string);
> + printf_filtered (": %s.",
> + event_location_to_string (b->location));
> }
>
> if (b->loc->next)
> @@ -12917,9 +13011,9 @@ base_breakpoint_dtor (struct breakpoint *self)
> decref_counted_command_line (&self->commands);
> xfree (self->cond_string);
> xfree (self->extra_string);
> - xfree (self->addr_string);
> xfree (self->filter);
> - xfree (self->addr_string_range_end);
> + delete_event_location (self->location);
> + delete_event_location (self->location_range_end);
Making delete_event_location type-safe and having another for
cleanups that takes a void * feels better.
> }
>
> static struct bp_location *
> @@ -13012,11 +13106,9 @@ base_breakpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
> }
>
> static void
> -base_breakpoint_create_sals_from_location (char **arg,
> +base_breakpoint_create_sals_from_location (struct event_location *location,
> struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start,
> - char **copy_arg)
> + enum bptype type_wanted)
> {
> internal_error_pure_virtual_called ();
> }
> @@ -13038,7 +13130,8 @@ base_breakpoint_create_breakpoints_sal (struct gdbarch *gdbarch,
> }
>
> static void
> -base_breakpoint_decode_location (struct breakpoint *b, char **s,
> +base_breakpoint_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> internal_error_pure_virtual_called ();
> @@ -13089,7 +13182,7 @@ static void
> bkpt_re_set (struct breakpoint *b)
> {
> /* FIXME: is this still reachable? */
> - if (b->addr_string == NULL)
> + if (event_location_empty_p (b->location))
> {
> /* Anything without a string can't be re-set. */
> delete_breakpoint (b);
> @@ -13228,18 +13321,17 @@ bkpt_print_recreate (struct breakpoint *tp, struct ui_file *fp)
> internal_error (__FILE__, __LINE__,
> _("unhandled breakpoint type %d"), (int) tp->type);
>
> - fprintf_unfiltered (fp, " %s", tp->addr_string);
> + fprintf_unfiltered (fp, " %s",
> + EVENT_LOCATION_SAVE_SPEC (tp->location));
> print_recreate_thread (tp, fp);
> }
>
> static void
> -bkpt_create_sals_from_location (char **arg,
> - struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> +bkpt_create_sals_from_location (struct event_location *location,
> + struct linespec_result *canonical,
> + enum bptype type_wanted)
> {
> - create_sals_from_location_default (arg, canonical, type_wanted,
> - addr_start, copy_arg);
> + create_sals_from_location_default (location, canonical, type_wanted);
> }
>
> static void
> @@ -13264,10 +13356,11 @@ bkpt_create_breakpoints_sal (struct gdbarch *gdbarch,
> }
>
> static void
> -bkpt_decode_location (struct breakpoint *b, char **s,
> +bkpt_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> - decode_location_default (b, s, sals);
> + decode_location_default (b, location, sals);
> }
>
> /* Virtual table for internal breakpoints. */
> @@ -13465,26 +13558,23 @@ bkpt_probe_remove_location (struct bp_location *bl)
> }
>
> static void
> -bkpt_probe_create_sals_from_location (char **arg,
> +bkpt_probe_create_sals_from_location (struct event_location *location,
> struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> + enum bptype type_wanted)
> {
> struct linespec_sals lsal;
>
> - lsal.sals = parse_probes (arg, canonical);
> -
> - *copy_arg = xstrdup (canonical->addr_string);
> - lsal.canonical = xstrdup (*copy_arg);
> -
> + lsal.sals = parse_probes (location, canonical);
> + lsal.canonical = xstrdup (event_location_to_string (canonical->location));
> VEC_safe_push (linespec_sals, canonical->sals, &lsal);
> }
>
> static void
> -bkpt_probe_decode_location (struct breakpoint *b, char **s,
> +bkpt_probe_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> - *sals = parse_probes (s, NULL);
> + *sals = parse_probes (location, NULL);
> if (!sals->sals)
> error (_("probe not found"));
> }
> @@ -13566,7 +13656,8 @@ tracepoint_print_recreate (struct breakpoint *self, struct ui_file *fp)
> internal_error (__FILE__, __LINE__,
> _("unhandled tracepoint type %d"), (int) self->type);
>
> - fprintf_unfiltered (fp, " %s", self->addr_string);
> + fprintf_unfiltered (fp, " %s",
> + EVENT_LOCATION_SAVE_SPEC (self->location));
> print_recreate_thread (self, fp);
>
> if (tp->pass_count)
> @@ -13574,13 +13665,11 @@ tracepoint_print_recreate (struct breakpoint *self, struct ui_file *fp)
> }
>
> static void
> -tracepoint_create_sals_from_location (char **arg,
> - struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> +tracepoint_create_sals_from_location (struct event_location *location,
> + struct linespec_result *canonical,
> + enum bptype type_wanted)
> {
> - create_sals_from_location_default (arg, canonical, type_wanted,
> - addr_start, copy_arg);
> + create_sals_from_location_default (location, canonical, type_wanted);
> }
>
> static void
> @@ -13605,10 +13694,11 @@ tracepoint_create_breakpoints_sal (struct gdbarch *gdbarch,
> }
>
> static void
> -tracepoint_decode_location (struct breakpoint *b, char **s,
> +tracepoint_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> - decode_location_default (b, s, sals);
> + decode_location_default (b, location, sals);
> }
>
> struct breakpoint_ops tracepoint_breakpoint_ops;
> @@ -13617,22 +13707,21 @@ struct breakpoint_ops tracepoint_breakpoint_ops;
> static probe. */
>
> static void
> -tracepoint_probe_create_sals_from_location (char **arg,
> +tracepoint_probe_create_sals_from_location (struct event_location *location,
> struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> + enum bptype type_wanted)
> {
> /* We use the same method for breakpoint on probes. */
> - bkpt_probe_create_sals_from_location (arg, canonical, type_wanted,
> - addr_start, copy_arg);
> + bkpt_probe_create_sals_from_location (location, canonical, type_wanted);
> }
>
> static void
> -tracepoint_probe_decode_location (struct breakpoint *b, char **s,
> +tracepoint_probe_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> /* We use the same method for breakpoint on probes. */
> - bkpt_probe_decode_location (b, s, sals);
> + bkpt_probe_decode_location (b, location, sals);
> }
>
> static struct breakpoint_ops tracepoint_probe_breakpoint_ops;
> @@ -13671,7 +13760,8 @@ dprintf_re_set (struct breakpoint *b)
> static void
> dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
> {
> - fprintf_unfiltered (fp, "dprintf %s%s", tp->addr_string,
> + fprintf_unfiltered (fp, "dprintf %s%s",
> + EVENT_LOCATION_SAVE_SPEC (tp->location),
> tp->extra_string);
> print_recreate_thread (tp, fp);
> }
> @@ -13718,19 +13808,25 @@ dprintf_after_condition_true (struct bpstats *bs)
> markers (`-m'). */
>
> static void
> -strace_marker_create_sals_from_location (char **arg,
> +strace_marker_create_sals_from_location (struct event_location *location,
> struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> + enum bptype type_wanted)
> {
> struct linespec_sals lsal;
> + char *arg_start, *arg;
>
> - lsal.sals = decode_static_tracepoint_spec (arg);
> + arg = arg_start = EVENT_LOCATION_LINESPEC (location);
> + lsal.sals = decode_static_tracepoint_spec (&arg);
>
> - *copy_arg = savestring (addr_start, *arg - addr_start);
> + if (canonical != NULL)
> + {
> + canonical->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (canonical->location)
> + = savestring (arg_start, arg - arg_start);
> + }
>
> - canonical->addr_string = xstrdup (*copy_arg);
> - lsal.canonical = xstrdup (*copy_arg);
> + EVENT_LOCATION_LINESPEC (location) = arg;
> + lsal.canonical = xstrdup (event_location_to_string (canonical->location));
> VEC_safe_push (linespec_sals, canonical->sals, &lsal);
> }
>
> @@ -13763,17 +13859,17 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
> struct symtabs_and_lines expanded;
> struct tracepoint *tp;
> struct cleanup *old_chain;
> - char *addr_string;
> + struct event_location *location;
>
> expanded.nelts = 1;
> expanded.sals = &lsal->sals.sals[i];
>
> - addr_string = xstrdup (canonical->addr_string);
> - old_chain = make_cleanup (xfree, addr_string);
> + location = copy_event_location (canonical->location);
> + old_chain = make_cleanup (delete_event_location, location);
>
> tp = XCNEW (struct tracepoint);
> init_breakpoint_sal (&tp->base, gdbarch, expanded,
> - addr_string, NULL,
> + location, NULL,
> cond_string, extra_string,
> type_wanted, disposition,
> thread, task, ignore_count, ops,
> @@ -13794,12 +13890,14 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
> }
>
> static void
> -strace_marker_decode_location (struct breakpoint *b, char **s,
> +strace_marker_decode_location (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> struct tracepoint *tp = (struct tracepoint *) b;
> + char *s = EVENT_LOCATION_LINESPEC (location);
IWBN if this could be const char *.
If that requires too many further changes,
disregard and leave for another day.
>
> - *sals = decode_static_tracepoint_spec (s);
> + *sals = decode_static_tracepoint_spec (&s);
> if (sals->nelts > tp->static_trace_marker_id_idx)
> {
> sals->sals[0] = sals->sals[tp->static_trace_marker_id_idx];
> @@ -14175,10 +14273,12 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
> b->loc->line_number = sal2.line;
> b->loc->symtab = sym != NULL ? sal2.symtab : NULL;
>
> - xfree (b->addr_string);
> - b->addr_string = xstrprintf ("%s:%d",
> - symtab_to_filename_for_display (sal2.symtab),
> - b->loc->line_number);
> + delete_event_location (b->location);
> + b->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (b->location)
> + = xstrprintf ("%s:%d",
> + symtab_to_filename_for_display (sal2.symtab),
> + b->loc->line_number);
>
> /* Might be nice to check if function changed, and warn if
> so. */
> @@ -14339,22 +14439,24 @@ update_breakpoint_locations (struct breakpoint *b,
> update_global_location_list (1);
> }
>
> -/* Find the SaL locations corresponding to the given ADDR_STRING.
> +/* Find the SaL locations corresponding to the given LOCATION.
> On return, FOUND will be 1 if any SaL was found, zero otherwise. */
>
> static struct symtabs_and_lines
> -addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found)
> +location_to_sals (struct breakpoint *b, struct event_location *location,
> + int *found)
> {
> - char *s;
> struct symtabs_and_lines sals = {0};
> volatile struct gdb_exception e;
> + const char *prev;
>
> gdb_assert (b->ops != NULL);
> - s = addr_string;
> +
> + prev = event_location_to_string (location);
>
> TRY_CATCH (e, RETURN_MASK_ERROR)
> {
> - b->ops->decode_location (b, &s, &sals);
> + b->ops->decode_location (b, location, &sals);
> }
> if (e.reason < 0)
> {
> @@ -14389,24 +14491,54 @@ addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found)
> if (e.reason == 0 || e.error != NOT_FOUND_ERROR)
> {
> int i;
> + const char *s;
>
> for (i = 0; i < sals.nelts; ++i)
> resolve_sal_pc (&sals.sals[i]);
> - if (b->condition_not_parsed && s && s[0])
> + if (b->condition_not_parsed)
> {
> - char *cond_string, *extra_string;
> - int thread, task;
> + if (EVENT_LOCATION_TYPE (location) == EVENT_LOCATION_LINESPEC)
> + s = EVENT_LOCATION_LINESPEC (location);
> + else
> + s = b->extra_string;
>
> - find_condition_and_thread (s, sals.sals[0].pc,
> - &cond_string, &thread, &task,
> - &extra_string);
> - if (cond_string)
> - b->cond_string = cond_string;
> - b->thread = thread;
> - b->task = task;
> - if (extra_string)
> - b->extra_string = extra_string;
> - b->condition_not_parsed = 0;
> + if (s != NULL && *s != '\0')
> + {
> + char *cond_string, *extra_string;
> + int thread, task;
> + const char *orig = s;
> +
> + find_condition_and_thread (s, sals.sals[0].pc,
> + &cond_string, &thread, &task,
> + &extra_string);
> + if (cond_string != NULL)
> + b->cond_string = cond_string;
> + b->thread = thread;
> + b->task = task;
> + if (extra_string != NULL)
> + {
> + xfree (b->extra_string);
> + b->extra_string = extra_string;
> + }
> + b->condition_not_parsed = 0;
> +
The following is unexpected extra logic.
Can you elaborate on why it's needed?
[Not in comments in the code, unless you want to.
Just in email is fine with me.]
> + /* For pending breakpoints which were just resolved, reset the
> + location's save spec. */
> + if (b->loc == NULL)
> + {
> + size_t len;
> + char *p, *str;
> + char *old = EVENT_LOCATION_SAVE_SPEC (location);
> +
> + len = orig - prev;
> +
> + str = savestring (prev, len);
> + p = remove_trailing_whitespace (str, str + len);
> + *p = '\0';
> + EVENT_LOCATION_SAVE_SPEC (b->location) = str;
> + xfree (old);
> + }
> + }
> }
>
> if (b->type == bp_static_tracepoint && !strace_marker_p (b))
> @@ -14431,17 +14563,20 @@ breakpoint_re_set_default (struct breakpoint *b)
> struct symtabs_and_lines sals, sals_end;
> struct symtabs_and_lines expanded = {0};
> struct symtabs_and_lines expanded_end = {0};
> + struct event_location copy_location;
>
> - sals = addr_string_to_sals (b, b->addr_string, &found);
> + copy_location = *b->location;
> + sals = location_to_sals (b, ©_location, &found);
> if (found)
> {
> make_cleanup (xfree, sals.sals);
> expanded = sals;
> }
>
> - if (b->addr_string_range_end)
> + if (b->location_range_end != NULL)
> {
> - sals_end = addr_string_to_sals (b, b->addr_string_range_end, &found);
> + copy_location = *b->location_range_end;
> + sals_end = location_to_sals (b, ©_location, &found);
> if (found)
> {
> make_cleanup (xfree, sals_end.sals);
> @@ -14456,12 +14591,11 @@ breakpoint_re_set_default (struct breakpoint *b)
> calls parse_breakpoint_sals. Return 1 for success, zero for failure. */
>
> static void
> -create_sals_from_location_default (char **arg,
> - struct linespec_result *canonical,
> - enum bptype type_wanted,
> - char *addr_start, char **copy_arg)
> +create_sals_from_location_default (struct event_location *location,
> + struct linespec_result *canonical,
> + enum bptype type_wanted)
> {
> - parse_breakpoint_sals (arg, canonical);
> + parse_breakpoint_sals (location, canonical);
> }
>
> /* Call create_breakpoints_sal for the given arguments. This is the default
> @@ -14492,13 +14626,14 @@ create_breakpoints_sal_default (struct gdbarch *gdbarch,
> default function for the `decode_location' method of breakpoint_ops. */
>
> static void
> -decode_location_default (struct breakpoint *b, char **s,
> +decode_location_default (struct breakpoint *b,
> + struct event_location *location,
> struct symtabs_and_lines *sals)
> {
> struct linespec_result canonical;
>
> init_linespec_result (&canonical);
> - decode_line_full (s, DECODE_LINE_FUNFIRSTLINE,
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE,
> (struct symtab *) NULL, 0,
> &canonical, multiple_symbols_all,
> b->filter);
> @@ -15336,16 +15471,20 @@ static void
> trace_command (char *arg, int from_tty)
> {
> struct breakpoint_ops *ops;
> + struct event_location *location;
> + struct cleanup *back_to;
> const char *arg_cp = arg;
>
> - if (arg && probe_linespec_to_ops (&arg_cp))
> + location = string_to_event_location (&arg, current_language);
> + back_to = make_cleanup (delete_event_location, location);
> + if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
extra space after NULL
> ops = &tracepoint_probe_breakpoint_ops;
> else
> ops = &tracepoint_breakpoint_ops;
>
> create_breakpoint (get_current_arch (),
> - arg,
> - NULL, 0, NULL, 1 /* parse arg */,
> + location,
> + NULL, 0, arg, 1 /* parse arg */,
> 0 /* tempflag */,
> bp_tracepoint /* type_wanted */,
> 0 /* Ignore count */,
> @@ -15354,14 +15493,20 @@ trace_command (char *arg, int from_tty)
> from_tty,
> 1 /* enabled */,
> 0 /* internal */, 0);
> + do_cleanups (back_to);
> }
>
> static void
> ftrace_command (char *arg, int from_tty)
> {
> + struct event_location *location;
> + struct cleanup *back_to;
> +
> + location = string_to_event_location (&arg, current_language);
> + back_to = make_cleanup (delete_event_location, location);
> create_breakpoint (get_current_arch (),
> - arg,
> - NULL, 0, NULL, 1 /* parse arg */,
> + location,
> + NULL, 0, arg, 1 /* parse arg */,
> 0 /* tempflag */,
> bp_fast_tracepoint /* type_wanted */,
> 0 /* Ignore count */,
> @@ -15370,6 +15515,7 @@ ftrace_command (char *arg, int from_tty)
> from_tty,
> 1 /* enabled */,
> 0 /* internal */, 0);
> + do_cleanups (back_to);
> }
>
> /* strace command implementation. Creates a static tracepoint. */
> @@ -15378,17 +15524,27 @@ static void
> strace_command (char *arg, int from_tty)
> {
> struct breakpoint_ops *ops;
> + struct event_location *location;
> + struct cleanup *back_to;
>
> /* Decide if we are dealing with a static tracepoint marker (`-m'),
> or with a normal static tracepoint. */
> if (arg && strncmp (arg, "-m", 2) == 0 && isspace (arg[2]))
> - ops = &strace_marker_breakpoint_ops;
> + {
> + ops = &strace_marker_breakpoint_ops;
> + location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (location) = xstrdup (arg);
It's weird storing "-m " as part of the linespec.
Is this addressed in a later patch maybe?
> + }
> else
> - ops = &tracepoint_breakpoint_ops;
> + {
> + ops = &tracepoint_breakpoint_ops;
> + location = string_to_event_location (&arg, current_language);
> + }
>
> + back_to = make_cleanup (delete_event_location, location);
> create_breakpoint (get_current_arch (),
> - arg,
> - NULL, 0, NULL, 1 /* parse arg */,
> + location,
> + NULL, 0, arg, 1 /* parse arg */,
> 0 /* tempflag */,
> bp_static_tracepoint /* type_wanted */,
> 0 /* Ignore count */,
> @@ -15397,6 +15553,7 @@ strace_command (char *arg, int from_tty)
> from_tty,
> 1 /* enabled */,
> 0 /* internal */, 0);
> + do_cleanups (back_to);
> }
>
> /* Set up a fake reader function that gets command lines from a linked
> @@ -15428,6 +15585,8 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
> {
> char *addr_str, small_buf[100];
> struct tracepoint *tp;
> + struct event_location *location;
> + struct cleanup *cleanup;
>
> if (utp->at_string)
> addr_str = utp->at_string;
> @@ -15450,9 +15609,11 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
> "has no source form, ignoring it"),
> utp->number);
>
> + location = string_to_event_location (&addr_str, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
> if (!create_breakpoint (get_current_arch (),
> - addr_str,
> - utp->cond_string, -1, NULL,
> + location,
> + utp->cond_string, -1, addr_str,
> 0 /* parse cond/thread */,
> 0 /* tempflag */,
> utp->type /* type_wanted */,
> @@ -15463,7 +15624,12 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
> utp->enabled /* enabled */,
> 0 /* internal */,
> CREATE_BREAKPOINT_FLAGS_INSERTED))
> - return NULL;
> + {
> + do_cleanups (cleanup);
> + return NULL;
> + }
> +
> + do_cleanups (cleanup);
>
> /* Get the tracepoint we just created. */
> tp = get_tracepoint (tracepoint_count);
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index b23d2eb..879264a 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -37,6 +37,7 @@ struct bpstats;
> struct bp_location;
> struct linespec_result;
> struct linespec_sals;
> +struct event_location;
>
> /* This is the maximum number of bytes a breakpoint instruction can
> take. Feel free to increase it. It's just used in a few places to
> @@ -561,8 +562,9 @@ struct breakpoint_ops
> `create_sals_from_location_default'.
>
> This function is called inside `create_breakpoint'. */
> - void (*create_sals_from_location) (char **, struct linespec_result *,
> - enum bptype, char *, char **);
> + void (*create_sals_from_location) (struct event_location *location,
> + struct linespec_result *canonical,
> + enum bptype type_wanted);
>
> /* This method will be responsible for creating a breakpoint given its SALs.
> Usually, it just calls `create_breakpoints_sal' (for ordinary
> @@ -583,8 +585,9 @@ struct breakpoint_ops
> it calls `decode_line_full'.
>
> This function is called inside `location_to_sals'. */
> - void (*decode_location) (struct breakpoint *, char **,
> - struct symtabs_and_lines *);
> + void (*decode_location) (struct breakpoint *b,
> + struct event_location *location,
> + struct symtabs_and_lines *sals);
>
> /* Return true if this breakpoint explains a signal. See
> bpstat_explains_signal. */
> @@ -683,17 +686,17 @@ struct breakpoint
> non-thread-specific ordinary breakpoints this is NULL. */
> struct program_space *pspace;
>
> - /* String we used to set the breakpoint (malloc'd). */
> - char *addr_string;
> + /* Location we used to set the breakpoint (malloc'd). */
> + struct event_location *location;
>
> /* The filter that should be passed to decode_line_full when
> re-setting this breakpoint. This may be NULL, but otherwise is
> allocated with xmalloc. */
> char *filter;
>
> - /* For a ranged breakpoint, the string we used to find
> + /* For a ranged breakpoint, the location we used to find
> the end of the range (malloc'd). */
> - char *addr_string_range_end;
> + struct event_location *location_range_end;
>
> /* Architecture we used to set the breakpoint. */
> struct gdbarch *gdbarch;
> @@ -1263,7 +1266,8 @@ enum breakpoint_create_flags
> CREATE_BREAKPOINT_FLAGS_INSERTED = 1 << 0
> };
>
> -extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
> +extern int create_breakpoint (struct gdbarch *gdbarch,
> + const struct event_location *location,
> char *cond_string, int thread,
> char *extra_string,
> int parse_arg,
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index a5ef9c6..bb54c76 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -40,6 +40,7 @@
> #include "disasm.h"
> #include "tracepoint.h"
> #include "filestuff.h"
> +#include "location.h"
>
> #include "ui-out.h"
>
> @@ -762,7 +763,6 @@ edit_command (char *arg, int from_tty)
> struct symtabs_and_lines sals;
> struct symtab_and_line sal;
> struct symbol *sym;
> - char *arg1;
> char *editor;
> char *p;
> const char *fn;
> @@ -784,21 +784,32 @@ edit_command (char *arg, int from_tty)
> }
> else
> {
> - /* Now should only be one argument -- decode it in SAL. */
> + struct cleanup *cleanup;
> + struct event_location *location, copy_location;
> + char *arg1;
>
> + /* Now should only be one argument -- decode it in SAL. */
> arg1 = arg;
> - sals = decode_line_1 (&arg1, DECODE_LINE_LIST_MODE, 0, 0);
> + location = string_to_event_location (&arg1, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
> + copy_location = *location;
> + sals = decode_line_1 (©_location, DECODE_LINE_LIST_MODE, 0, 0);
> +
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg1 = EVENT_LOCATION_LINESPEC (©_location);
Assigning to arg1 here is another case of setting something right before
the "junk at end of line spec" check. Is this right?
[It seems like arg1 would point to a beginning of the linespec and
thus always trigger the error.]
>
> filter_sals (&sals);
> if (! sals.nelts)
> {
> /* C++ */
> + do_cleanups (cleanup);
> return;
> }
> if (sals.nelts > 1)
> {
> ambiguous_line_spec (&sals);
> xfree (sals.sals);
> + do_cleanups (cleanup);
> return;
> }
>
> @@ -840,6 +851,7 @@ edit_command (char *arg, int from_tty)
>
> if (sal.symtab == 0)
> error (_("No line number known for %s."), arg);
> + do_cleanups (cleanup);
> }
>
> if ((editor = (char *) getenv ("EDITOR")) == NULL)
> @@ -868,6 +880,9 @@ list_command (char *arg, int from_tty)
> int dummy_beg = 0;
> int linenum_beg = 0;
> char *p;
> + struct cleanup *cleanup;
> +
> + cleanup = make_cleanup (null_cleanup, NULL);
>
> /* Pull in the current default source line if necessary. */
> if (arg == 0 || arg[0] == '+' || arg[0] == '-')
> @@ -910,15 +925,28 @@ list_command (char *arg, int from_tty)
> dummy_beg = 1;
> else
> {
> - sals = decode_line_1 (&arg1, DECODE_LINE_LIST_MODE, 0, 0);
> + struct event_location *location, copy_location;
> +
> + location = string_to_event_location (&arg1, current_language);
> + make_cleanup (delete_event_location, location);
> + copy_location = *location;
> + sals = decode_line_1 (©_location, DECODE_LINE_LIST_MODE, 0, 0);
> +
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg1 = EVENT_LOCATION_LINESPEC (©_location);
>
> filter_sals (&sals);
> if (!sals.nelts)
> - return; /* C++ */
> + {
> + /* C++ */
> + do_cleanups (cleanup);
> + return;
> + }
> if (sals.nelts > 1)
> {
> ambiguous_line_spec (&sals);
> xfree (sals.sals);
> + do_cleanups (cleanup);
> return;
> }
>
> @@ -943,18 +971,32 @@ list_command (char *arg, int from_tty)
> dummy_end = 1;
> else
> {
> + struct event_location *location, copy_location;
> +
> + location = string_to_event_location (&arg1, current_language);
> + make_cleanup (delete_event_location, location);
> + copy_location = *location;
> if (dummy_beg)
> - sals_end = decode_line_1 (&arg1, DECODE_LINE_LIST_MODE, 0, 0);
> + sals_end = decode_line_1 (©_location,
> + DECODE_LINE_LIST_MODE, 0, 0);
> else
> - sals_end = decode_line_1 (&arg1, DECODE_LINE_LIST_MODE,
> + sals_end = decode_line_1 (©_location, DECODE_LINE_LIST_MODE,
> sal.symtab, sal.line);
> +
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + arg1 = EVENT_LOCATION_LINESPEC (©_location);
> +
> filter_sals (&sals_end);
> if (sals_end.nelts == 0)
> - return;
> + {
> + do_cleanups (cleanup);
> + return;
> + }
> if (sals_end.nelts > 1)
> {
> ambiguous_line_spec (&sals_end);
> xfree (sals_end.sals);
> + do_cleanups (cleanup);
> return;
> }
> sal_end = sals_end.sals[0];
> @@ -1035,6 +1077,7 @@ list_command (char *arg, int from_tty)
> ? sal.line + get_lines_to_list ()
> : sal_end.line + 1),
> 0);
> + do_cleanups (cleanup);
> }
>
> /* Subroutine of disassemble_command to simplify it.
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 085ff47..2603acb 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -46,6 +46,7 @@
> #include "bcache.h"
> #include "gdb_bfd.h"
> #include "build-id.h"
> +#include "location.h"
>
> extern void _initialize_elfread (void);
>
> @@ -1069,7 +1070,8 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
> resolved_pc = gdbarch_addr_bits_remove (gdbarch, resolved_pc);
>
> gdb_assert (current_program_space == b->pspace || b->pspace == NULL);
> - elf_gnu_ifunc_record_cache (b->addr_string, resolved_pc);
> + elf_gnu_ifunc_record_cache (event_location_to_string (b->location),
> + resolved_pc);
>
> sal = find_pc_line (resolved_pc, 0);
> sals.nelts = 1;
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index cb76b9c..717ea68 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -44,6 +44,7 @@
> #include "filenames.h"
> #include "ada-lang.h"
> #include "stack.h"
> +#include "location.h"
>
> typedef struct symbol *symbolp;
> DEF_VEC_P (symbolp);
> @@ -1771,13 +1772,24 @@ canonicalize_linespec (struct linespec_state *state, linespec_p ls)
>
> /* Shortcut expressions, which can only appear by themselves. */
> if (ls->expression != NULL)
> - state->canonical->addr_string = xstrdup (ls->expression);
> + {
> + state->canonical->location
> + = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (state->canonical->location)
> + = xstrdup (ls->expression);
> + EVENT_LOCATION_SAVE_SPEC (state->canonical->location)
> + = xstrdup (ls->expression);
> + }
> else
> {
> struct ui_file *buf;
> int need_colon = 0;
>
> buf = mem_fileopen ();
> +
> + state->canonical->location
> + = new_event_location (EVENT_LOCATION_LINESPEC);
> +
> if (ls->source_filename)
> {
> fputs_unfiltered (ls->source_filename, buf);
> @@ -1826,7 +1838,10 @@ canonicalize_linespec (struct linespec_state *state, linespec_p ls)
> ls->line_offset.offset);
> }
>
> - state->canonical->addr_string = ui_file_xstrdup (buf, NULL);
> + EVENT_LOCATION_LINESPEC (state->canonical->location)
> + = ui_file_xstrdup (buf, NULL);
> + EVENT_LOCATION_SAVE_SPEC (state->canonical->location)
> + = xstrdup (EVENT_LOCATION_LINESPEC (state->canonical->location));
> ui_file_delete (buf);
> }
> }
> @@ -2371,6 +2386,7 @@ linespec_parser_new (linespec_parser *parser,
> int default_line,
> struct linespec_result *canonical)
> {
> + memset (parser, 0, sizeof (linespec_parser));
> parser->lexer.current.type = LSTOKEN_CONSUMED;
> memset (PARSER_RESULT (parser), 0, sizeof (struct linespec));
> PARSER_RESULT (parser)->line_offset.sign = LINE_OFFSET_UNKNOWN;
> @@ -2416,10 +2432,60 @@ linespec_parser_delete (void *arg)
> linespec_state_destructor (PARSER_STATE (parser));
> }
>
> +/* A helper function for decode_line_full and decode_line_1 to
> + turn LOCATION into symtabs_and_lines. */
> +
> +static struct symtabs_and_lines
> +event_location_to_sals (linespec_parser *parser,
> + struct event_location *location)
> +{
> + struct symtabs_and_lines result = {NULL, 0};
> +
> + switch (EVENT_LOCATION_TYPE (location))
> + {
> + case EVENT_LOCATION_LINESPEC:
> + {
> + const char *copy, *orig;
> + volatile struct gdb_exception except;
> +
> + TRY_CATCH (except, RETURN_MASK_ERROR)
> + {
> + orig = copy = EVENT_LOCATION_LINESPEC (location);
> + result = parse_linespec (parser, ©);
> + }
> + EVENT_LOCATION_LINESPEC (location) += copy - orig;
Yikes, I just noticed EVENT_LOCATION_LINESPEC is both an enum and a macro. :)
One of them needs a better name.
> +
> + if (except.reason < 0)
> + throw_exception (except);
> + }
> + break;
> +
> + default:
> + gdb_assert_not_reached ("unhandled event location type");
> + }
> +
> +
> + /* If the original location has a save spec, copy it into the
> + canonical location. This will preserve how the breakpoint
> + was originally created. */
> + if (EVENT_LOCATION_SAVE_SPEC (location) != NULL
> + && PARSER_STATE (parser)->canonical != NULL)
> + {
> + struct event_location *loc
> + = PARSER_STATE (parser)->canonical->location;
> +
> + xfree (EVENT_LOCATION_SAVE_SPEC (loc));
> + EVENT_LOCATION_SAVE_SPEC (loc)
> + = xstrdup (EVENT_LOCATION_SAVE_SPEC (location));
> + }
> +
> + return result;
> +}
> +
> /* See linespec.h. */
>
> void
> -decode_line_full (char **argptr, int flags,
> +decode_line_full (struct event_location *location, int flags,
> struct symtab *default_symtab,
> int default_line, struct linespec_result *canonical,
> const char *select_mode,
> @@ -2430,7 +2496,6 @@ decode_line_full (char **argptr, int flags,
> VEC (const_char_ptr) *filters = NULL;
> linespec_parser parser;
> struct linespec_state *state;
> - const char *copy, *orig;
>
> gdb_assert (canonical != NULL);
> /* The filter only makes sense for 'all'. */
> @@ -2446,13 +2511,10 @@ decode_line_full (char **argptr, int flags,
> cleanups = make_cleanup (linespec_parser_delete, &parser);
> save_current_program_space ();
>
> - orig = copy = *argptr;
> - result = parse_linespec (&parser, ©);
> - *argptr += copy - orig;
> + result = event_location_to_sals (&parser, location);
> state = PARSER_STATE (&parser);
>
> gdb_assert (result.nelts == 1 || canonical->pre_expanded);
> - gdb_assert (canonical->addr_string != NULL);
> canonical->pre_expanded = 1;
>
> /* Arrange for allocated canonical names to be freed. */
> @@ -2496,23 +2558,20 @@ decode_line_full (char **argptr, int flags,
> /* See linespec.h. */
>
> struct symtabs_and_lines
> -decode_line_1 (char **argptr, int flags,
> +decode_line_1 (struct event_location *location, int flags,
> struct symtab *default_symtab,
> int default_line)
> {
> struct symtabs_and_lines result;
> linespec_parser parser;
> struct cleanup *cleanups;
> - const char *copy, *orig;
>
> linespec_parser_new (&parser, flags, current_language, default_symtab,
> default_line, NULL);
> cleanups = make_cleanup (linespec_parser_delete, &parser);
> save_current_program_space ();
>
> - orig = copy = *argptr;
> - result = parse_linespec (&parser, ©);
> - *argptr += copy - orig;
> + result = event_location_to_sals (&parser, location);
>
> do_cleanups (cleanups);
> return result;
> @@ -2525,6 +2584,8 @@ decode_line_with_current_source (char *string, int flags)
> {
> struct symtabs_and_lines sals;
> struct symtab_and_line cursal;
> + struct event_location *location, copy_location;
> + struct cleanup *cleanup;
>
> if (string == 0)
> error (_("Empty line specification."));
> @@ -2533,11 +2594,19 @@ decode_line_with_current_source (char *string, int flags)
> and get a default source symtab+line or it will recursively call us! */
> cursal = get_current_source_symtab_and_line ();
>
> - sals = decode_line_1 (&string, flags,
> + location = string_to_event_location (&string, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
> +
> + copy_location = *location;
> + sals = decode_line_1 (©_location, flags,
> cursal.symtab, cursal.line);
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + string = EVENT_LOCATION_LINESPEC (©_location);
>
> if (*string)
> error (_("Junk at end of line specification: %s"), string);
> +
> + do_cleanups (cleanup);
> return sals;
> }
>
> @@ -2547,19 +2616,30 @@ struct symtabs_and_lines
> decode_line_with_last_displayed (char *string, int flags)
> {
> struct symtabs_and_lines sals;
> + struct event_location *location, copy_location;
> + struct cleanup *cleanup;
>
> if (string == 0)
> error (_("Empty line specification."));
>
> + location = string_to_event_location (&string, current_language);
> + cleanup = make_cleanup (delete_event_location, location);
> +
> + copy_location = *location;
> if (last_displayed_sal_is_valid ())
> - sals = decode_line_1 (&string, flags,
> + sals = decode_line_1 (©_location, flags,
> get_last_displayed_symtab (),
> get_last_displayed_line ());
> else
> - sals = decode_line_1 (&string, flags, (struct symtab *) NULL, 0);
> + sals = decode_line_1 (©_location, flags, (struct symtab *) NULL, 0);
> +
> + if (EVENT_LOCATION_TYPE (©_location) == EVENT_LOCATION_LINESPEC)
> + string = EVENT_LOCATION_LINESPEC (©_location);
>
> if (*string)
> error (_("Junk at end of line specification: %s"), string);
> +
> + do_cleanups (cleanup);
> return sals;
> }
>
> @@ -2656,11 +2736,15 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char **argptr)
> if (self->canonical)
> {
> self->canonical->pre_expanded = 1;
> + self->canonical->location
> + = new_event_location (EVENT_LOCATION_LINESPEC);
> +
> if (ls->source_filename)
> - self->canonical->addr_string
> + EVENT_LOCATION_LINESPEC (self->canonical->location)
> = xstrprintf ("%s:%s", ls->source_filename, saved_arg);
> else
> - self->canonical->addr_string = xstrdup (saved_arg);
> + EVENT_LOCATION_LINESPEC (self->canonical->location)
> + = xstrdup (saved_arg);
> }
> }
>
> @@ -3693,7 +3777,7 @@ destroy_linespec_result (struct linespec_result *ls)
> int i;
> struct linespec_sals *lsal;
>
> - xfree (ls->addr_string);
> + delete_event_location (ls->location);
> for (i = 0; VEC_iterate (linespec_sals, ls->sals, i, lsal); ++i)
> {
> xfree (lsal->canonical);
> diff --git a/gdb/linespec.h b/gdb/linespec.h
> index 5c065cf..658f2a1 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -39,7 +39,7 @@ enum decode_line_flags
>
> struct linespec_sals
> {
> - /* This is the linespec corresponding to the sals contained in this
> + /* This is the location corresponding to the sals contained in this
> object. It can be passed as the FILTER argument to future calls
> to decode_line_full. This is freed by
> destroy_linespec_result. */
> @@ -71,9 +71,9 @@ struct linespec_result
> object. */
> int pre_expanded;
>
> - /* If PRE_EXPANDED is non-zero, this is set to the linespec entered
> + /* If PRE_EXPANDED is non-zero, this is set to the location entered
> by the user. This will be freed by destroy_linespec_result. */
> - char *addr_string;
> + struct event_location *location;
>
> /* The sals. The vector will be freed by
> destroy_linespec_result. */
> @@ -96,10 +96,10 @@ extern struct cleanup *
> /* Decode a linespec using the provided default symtab and line. */
>
> extern struct symtabs_and_lines
> - decode_line_1 (char **argptr, int flags,
> + decode_line_1 (struct event_location *location, int flags,
> struct symtab *default_symtab, int default_line);
>
> -/* Parse *ARGPTR as a linespec and return results. This is the "full"
> +/* Parse LOCATION and return results. This is the "full"
> interface to this module, which handles multiple results
> properly.
>
> @@ -135,7 +135,7 @@ extern struct symtabs_and_lines
> strcmp sense) to FILTER will be returned; all others will be
> filtered out. */
>
> -extern void decode_line_full (char **argptr, int flags,
> +extern void decode_line_full (struct event_location *location, int flags,
> struct symtab *default_symtab, int default_line,
> struct linespec_result *canonical,
> const char *select_mode,
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index 29e3fb3..93d62c8 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -30,6 +30,9 @@
> #include "observer.h"
> #include "mi-main.h"
> #include "mi-cmd-break.h"
> +#include "language.h"
> +#include "location.h"
> +#include "linespec.h"
> #include "gdb_obstack.h"
> #include <ctype.h>
>
> @@ -179,6 +182,7 @@ mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
> int tracepoint = 0;
> struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
> enum bptype type_wanted;
> + struct event_location *location;
> struct breakpoint_ops *ops;
> char *extra_string = NULL;
>
> @@ -289,7 +293,10 @@ mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
> ops = &bkpt_breakpoint_ops;
> }
>
> - create_breakpoint (get_current_arch (), address, condition, thread,
> + location = string_to_event_location (&address, current_language);
> + make_cleanup (delete_event_location, location);
> +
> + create_breakpoint (get_current_arch (), location, condition, thread,
> extra_string,
> 0 /* condition and thread are valid. */,
> temp_p, type_wanted,
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 838d9f9..f7afe0b 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -31,6 +31,8 @@
> #include "gdb_regex.h"
> #include "frame.h"
> #include "arch-utils.h"
> +#include "location.h"
> +
> #include <ctype.h>
>
> typedef struct bound_probe bound_probe_s;
> @@ -41,23 +43,24 @@ DEF_VEC_O (bound_probe_s);
> /* See definition in probe.h. */
>
> struct symtabs_and_lines
> -parse_probes (char **argptr, struct linespec_result *canonical)
> +parse_probes (struct event_location *location,
> + struct linespec_result *canonical)
> {
> - char *arg_start, *arg_end, *arg;
> + char *arg_end, *arg;
> char *objfile_namestr = NULL, *provider = NULL, *name, *p;
> struct cleanup *cleanup;
> struct symtabs_and_lines result;
> struct objfile *objfile;
> struct program_space *pspace;
> const struct probe_ops *probe_ops;
> - const char *cs;
> + const char *arg_start, *cs;
>
> result.sals = NULL;
> result.nelts = 0;
>
> - arg_start = *argptr;
> + arg_start = EVENT_LOCATION_LINESPEC (location);
>
> - cs = *argptr;
> + cs = arg_start;
> probe_ops = probe_linespec_to_ops (&cs);
> if (probe_ops == NULL)
> error (_("'%s' is not a probe linespec"), arg_start);
> @@ -170,10 +173,12 @@ parse_probes (char **argptr, struct linespec_result *canonical)
> {
> canonical->special_display = 1;
> canonical->pre_expanded = 1;
> - canonical->addr_string = savestring (*argptr, arg_end - *argptr);
> + canonical->location = new_event_location (EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (canonical->location)
> + = savestring (arg_start, arg_end - arg_start);
> }
>
> - *argptr = arg_end;
> + EVENT_LOCATION_LINESPEC (location) = arg_end;
> do_cleanups (cleanup);
>
> return result;
> diff --git a/gdb/probe.h b/gdb/probe.h
> index aa8aba8..f53fec1 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -20,6 +20,8 @@
> #if !defined (PROBE_H)
> #define PROBE_H 1
>
> +struct event_location;
> +
> #include "gdb_vecs.h"
>
> /* Definition of a vector of probes. */
> @@ -201,9 +203,9 @@ struct bound_probe
> };
>
> /* A helper for linespec that decodes a probe specification. It returns a
> - symtabs_and_lines object and updates *ARGPTR or throws an error. */
> + symtabs_and_lines object and updates LOC or throws an error. */
>
> -extern struct symtabs_and_lines parse_probes (char **argptr,
> +extern struct symtabs_and_lines parse_probes (struct event_location *loc,
> struct linespec_result *canon);
>
> /* Helper function to register the proper probe_ops to a newly created probe.
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index 125c6fd..e808a47 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -31,6 +31,7 @@
> #include "ada-lang.h"
> #include "arch-utils.h"
> #include "language.h"
> +#include "location.h"
>
> /* Number of live breakpoints. */
> static int bppy_live;
> @@ -369,7 +370,7 @@ bppy_set_hit_count (PyObject *self, PyObject *newvalue, void *closure)
> static PyObject *
> bppy_get_location (PyObject *self, void *closure)
> {
> - char *str;
> + const char *str;
> gdbpy_breakpoint_object *obj = (gdbpy_breakpoint_object *) self;
>
> BPPY_REQUIRE_VALID (obj);
> @@ -377,8 +378,7 @@ bppy_get_location (PyObject *self, void *closure)
> if (obj->bp->type != bp_breakpoint)
> Py_RETURN_NONE;
>
> - str = obj->bp->addr_string;
> -
> + str = event_location_to_string (obj->bp->location);
> if (! str)
> str = "";
> return PyString_Decode (str, strlen (str), host_charset (), NULL);
> @@ -654,8 +654,12 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
> {
> case bp_breakpoint:
> {
> + struct event_location location;
> +
> + initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (&location) = copy;
> create_breakpoint (python_gdbarch,
> - copy, NULL, -1, NULL,
> + &location, NULL, -1, NULL,
> 0,
> temporary_bp, bp_breakpoint,
> 0,
> diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
> index 712a9ee..b4db5ec 100644
> --- a/gdb/python/py-finishbreakpoint.c
> +++ b/gdb/python/py-finishbreakpoint.c
> @@ -30,6 +30,7 @@
> #include "observer.h"
> #include "inferior.h"
> #include "block.h"
> +#include "location.h"
>
> /* Function that is called when a Python finish bp is found out of scope. */
> static char * const outofscope_func = "out_of_scope";
> @@ -168,7 +169,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
> int internal_bp = 0;
> CORE_ADDR finish_pc, pc;
> volatile struct gdb_exception except;
> - char *addr_str, small_buf[100];
> + char small_buf[100];
> struct symbol *function;
>
> if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
> @@ -287,13 +288,15 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>
> TRY_CATCH (except, RETURN_MASK_ALL)
> {
> + 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));
> - addr_str = small_buf;
> -
> + EVENT_LOCATION_LINESPEC (&location) = small_buf;
> create_breakpoint (python_gdbarch,
> - addr_str, NULL, thread, NULL,
> + &location, NULL, thread, NULL,
> 0,
> 1 /*temp_flag*/,
> bp_breakpoint,
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index cbfa73a..4b8b4d6 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -35,6 +35,7 @@
> #include "extension-priv.h"
> #include "cli/cli-utils.h"
> #include <ctype.h>
> +#include "location.h"
>
> /* Declared constants and enum for python stack printing. */
> static const char python_excp_none[] = "none";
> @@ -725,9 +726,14 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
> {
> if (arg)
> {
> + struct event_location location;
> +
> copy = xstrdup (arg);
> copy_to_free = copy;
> - sals = decode_line_1 (©, 0, 0, 0);
> + initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (&location) = copy;
> + sals = decode_line_1 (&location, 0, 0, 0);
> + copy = EVENT_LOCATION_LINESPEC (&location);
> }
> else
> {
> diff --git a/gdb/remote.c b/gdb/remote.c
> index ba04d0c..0a39c5b 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -46,6 +46,7 @@
> #include "gdb_bfd.h"
> #include "filestuff.h"
> #include "rsp-low.h"
> +#include "location.h"
>
> #include <sys/time.h>
>
> @@ -10539,13 +10540,12 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
>
> if (packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
> {
> - if (b->addr_string)
> + if (b->location != NULL)
> {
> strcpy (buf, "QTDPsrc:");
> - encode_source_string (b->number, loc->address,
> - "at", b->addr_string, buf + strlen (buf),
> - 2048 - strlen (buf));
> -
> + encode_source_string (b->number, loc->address, "at",
> + event_location_to_string (b->location),
> + buf + strlen (buf), 2048 - strlen (buf));
> putpkt (buf);
> remote_get_noisy_reply (&target_buf, &target_buf_size);
> if (strcmp (target_buf, "OK"))
> diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
> index 4fc3ca5..43b4fec 100644
> --- a/gdb/spu-tdep.c
> +++ b/gdb/spu-tdep.c
> @@ -48,7 +48,7 @@
> #include "ax.h"
> #include "exceptions.h"
> #include "spu-tdep.h"
> -
> +#include "location.h"
>
> /* The list of available "set spu " and "show spu " commands. */
> static struct cmd_list_element *setspucmdlist = NULL;
> @@ -1958,7 +1958,7 @@ spu_catch_start (struct objfile *objfile)
> struct bound_minimal_symbol minsym;
> struct symtab *symtab;
> CORE_ADDR pc;
> - char buf[32];
> + struct event_location location;
>
> /* Do this only if requested by "set spu stop-on-load on". */
> if (!spu_stop_on_load_p)
> @@ -2001,8 +2001,9 @@ spu_catch_start (struct objfile *objfile)
>
> /* Use a numerical address for the set_breakpoint command to avoid having
> the breakpoint re-set incorrectly. */
> - xsnprintf (buf, sizeof buf, "*%s", core_addr_to_string (pc));
> - create_breakpoint (get_objfile_arch (objfile), buf /* arg */,
> + initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
> + EVENT_LOCATION_LINESPEC (&location) = xstrdup (core_addr_to_string (pc));
> + create_breakpoint (get_objfile_arch (objfile), &location,
> NULL /* cond_string */, -1 /* thread */,
> NULL /* extra_string */,
> 0 /* parse_condition_and_thread */, 1 /* tempflag */,
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index c522193..d565c31 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -57,6 +57,7 @@
> #include "filestuff.h"
> #include "rsp-low.h"
> #include "tracefile.h"
> +#include "location.h"
>
> /* readline include files */
> #include "readline/readline.h"
> @@ -2712,14 +2713,23 @@ scope_info (char *args, int from_tty)
> int j, count = 0;
> struct gdbarch *gdbarch;
> int regno;
> + struct event_location *location, copy_location;
> + struct cleanup *back_to;
>
> if (args == 0 || *args == 0)
> error (_("requires an argument (function, "
> "line or *addr) to define a scope"));
>
> - sals = decode_line_1 (&args, DECODE_LINE_FUNFIRSTLINE, NULL, 0);
> + location = string_to_event_location (&args, current_language);
> + back_to = make_cleanup (delete_event_location, location);
> + copy_location = *location;
> + sals = decode_line_1 (©_location, DECODE_LINE_FUNFIRSTLINE, NULL, 0);
> if (sals.nelts == 0)
> - return; /* Presumably decode_line_1 has already warned. */
> + {
> + /* Presumably decode_line_1 has already warned. */
> + do_cleanups (back_to);
> + return;
> + }
>
> /* Resolve line numbers to PC. */
> resolve_sal_pc (&sals.sals[0]);
> @@ -2856,6 +2866,7 @@ scope_info (char *args, int from_tty)
> if (count <= 0)
> printf_filtered ("Scope for %s contains no locals or arguments.\n",
> save_args);
> + do_cleanups (back_to);
> }
>
> /* Helper for trace_dump_command. Dump the action list starting at
> @@ -3078,7 +3089,7 @@ trace_dump_command (char *args, int from_tty)
>
> extern int
> encode_source_string (int tpnum, ULONGEST addr,
> - char *srctype, char *src, char *buf, int buf_size)
> + char *srctype, const char *src, char *buf, int buf_size)
> {
> if (80 + strlen (srctype) > buf_size)
> error (_("Buffer too small for source encoding"));
> diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
> index 39b5edb..57e5807 100644
> --- a/gdb/tracepoint.h
> +++ b/gdb/tracepoint.h
> @@ -298,7 +298,7 @@ extern struct trace_state_variable *
> extern struct trace_state_variable *create_trace_state_variable (const char *name);
>
> extern int encode_source_string (int num, ULONGEST addr,
> - char *srctype, char *src,
> + char *srctype, const char *src,
> char *buf, int buf_size);
>
> extern void parse_trace_status (char *line, struct trace_status *ts);
--
/dje