* [RFA 5/9] Explicit locations v2 - Add probe locations
@ 2014-05-08 18:02 Keith Seitz
2014-05-30 18:19 ` Keith Seitz
0 siblings, 1 reply; 5+ messages in thread
From: Keith Seitz @ 2014-05-08 18:02 UTC (permalink / raw)
To: gdb-patches@sourceware.org ml
[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]
Hi,
This patch adds support for probe locations. This is a new location type
that I added in this v2 revision of this series.
Like the previous patch for address locations, this patch does not add
any new functionality. It is also "simply" an elaboration of the new
location API.
Keith
ChangeLog
2014-05-08 Keith Seitz <keiths@redhat.com>
* break-catch-throw.c (re_set_exception_catchpoint): Convert
linespec for stap probe to probe location.
* breakpoint.c (create_longjmp_master_breakpoint): Likewise.
(create_exception_master_breakpoint): Likewise.
(break_command_1): Remove local variable `arg_cp'.
Check location type to set appropriate breakpoint ops methods.
(trace_command): Likewise.
* linespec.c (event_location_to_sals): Handle probe locations.
* location.c (copy_event_location): Likewise.
(delete_event_location): Likewise.
(event_location_to_string): Likewise.
(string_to_event_location): Likewise.
(event_location_empty_p): Handel probe locations.
* location.h (enum event_location_type): Add EVENT_LOCATION_PROBE.
(EVENT_LOCATION_PROBE): Define accessor macro.
* probe.c (parse_probes): Assert that LOCATION is a probe location.
Convert linespec into probe location.
[-- Attachment #2: explicit-probe-locations.patch --]
[-- Type: text/x-patch, Size: 7482 bytes --]
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 31cce47..e352935 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -214,8 +214,8 @@ re_set_exception_catchpoint (struct breakpoint *self)
{
struct event_location location;
- initialize_event_location (&location, EVENT_LOCATION_LINESPEC);
- EVENT_LOCATION_LINESPEC (&location)
+ initialize_event_location (&location, EVENT_LOCATION_PROBE);
+ EVENT_LOCATION_PROBE (&location)
= ASTRDUP (exception_functions[kind].probe);
sals = parse_probes (&location, NULL);
}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f509d7a..fdf7c10 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3387,8 +3387,8 @@ create_longjmp_master_breakpoint (void)
objfile),
bp_longjmp_master,
&internal_breakpoint_ops);
- b->location = new_event_location (EVENT_LOCATION_LINESPEC);
- EVENT_LOCATION_LINESPEC (b->location)
+ b->location = new_event_location (EVENT_LOCATION_PROBE);
+ EVENT_LOCATION_PROBE (b->location)
= xstrdup ("-probe-stap libc:longjmp");
b->enable_state = bp_disabled;
}
@@ -3554,8 +3554,8 @@ create_exception_master_breakpoint (void)
objfile),
bp_exception_master,
&internal_breakpoint_ops);
- b->location = new_event_location (EVENT_LOCATION_LINESPEC);
- EVENT_LOCATION_LINESPEC (b->location)
+ b->location = new_event_location (EVENT_LOCATION_PROBE);
+ EVENT_LOCATION_PROBE (b->location)
= xstrdup ("-probe-stap libgcc:unwind");
b->enable_state = bp_disabled;
}
@@ -10086,7 +10086,6 @@ break_command_1 (char *arg, int flag, int from_tty)
? bp_hardware_breakpoint
: bp_breakpoint);
struct breakpoint_ops *ops;
- const char *arg_cp = arg;
struct event_location *location;
struct cleanup *cleanup;
@@ -10094,7 +10093,8 @@ break_command_1 (char *arg, int flag, int from_tty)
cleanup = make_cleanup (delete_event_location, location);
/* Matching breakpoints on probes. */
- if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
+ if (location != NULL
+ && EVENT_LOCATION_TYPE (location) == EVENT_LOCATION_PROBE)
ops = &bkpt_probe_breakpoint_ops;
else
ops = &bkpt_breakpoint_ops;
@@ -15471,11 +15471,11 @@ trace_command (char *arg, int from_tty)
struct breakpoint_ops *ops;
struct event_location *location;
struct cleanup *back_to;
- const char *arg_cp = arg;
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)
+ if (location != NULL
+ && EVENT_LOCATION_TYPE (location) == EVENT_LOCATION_PROBE)
ops = &tracepoint_probe_breakpoint_ops;
else
ops = &tracepoint_breakpoint_ops;
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 07a84b5..069aab6 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2457,6 +2457,11 @@ event_location_to_sals (linespec_parser *parser,
EVENT_LOCATION_ADDRESS (location));
break;
+ case EVENT_LOCATION_PROBE:
+ /* Probes are handled by their own decoders. */
+ gdb_assert_not_reached ("attempt to decode probe location");
+ break;
+
default:
gdb_assert_not_reached ("unhandled event location type");
}
diff --git a/gdb/location.c b/gdb/location.c
index 22e7496..a64d37c 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -72,6 +72,10 @@ copy_event_location (const struct event_location *src)
EVENT_LOCATION_ADDRESS (dst) = EVENT_LOCATION_ADDRESS (src);
break;
+ case EVENT_LOCATION_PROBE:
+ EVENT_LOCATION_PROBE (dst) = xstrdup (EVENT_LOCATION_PROBE (src));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -101,6 +105,10 @@ delete_event_location (void *data)
/* Nothing to do. */
break;
+ case EVENT_LOCATION_PROBE:
+ xfree (EVENT_LOCATION_PROBE (location));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -128,6 +136,10 @@ event_location_to_string (const struct event_location *location)
result = EVENT_LOCATION_SAVE_SPEC (location);
break;
+ case EVENT_LOCATION_PROBE:
+ result = EVENT_LOCATION_PROBE (location);
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -158,12 +170,28 @@ string_to_event_location (char **stringp,
}
else
{
- /* Everything else is a linespec. */
- location = new_event_location (EVENT_LOCATION_LINESPEC);
- if (*stringp != NULL)
+ const char *cs;
+
+ /* Next, try the input as a probe spec. */
+ cs = *stringp;
+ if (cs != NULL && probe_linespec_to_ops (&cs) != NULL)
{
- EVENT_LOCATION_LINESPEC (location) = xstrdup (*stringp);
- *stringp += strlen (*stringp);
+ location = new_event_location (EVENT_LOCATION_PROBE);
+ if (*stringp != NULL)
+ {
+ EVENT_LOCATION_PROBE (location) = xstrdup (*stringp);
+ *stringp += strlen (*stringp);
+ }
+ }
+ 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);
+ }
}
}
@@ -183,6 +211,9 @@ event_location_empty_p (const struct event_location *location)
case EVENT_LOCATION_ADDRESS:
return 0;
+ case EVENT_LOCATION_PROBE:
+ return EVENT_LOCATION_PROBE (location) == NULL;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
diff --git a/gdb/location.h b/gdb/location.h
index 8575daf..b207c87 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -30,7 +30,10 @@ enum event_location_type
EVENT_LOCATION_LINESPEC,
/* An address in the inferior. */
- EVENT_LOCATION_ADDRESS
+ EVENT_LOCATION_ADDRESS,
+
+ /* A probe location. */
+ EVENT_LOCATION_PROBE
};
/* An event location used to set a stop event in the inferior.
@@ -50,6 +53,7 @@ struct event_location
probes. */
char *addr_string;
#define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
+#define EVENT_LOCATION_PROBE(S) ((S)->u.addr_string)
/* An address in the inferior. */
CORE_ADDR address;
diff --git a/gdb/probe.c b/gdb/probe.c
index f7afe0b..82bcdab 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
result.sals = NULL;
result.nelts = 0;
- arg_start = EVENT_LOCATION_LINESPEC (location);
+ gdb_assert (EVENT_LOCATION_TYPE (location) == EVENT_LOCATION_PROBE);
+ arg_start = EVENT_LOCATION_PROBE (location);
cs = arg_start;
probe_ops = probe_linespec_to_ops (&cs);
@@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
{
canonical->special_display = 1;
canonical->pre_expanded = 1;
- canonical->location = new_event_location (EVENT_LOCATION_LINESPEC);
- EVENT_LOCATION_LINESPEC (canonical->location)
+ canonical->location = new_event_location (EVENT_LOCATION_PROBE);
+ EVENT_LOCATION_PROBE (canonical->location)
= savestring (arg_start, arg_end - arg_start);
}
- EVENT_LOCATION_LINESPEC (location) = arg_end;
+ EVENT_LOCATION_PROBE (location) = arg_end;
do_cleanups (cleanup);
return result;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFA 5/9] Explicit locations v2 - Add probe locations
2014-05-08 18:02 [RFA 5/9] Explicit locations v2 - Add probe locations Keith Seitz
@ 2014-05-30 18:19 ` Keith Seitz
2014-08-02 23:39 ` Doug Evans
0 siblings, 1 reply; 5+ messages in thread
From: Keith Seitz @ 2014-05-30 18:19 UTC (permalink / raw)
To: gdb-patches@sourceware.org ml
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
This is also an update of this patch based on feedback from previous
patch reviews.
Keith
Changes since last revision:
- use make_cleanup_delete_event_location
- remove SAVE_SPEC
- new_probe_location
ChangeLog
2014-05-29 Keith Seitz <keiths@redhat.com>
* break-catch-throw.c (re_set_exception_catchpoint): Convert
linespec for stap probe to probe location.
* breakpoint.c (create_longjmp_master_breakpoint): Likewise.
(create_exception_master_breakpoint): Likewise.
(break_command_1): Remove local variable `arg_cp'.
Check location type to set appropriate breakpoint ops methods.
(trace_command): Likewise.
* linespec.c (event_location_to_sals): Handle probe locations.
* location.c (copy_event_location): Likewise.
(delete_event_location): Likewise.
(event_location_to_string): Likewise.
(string_to_event_location): Likewise.
(event_location_empty_p): Handel probe locations.
* location.h (enum event_location_type): Add PROBE_LOCATION.
(EVENT_LOCATION_PROBE): Define accessor macro.
* probe.c (parse_probes): Assert that LOCATION is a probe location.
Convert linespec into probe location.
[-- Attachment #2: explicit-probe-locations-2.patch --]
[-- Type: text/x-patch, Size: 7863 bytes --]
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index e2c8365..207e7af 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -214,8 +214,8 @@ re_set_exception_catchpoint (struct breakpoint *self)
{
struct event_location location;
- initialize_event_location (&location, LINESPEC_LOCATION);
- EVENT_LOCATION_LINESPEC (&location)
+ initialize_event_location (&location, PROBE_LOCATION);
+ EVENT_LOCATION_PROBE (&location)
= ASTRDUP (exception_functions[kind].probe);
sals = parse_probes (&location, NULL);
}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ef60f44..345b104 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3387,7 +3387,7 @@ create_longjmp_master_breakpoint (void)
bp_longjmp_master,
&internal_breakpoint_ops);
b->location
- = new_linespec_location ("-probe-stap libc:longjmp");
+ = new_probe_location ("-probe-stap libc:longjmp");
b->enable_state = bp_disabled;
}
@@ -3551,7 +3551,7 @@ create_exception_master_breakpoint (void)
bp_exception_master,
&internal_breakpoint_ops);
b->location
- = new_linespec_location ("-probe-stap libgcc:unwind");
+ = new_probe_location ("-probe-stap libgcc:unwind");
b->enable_state = bp_disabled;
}
@@ -10068,7 +10068,6 @@ break_command_1 (char *arg, int flag, int from_tty)
? bp_hardware_breakpoint
: bp_breakpoint);
struct breakpoint_ops *ops;
- const char *arg_cp = arg;
struct event_location *location;
struct cleanup *cleanup;
@@ -10076,7 +10075,8 @@ break_command_1 (char *arg, int flag, int from_tty)
cleanup = make_cleanup_delete_event_location (location);
/* Matching breakpoints on probes. */
- if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
+ if (location != NULL
+ && EVENT_LOCATION_TYPE (location) == PROBE_LOCATION)
ops = &bkpt_probe_breakpoint_ops;
else
ops = &bkpt_breakpoint_ops;
@@ -15432,11 +15432,11 @@ trace_command (char *arg, int from_tty)
struct breakpoint_ops *ops;
struct event_location *location;
struct cleanup *back_to;
- const char *arg_cp = arg;
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)
+ if (location != NULL
+ && EVENT_LOCATION_TYPE (location) == PROBE_LOCATION)
ops = &tracepoint_probe_breakpoint_ops;
else
ops = &tracepoint_breakpoint_ops;
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8ea0c7b..deda86e 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2433,6 +2433,11 @@ event_location_to_sals (linespec_parser *parser,
EVENT_LOCATION_ADDRESS (location));
break;
+ case PROBE_LOCATION:
+ /* Probes are handled by their own decoders. */
+ gdb_assert_not_reached ("attempt to decode probe location");
+ break;
+
default:
gdb_assert_not_reached ("unhandled event location type");
}
diff --git a/gdb/location.c b/gdb/location.c
index 3c430db..eafa27d 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -67,6 +67,21 @@ new_address_location (CORE_ADDR addr)
return location;
}
+/* Create a new probe location. The return result is malloc'd
+ and should be freed with delete_event_location. */
+
+struct event_location *
+new_probe_location (const char *probe)
+{
+ struct event_location *location;
+
+ location = XNEW (struct event_location);
+ initialize_event_location (location, PROBE_LOCATION);
+ if (probe != NULL)
+ EVENT_LOCATION_PROBE (location) = xstrdup (probe);
+ return location;
+}
+
/* Return a copy of the given SRC location. */
struct event_location *
@@ -89,6 +104,10 @@ copy_event_location (const struct event_location *src)
EVENT_LOCATION_ADDRESS (dst) = EVENT_LOCATION_ADDRESS (src);
break;
+ case PROBE_LOCATION:
+ EVENT_LOCATION_PROBE (dst) = xstrdup (EVENT_LOCATION_PROBE (src));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -133,6 +152,10 @@ delete_event_location (struct event_location *location)
/* Nothing to do. */
break;
+ case PROBE_LOCATION:
+ xfree (EVENT_LOCATION_PROBE (location));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -166,6 +189,10 @@ event_location_to_string_const (const struct event_location *location)
core_addr_to_string (EVENT_LOCATION_ADDRESS (location)));
break;
+ case PROBE_LOCATION:
+ result = xstrdup (EVENT_LOCATION_PROBE (location));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -212,10 +239,23 @@ string_to_event_location (char **stringp,
}
else
{
- /* Everything else is a linespec. */
- location = new_linespec_location (*stringp);
- if (*stringp != NULL)
- *stringp += strlen (*stringp);
+ const char *cs;
+
+ /* Next, try the input as a probe spec. */
+ cs = *stringp;
+ if (cs != NULL && probe_linespec_to_ops (&cs) != NULL)
+ {
+ location = new_probe_location (*stringp);
+ if (*stringp != NULL)
+ *stringp += strlen (*stringp);
+ }
+ else
+ {
+ /* Everything else is a linespec. */
+ location = new_linespec_location (*stringp);
+ if (*stringp != NULL)
+ *stringp += strlen (*stringp);
+ }
}
return location;
@@ -234,6 +274,9 @@ event_location_empty_p (const struct event_location *location)
case ADDRESS_LOCATION:
return 0;
+ case PROBE_LOCATION:
+ return EVENT_LOCATION_PROBE (location) == NULL;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
diff --git a/gdb/location.h b/gdb/location.h
index 1daefb6..e6f14d9 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -30,7 +30,10 @@ enum event_location_type
LINESPEC_LOCATION,
/* An address in the inferior. */
- ADDRESS_LOCATION
+ ADDRESS_LOCATION,
+
+ /* A probe location. */
+ PROBE_LOCATION
};
/* An event location used to set a stop event in the inferior.
@@ -50,6 +53,7 @@ struct event_location
probes. */
char *addr_string;
#define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
+#define EVENT_LOCATION_PROBE(S) ((S)->u.addr_string)
/* An address in the inferior. */
CORE_ADDR address;
@@ -98,6 +102,12 @@ extern struct event_location *
extern struct event_location *
new_address_location (CORE_ADDR addr);
+/* Create a new probe location. The return result is malloc'd
+ and should be freed with delete_event_location. */
+
+extern struct event_location *
+ new_probe_location (const char *probe);
+
/* Return a copy of the given SRC location. */
extern struct event_location *
diff --git a/gdb/probe.c b/gdb/probe.c
index c7597d9..2ff5d86 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
result.sals = NULL;
result.nelts = 0;
- arg_start = EVENT_LOCATION_LINESPEC (location);
+ gdb_assert (EVENT_LOCATION_TYPE (location) == PROBE_LOCATION);
+ arg_start = EVENT_LOCATION_PROBE (location);
cs = arg_start;
probe_ops = probe_linespec_to_ops (&cs);
@@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
{
canonical->special_display = 1;
canonical->pre_expanded = 1;
- canonical->location = new_linespec_location (NULL);
- EVENT_LOCATION_LINESPEC (canonical->location)
+ canonical->location = new_probe_location (NULL);
+ EVENT_LOCATION_PROBE (canonical->location)
= savestring (arg_start, arg_end - arg_start);
}
- EVENT_LOCATION_LINESPEC (location) = arg_end;
+ EVENT_LOCATION_PROBE (location) = arg_end;
do_cleanups (cleanup);
return result;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFA 5/9] Explicit locations v2 - Add probe locations
2014-05-30 18:19 ` Keith Seitz
@ 2014-08-02 23:39 ` Doug Evans
2014-08-03 0:22 ` Doug Evans
2014-09-03 19:32 ` Keith Seitz
0 siblings, 2 replies; 5+ messages in thread
From: Doug Evans @ 2014-08-02 23:39 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml
Hi. Comments inline, mostly nits.
Keith Seitz writes:
> This is also an update of this patch based on feedback from previous
> patch reviews.
>
> Keith
>
> Changes since last revision:
> - use make_cleanup_delete_event_location
> - remove SAVE_SPEC
> - new_probe_location
>
> ChangeLog
> 2014-05-29 Keith Seitz <keiths@redhat.com>
>
> * break-catch-throw.c (re_set_exception_catchpoint): Convert
> linespec for stap probe to probe location.
> * breakpoint.c (create_longjmp_master_breakpoint): Likewise.
> (create_exception_master_breakpoint): Likewise.
> (break_command_1): Remove local variable `arg_cp'.
> Check location type to set appropriate breakpoint ops methods.
> (trace_command): Likewise.
> * linespec.c (event_location_to_sals): Handle probe locations.
> * location.c (copy_event_location): Likewise.
> (delete_event_location): Likewise.
> (event_location_to_string): Likewise.
> (string_to_event_location): Likewise.
> (event_location_empty_p): Handel probe locations.
"Handle ...", or just "Likewise."
> * location.h (enum event_location_type): Add PROBE_LOCATION.
> (EVENT_LOCATION_PROBE): Define accessor macro.
> * probe.c (parse_probes): Assert that LOCATION is a probe location.
> Convert linespec into probe location.
>
>
>
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index e2c8365..207e7af 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -214,8 +214,8 @@ re_set_exception_catchpoint (struct breakpoint *self)
> {
> struct event_location location;
>
> - initialize_event_location (&location, LINESPEC_LOCATION);
> - EVENT_LOCATION_LINESPEC (&location)
> + initialize_event_location (&location, PROBE_LOCATION);
> + EVENT_LOCATION_PROBE (&location)
> = ASTRDUP (exception_functions[kind].probe);
> sals = parse_probes (&location, NULL);
> }
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index ef60f44..345b104 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -3387,7 +3387,7 @@ create_longjmp_master_breakpoint (void)
> bp_longjmp_master,
> &internal_breakpoint_ops);
> b->location
> - = new_linespec_location ("-probe-stap libc:longjmp");
> + = new_probe_location ("-probe-stap libc:longjmp");
> b->enable_state = bp_disabled;
> }
>
> @@ -3551,7 +3551,7 @@ create_exception_master_breakpoint (void)
> bp_exception_master,
> &internal_breakpoint_ops);
> b->location
> - = new_linespec_location ("-probe-stap libgcc:unwind");
> + = new_probe_location ("-probe-stap libgcc:unwind");
> b->enable_state = bp_disabled;
> }
>
> @@ -10068,7 +10068,6 @@ break_command_1 (char *arg, int flag, int from_tty)
> ? bp_hardware_breakpoint
> : bp_breakpoint);
> struct breakpoint_ops *ops;
> - const char *arg_cp = arg;
> struct event_location *location;
> struct cleanup *cleanup;
>
> @@ -10076,7 +10075,8 @@ break_command_1 (char *arg, int flag, int from_tty)
> cleanup = make_cleanup_delete_event_location (location);
>
> /* Matching breakpoints on probes. */
> - if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
> + if (location != NULL
> + && EVENT_LOCATION_TYPE (location) == PROBE_LOCATION)
> ops = &bkpt_probe_breakpoint_ops;
> else
> ops = &bkpt_breakpoint_ops;
> @@ -15432,11 +15432,11 @@ trace_command (char *arg, int from_tty)
> struct breakpoint_ops *ops;
> struct event_location *location;
> struct cleanup *back_to;
> - const char *arg_cp = arg;
>
> 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)
> + if (location != NULL
> + && EVENT_LOCATION_TYPE (location) == PROBE_LOCATION)
> ops = &tracepoint_probe_breakpoint_ops;
> else
> ops = &tracepoint_breakpoint_ops;
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 8ea0c7b..deda86e 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2433,6 +2433,11 @@ event_location_to_sals (linespec_parser *parser,
> EVENT_LOCATION_ADDRESS (location));
> break;
>
> + case PROBE_LOCATION:
> + /* Probes are handled by their own decoders. */
> + gdb_assert_not_reached ("attempt to decode probe location");
extra space of indentation
> + break;
> +
> default:
> gdb_assert_not_reached ("unhandled event location type");
> }
> diff --git a/gdb/location.c b/gdb/location.c
> index 3c430db..eafa27d 100644
> --- a/gdb/location.c
> +++ b/gdb/location.c
> @@ -67,6 +67,21 @@ new_address_location (CORE_ADDR addr)
> return location;
> }
>
> +/* Create a new probe location. The return result is malloc'd
> + and should be freed with delete_event_location. */
> +
> +struct event_location *
> +new_probe_location (const char *probe)
> +{
> + struct event_location *location;
> +
> + location = XNEW (struct event_location);
> + initialize_event_location (location, PROBE_LOCATION);
> + if (probe != NULL)
> + EVENT_LOCATION_PROBE (location) = xstrdup (probe);
> + return location;
> +}
> +
> /* Return a copy of the given SRC location. */
>
> struct event_location *
> @@ -89,6 +104,10 @@ copy_event_location (const struct event_location *src)
> EVENT_LOCATION_ADDRESS (dst) = EVENT_LOCATION_ADDRESS (src);
> break;
>
> + case PROBE_LOCATION:
> + EVENT_LOCATION_PROBE (dst) = xstrdup (EVENT_LOCATION_PROBE (src));
> + break;
> +
> default:
> gdb_assert_not_reached ("unknown event location type");
> }
> @@ -133,6 +152,10 @@ delete_event_location (struct event_location *location)
> /* Nothing to do. */
> break;
>
> + case PROBE_LOCATION:
> + xfree (EVENT_LOCATION_PROBE (location));
> + break;
> +
> default:
> gdb_assert_not_reached ("unknown event location type");
> }
> @@ -166,6 +189,10 @@ event_location_to_string_const (const struct event_location *location)
> core_addr_to_string (EVENT_LOCATION_ADDRESS (location)));
> break;
>
> + case PROBE_LOCATION:
> + result = xstrdup (EVENT_LOCATION_PROBE (location));
> + break;
> +
> default:
> gdb_assert_not_reached ("unknown event location type");
> }
> @@ -212,10 +239,23 @@ string_to_event_location (char **stringp,
> }
> else
> {
> - /* Everything else is a linespec. */
> - location = new_linespec_location (*stringp);
> - if (*stringp != NULL)
> - *stringp += strlen (*stringp);
> + const char *cs;
> +
> + /* Next, try the input as a probe spec. */
> + cs = *stringp;
> + if (cs != NULL && probe_linespec_to_ops (&cs) != NULL)
> + {
> + location = new_probe_location (*stringp);
> + if (*stringp != NULL)
This test is unnecessary (right?).
btw, when will this function be passed *stringp == NULL?
Can this function have an "early exit" if *stringp == NULL?
[assuming that makes sense]
> + *stringp += strlen (*stringp);
> + }
> + else
> + {
> + /* Everything else is a linespec. */
> + location = new_linespec_location (*stringp);
> + if (*stringp != NULL)
> + *stringp += strlen (*stringp);
> + }
> }
>
> return location;
> @@ -234,6 +274,9 @@ event_location_empty_p (const struct event_location *location)
> case ADDRESS_LOCATION:
> return 0;
>
> + case PROBE_LOCATION:
> + return EVENT_LOCATION_PROBE (location) == NULL;
> +
> default:
> gdb_assert_not_reached ("unknown event location type");
> }
> diff --git a/gdb/location.h b/gdb/location.h
> index 1daefb6..e6f14d9 100644
> --- a/gdb/location.h
> +++ b/gdb/location.h
> @@ -30,7 +30,10 @@ enum event_location_type
> LINESPEC_LOCATION,
>
> /* An address in the inferior. */
> - ADDRESS_LOCATION
> + ADDRESS_LOCATION,
> +
> + /* A probe location. */
> + PROBE_LOCATION
> };
>
> /* An event location used to set a stop event in the inferior.
> @@ -50,6 +53,7 @@ struct event_location
> probes. */
> char *addr_string;
> #define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
> +#define EVENT_LOCATION_PROBE(S) ((S)->u.addr_string)
>
> /* An address in the inferior. */
> CORE_ADDR address;
> @@ -98,6 +102,12 @@ extern struct event_location *
> extern struct event_location *
> new_address_location (CORE_ADDR addr);
>
> +/* Create a new probe location. The return result is malloc'd
> + and should be freed with delete_event_location. */
> +
> +extern struct event_location *
> + new_probe_location (const char *probe);
> +
> /* Return a copy of the given SRC location. */
>
> extern struct event_location *
> diff --git a/gdb/probe.c b/gdb/probe.c
> index c7597d9..2ff5d86 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
> result.sals = NULL;
> result.nelts = 0;
>
> - arg_start = EVENT_LOCATION_LINESPEC (location);
> + gdb_assert (EVENT_LOCATION_TYPE (location) == PROBE_LOCATION);
> + arg_start = EVENT_LOCATION_PROBE (location);
>
> cs = arg_start;
> probe_ops = probe_linespec_to_ops (&cs);
> @@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
> {
> canonical->special_display = 1;
> canonical->pre_expanded = 1;
> - canonical->location = new_linespec_location (NULL);
> - EVENT_LOCATION_LINESPEC (canonical->location)
> + canonical->location = new_probe_location (NULL);
> + EVENT_LOCATION_PROBE (canonical->location)
> = savestring (arg_start, arg_end - arg_start);
I see why you pass NULL to new_probe_location and then set EVENT_LOCATION_PROBE
afterwards (otherwise two copies of the string would be malloc'd, and you'd
need to deal with freeing one of them). One could add a version of
new_probe_location that took a char* and a length, but that seems excessive.
Another place where c++ would allow cleaner code.
> }
>
> - EVENT_LOCATION_LINESPEC (location) = arg_end;
> + EVENT_LOCATION_PROBE (location) = arg_end;
> do_cleanups (cleanup);
The function starts out with:
arg_start = EVENT_LOCATION_PROBE (location);
and at the end we do this:
EVENT_LOCATION_PROBE (location) = arg_end;
IWBN to document why we do this, it's not obvious to me why that is.
[doesn't have to be part of this patch set though]
>
> return result;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFA 5/9] Explicit locations v2 - Add probe locations
2014-08-02 23:39 ` Doug Evans
@ 2014-08-03 0:22 ` Doug Evans
2014-09-03 19:32 ` Keith Seitz
1 sibling, 0 replies; 5+ messages in thread
From: Doug Evans @ 2014-08-03 0:22 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml
On Sat, Aug 2, 2014 at 4:38 PM, Doug Evans <dje@google.com> wrote:
> [...]
> > diff --git a/gdb/probe.c b/gdb/probe.c
> > index c7597d9..2ff5d86 100644
> > --- a/gdb/probe.c
> > +++ b/gdb/probe.c
> > @@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
> > result.sals = NULL;
> > result.nelts = 0;
> >
> > - arg_start = EVENT_LOCATION_LINESPEC (location);
> > + gdb_assert (EVENT_LOCATION_TYPE (location) == PROBE_LOCATION);
> > + arg_start = EVENT_LOCATION_PROBE (location);
> >
> > cs = arg_start;
> > probe_ops = probe_linespec_to_ops (&cs);
> > @@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
> > {
> > canonical->special_display = 1;
> > canonical->pre_expanded = 1;
> > - canonical->location = new_linespec_location (NULL);
> > - EVENT_LOCATION_LINESPEC (canonical->location)
> > + canonical->location = new_probe_location (NULL);
> > + EVENT_LOCATION_PROBE (canonical->location)
> > = savestring (arg_start, arg_end - arg_start);
>
> I see why you pass NULL to new_probe_location and then set EVENT_LOCATION_PROBE
> afterwards (otherwise two copies of the string would be malloc'd, and you'd
> need to deal with freeing one of them). One could add a version of
> new_probe_location that took a char* and a length, but that seems excessive.
> Another place where c++ would allow cleaner code.
OTOH, is that, or any other variation, excessive?
It's preferable to not have users of object foo know how to physically
construct a foo.
/just a thought for discussion's sake - I wouldn't change the patch if
this is but one of many similar instances, unless it makes sense to
change all of them.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFA 5/9] Explicit locations v2 - Add probe locations
2014-08-02 23:39 ` Doug Evans
2014-08-03 0:22 ` Doug Evans
@ 2014-09-03 19:32 ` Keith Seitz
1 sibling, 0 replies; 5+ 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: 3038 bytes --]
On 08/02/2014 04:38 PM, Doug Evans wrote:
> > (event_location_empty_p): Handel probe locations.
>
> "Handle ...", or just "Likewise."
Fixed.
> > + case PROBE_LOCATION:
> > + /* Probes are handled by their own decoders. */
> > + gdb_assert_not_reached ("attempt to decode probe location");
>
> extra space of indentation
Fixed.
> > + if (cs != NULL && probe_linespec_to_ops (&cs) != NULL)
> > + {
> > + location = new_probe_location (*stringp);
> > + if (*stringp != NULL)
>
> This test is unnecessary (right?).
Yes, that's correct. I've removed it.
> btw, when will this function be passed *stringp == NULL?
> Can this function have an "early exit" if *stringp == NULL?
> [assuming that makes sense]
NULL is a valid linespec, e.g, a user does:
(gdb) start
(gdb) break
Breakpoint 1, ...
As to an early exit, ... I didn't implement it. If you would like me to,
just say the word!
> > @@ -173,12 +174,12 @@ parse_probes (struct event_location *location,
> > {
> > canonical->special_display = 1;
> > canonical->pre_expanded = 1;
> > - canonical->location = new_linespec_location (NULL);
> > - EVENT_LOCATION_LINESPEC (canonical->location)
> > + canonical->location = new_probe_location (NULL);
> > + EVENT_LOCATION_PROBE (canonical->location)
> > = savestring (arg_start, arg_end - arg_start);
>
> I see why you pass NULL to new_probe_location and then set EVENT_LOCATION_PROBE
> afterwards (otherwise two copies of the string would be malloc'd, and you'd
> need to deal with freeing one of them). One could add a version of
> new_probe_location that took a char* and a length, but that seems excessive.
> Another place where c++ would allow cleaner code.
As mentioned in patch #4, I've fixed in the prescribed manner (free the
savestring after calling the ctor).
>
> > }
> >
> > - EVENT_LOCATION_LINESPEC (location) = arg_end;
> > + EVENT_LOCATION_PROBE (location) = arg_end;
> > do_cleanups (cleanup);
>
> The function starts out with:
>
> arg_start = EVENT_LOCATION_PROBE (location);
>
> and at the end we do this:
>
> EVENT_LOCATION_PROBE (location) = arg_end;
>
> IWBN to document why we do this, it's not obvious to me why that is.
> [doesn't have to be part of this patch set though]
This now uses the more explicit API functions to do this,
event_location_advance_ptr and added a comment. [Of course, this is part
of the "skip past any processed input" paradigm that we have to deal
with for linespecs/probes.]
>
> >
> > return result;
>
Updated patch attached.
Keith
Changes since last revision:
- Fix "Handel" in ChangeLog
- Remove leading whitespace
- Remove unnecessary NULL check in string_to_event_location
- Pass savestring() to new_probe_location instead of
NULL and accessing struct directly
- Use functions instead of EVENT_LOCAITON_* macros.
- Add comment and use event_location_advance_ptr to "skip"
past processed input in parse_probes
[-- Attachment #2: explicit-probe-locations.patch --]
[-- Type: text/x-patch, Size: 9809 bytes --]
gdb/ChangeLog:
* break-catch-throw.c (re_set_exception_catchpoint): Convert
linespec for stap probe to probe location.
* breakpoint.c (create_longjmp_master_breakpoint): Likewise.
(create_exception_master_breakpoint): Likewise.
(break_command_1): Remove local variable `arg_cp'.
Check location type to set appropriate breakpoint ops methods.
(trace_command): Likewise.
* linespec.c (event_location_to_sals): Handle probe locations.
* location.c (copy_event_location): Likewise.
(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 PROBE_LOCATION.
* probe.c (parse_probes): Assert that LOCATION is a probe location.
Convert linespec into probe location.
---
gdb/break-catch-throw.c | 2 +
gdb/breakpoint.c | 12 ++++-----
gdb/linespec.c | 5 ++++
gdb/location.c | 64 ++++++++++++++++++++++++++++++++++++++++++++---
gdb/location.h | 17 ++++++++++++
gdb/probe.c | 7 +++--
6 files changed, 92 insertions(+), 15 deletions(-)
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 098ab1a..57b18ad 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -212,7 +212,7 @@ re_set_exception_catchpoint (struct breakpoint *self)
/* We first try to use the probe interface. */
location
- = new_linespec_location (ASTRDUP (exception_functions[kind].probe));
+ = new_probe_location (ASTRDUP (exception_functions[kind].probe));
cleanup = make_cleanup_delete_event_location (location);
copy_location = copy_event_location_tmp (location);
make_cleanup (xfree, copy_location);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 395e0f4..0fceeac 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3387,7 +3387,7 @@ create_longjmp_master_breakpoint (void)
bp_longjmp_master,
&internal_breakpoint_ops);
b->location
- = new_linespec_location ("-probe-stap libc:longjmp");
+ = new_probe_location ("-probe-stap libc:longjmp");
b->enable_state = bp_disabled;
}
@@ -3551,7 +3551,7 @@ create_exception_master_breakpoint (void)
bp_exception_master,
&internal_breakpoint_ops);
b->location
- = new_linespec_location ("-probe-stap libgcc:unwind");
+ = new_probe_location ("-probe-stap libgcc:unwind");
b->enable_state = bp_disabled;
}
@@ -10082,7 +10082,6 @@ break_command_1 (char *arg, int flag, int from_tty)
? bp_hardware_breakpoint
: bp_breakpoint);
struct breakpoint_ops *ops;
- const char *arg_cp = arg;
struct event_location *location;
struct cleanup *cleanup;
@@ -10090,7 +10089,8 @@ break_command_1 (char *arg, int flag, int from_tty)
cleanup = make_cleanup_delete_event_location (location);
/* Matching breakpoints on probes. */
- if (arg_cp != NULL && probe_linespec_to_ops (&arg_cp) != NULL)
+ if (location != NULL
+ && event_location_type (location) == PROBE_LOCATION)
ops = &bkpt_probe_breakpoint_ops;
else
ops = &bkpt_breakpoint_ops;
@@ -15453,11 +15453,11 @@ trace_command (char *arg, int from_tty)
struct breakpoint_ops *ops;
struct event_location *location;
struct cleanup *back_to;
- const char *arg_cp = arg;
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)
+ if (location != NULL
+ && event_location_type (location) == PROBE_LOCATION)
ops = &tracepoint_probe_breakpoint_ops;
else
ops = &tracepoint_breakpoint_ops;
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7048d02..bc4484f 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2433,6 +2433,11 @@ event_location_to_sals (linespec_parser *parser,
get_address_location (location));
break;
+ case PROBE_LOCATION:
+ /* Probes are handled by their own decoders. */
+ gdb_assert_not_reached ("attempt to decode probe location");
+ break;
+
default:
gdb_assert_not_reached ("unhandled event location type");
}
diff --git a/gdb/location.c b/gdb/location.c
index 9e86107..8d8fa89 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -45,6 +45,7 @@ struct event_location
probes. */
char *addr_string;
#define EL_LINESPEC(PTR) ((PTR)->u.addr_string)
+#define EL_PROBE(PTR) ((PTR)->u.addr_string)
/* An address in the inferior. */
CORE_ADDR address;
@@ -113,6 +114,29 @@ get_address_location (struct event_location *location)
/* See description in location.h. */
struct event_location *
+new_probe_location (const char *probe)
+{
+ struct event_location *location;
+
+ location = XCNEW (struct event_location);
+ EL_TYPE (location) = PROBE_LOCATION;
+ if (probe != NULL)
+ EL_PROBE (location) = xstrdup (probe);
+ return location;
+}
+
+/* See description in location.h. */
+
+char *
+get_probe_location (struct event_location *location)
+{
+ gdb_assert (EL_TYPE (location) == PROBE_LOCATION);
+ return EL_PROBE (location);
+}
+
+/* See description in location.h. */
+
+struct event_location *
copy_event_location (const struct event_location *src)
{
struct event_location *dst;
@@ -133,6 +157,11 @@ copy_event_location (const struct event_location *src)
EL_ADDRESS (dst) = EL_ADDRESS (src);
break;
+ case PROBE_LOCATION:
+ if (EL_PROBE (src) != NULL)
+ EL_PROBE (dst) = xstrdup (EL_PROBE (src));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -177,6 +206,10 @@ delete_event_location (struct event_location *location)
/* Nothing to do. */
break;
+ case PROBE_LOCATION:
+ xfree (EL_PROBE (location));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -219,6 +252,10 @@ event_location_to_string_const (const struct event_location *location)
core_addr_to_string (EL_ADDRESS (location)));
break;
+ case PROBE_LOCATION:
+ result = xstrdup (EL_PROBE (location));
+ break;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -259,10 +296,22 @@ string_to_event_location (char **stringp,
}
else
{
- /* Everything else is a linespec. */
- location = new_linespec_location (*stringp);
- if (*stringp != NULL)
- *stringp += strlen (*stringp);
+ const char *cs;
+
+ /* Next, try the input as a probe spec. */
+ cs = *stringp;
+ if (cs != NULL && probe_linespec_to_ops (&cs) != NULL)
+ {
+ location = new_probe_location (*stringp);
+ *stringp += strlen (*stringp);
+ }
+ else
+ {
+ /* Everything else is a linespec. */
+ location = new_linespec_location (*stringp);
+ if (*stringp != NULL)
+ *stringp += strlen (*stringp);
+ }
}
return location;
@@ -282,6 +331,9 @@ event_location_empty_p (const struct event_location *location)
case ADDRESS_LOCATION:
return 0;
+ case PROBE_LOCATION:
+ return EL_PROBE (location) == NULL;
+
default:
gdb_assert_not_reached ("unknown event location type");
}
@@ -294,6 +346,8 @@ void event_location_advance_input_ptr (char **inp,
{
if (EL_TYPE (location) == LINESPEC_LOCATION)
*inp = EL_LINESPEC (location);
+ else if (EL_TYPE (location) == PROBE_LOCATION)
+ *inp = EL_PROBE (location);
}
/* See description in location.h. */
@@ -303,6 +357,8 @@ advance_event_location_ptr (struct event_location *location, size_t num)
{
if (EL_TYPE (location) == LINESPEC_LOCATION)
EL_LINESPEC (location) += num;
+ else if (EL_TYPE (location) == PROBE_LOCATION)
+ EL_PROBE (location) += num;
}
/* See description in location.h. */
diff --git a/gdb/location.h b/gdb/location.h
index 14aaffa..fa4d0f5 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -31,7 +31,10 @@ enum event_location_type
LINESPEC_LOCATION,
/* An address in the inferior. */
- ADDRESS_LOCATION
+ ADDRESS_LOCATION,
+
+ /* A probe location. */
+ PROBE_LOCATION
};
/* Return the type of the given event location. */
@@ -79,6 +82,18 @@ extern struct event_location *
extern CORE_ADDR
get_address_location (struct event_location *location);
+/* Create a new probe location. The return result is malloc'd
+ and should be freed with delete_event_location. */
+
+extern struct event_location *
+ new_probe_location (const char *probe);
+
+/* Return the probe location (a string) of the given event_location
+ (which must be of type PROBE_LOCATION). */
+
+extern char *
+ get_probe_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/probe.c b/gdb/probe.c
index edf16f7..e28082e 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -58,7 +58,8 @@ parse_probes (struct event_location *location,
result.sals = NULL;
result.nelts = 0;
- arg_start = get_linespec_location (location);
+ gdb_assert (event_location_type (location) == PROBE_LOCATION);
+ arg_start = get_probe_location (location);
cs = arg_start;
probe_ops = probe_linespec_to_ops (&cs);
@@ -175,11 +176,11 @@ parse_probes (struct event_location *location,
canonical->special_display = 1;
canonical->pre_expanded = 1;
- canonical->location = new_linespec_location (canon);
+ canonical->location = new_probe_location (canon);
xfree (canon);
}
- /* Advance the location past all parsed input. */
+ /* Advance the probe location past all parsed input. */
advance_event_location_ptr (location, arg_end - arg_start);
do_cleanups (cleanup);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-03 19:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-08 18:02 [RFA 5/9] Explicit locations v2 - Add probe locations Keith Seitz
2014-05-30 18:19 ` Keith Seitz
2014-08-02 23:39 ` Doug Evans
2014-08-03 0:22 ` 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).