* [PATCH] Use "get_current_arch" instead of "get_objfile_arch" on SystemTap SDT code (and fix ARM bug)
@ 2013-12-05 6:34 Sergio Durigan Junior
2013-12-05 13:04 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-05 6:34 UTC (permalink / raw)
To: GDB Patches; +Cc: Sergio Durigan Junior
While implementing another patch and testing things on ARM, I found a
strange bug (at least initially). There was an expression on some
probe's argument that did a displacement of the "fp" register (i.e.,
"[fp, #-8]"). However, during the evaluation of the parsed expression
GDB got confused and could not access the "fp" register due to an
invalid address being returned.
After a good deal of time spent on the investigation of this, I found
that the culprit was the gdbarch being used; it did not provide the
correct number of pseudo-registers for the architecture, and thus the
internal number of the "fp" register ended up being completely wrong.
Then, I began to read the code, and found this on gdb/objfiles.h:struct
objfile_per_bfd_storage:
/* The gdbarch associated with the BFD. Note that this gdbarch is
determined solely from BFD information, without looking at target
information. The gdbarch determined from a running target may
differ from this e.g. with respect to register types and names. */
struct gdbarch *gdbarch;
Well, then the fix became obvious. I replaced all the occurrences of
"get_objfile_arch" on gdb/stap-probe.c to "get_current_arch", and voilà .
This patch has been tested on both x86_64 Fedora 17 and ARM Fedora 19.
2013-12-05 Sergio Durigan Junior <sergiodj@redhat.com>
* stap-probe.c (stap_parse_probe_arguments): Use "get_current_arch"
instead of "get_objfile_arch".
(stap_can_evaluate_probe_arguments): Likewise.
(handle_stap_probe): Likewise.
(stap_gen_info_probes_table_values): Likewise.
---
gdb/ChangeLog | 8 ++++++++
gdb/stap-probe.c | 8 ++++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5ba7d3..ec56b57 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2013-12-05 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ * stap-probe.c (stap_parse_probe_arguments): Use "get_current_arch"
+ instead of "get_objfile_arch".
+ (stap_can_evaluate_probe_arguments): Likewise.
+ (handle_stap_probe): Likewise.
+ (stap_gen_info_probes_table_values): Likewise.
+
2013-12-05 Joel Brobecker <brobecker@adacore.com>
Tristan Gingold <gingold@adacore.com>
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index a734793..3ab54b0 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -917,7 +917,7 @@ static void
stap_parse_probe_arguments (struct stap_probe *probe)
{
const char *cur;
- struct gdbarch *gdbarch = get_objfile_arch (probe->p.objfile);
+ struct gdbarch *gdbarch = get_current_arch ();
gdb_assert (!probe->args_parsed);
cur = probe->args_u.text;
@@ -1086,7 +1086,7 @@ static int
stap_can_evaluate_probe_arguments (struct probe *probe_generic)
{
struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
- struct gdbarch *gdbarch = get_objfile_arch (stap_probe->p.objfile);
+ struct gdbarch *gdbarch = get_current_arch ();
/* For SystemTap probes, we have to guarantee that the method
stap_is_single_operand is defined on gdbarch. If it is not, then it
@@ -1337,7 +1337,7 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
{
bfd *abfd = objfile->obfd;
int size = bfd_get_arch_size (abfd) / 8;
- struct gdbarch *gdbarch = get_objfile_arch (objfile);
+ struct gdbarch *gdbarch = get_current_arch ();
struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
CORE_ADDR base_ref;
const char *probe_args = NULL;
@@ -1540,7 +1540,7 @@ stap_gen_info_probes_table_values (struct probe *probe_generic,
gdb_assert (probe_generic->pops == &stap_probe_ops);
- gdbarch = get_objfile_arch (probe->p.objfile);
+ gdbarch = get_current_arch ();
if (probe->sem_addr)
val = print_core_address (gdbarch, probe->sem_addr);
--
1.7.11.7
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Use "get_current_arch" instead of "get_objfile_arch" on SystemTap SDT code (and fix ARM bug)
2013-12-05 6:34 [PATCH] Use "get_current_arch" instead of "get_objfile_arch" on SystemTap SDT code (and fix ARM bug) Sergio Durigan Junior
@ 2013-12-05 13:04 ` Pedro Alves
2013-12-05 22:12 ` [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...) Sergio Durigan Junior
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2013-12-05 13:04 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches
On 12/05/2013 06:33 AM, Sergio Durigan Junior wrote:
> While implementing another patch and testing things on ARM, I found a
> strange bug (at least initially). There was an expression on some
> probe's argument that did a displacement of the "fp" register (i.e.,
> "[fp, #-8]"). However, during the evaluation of the parsed expression
> GDB got confused and could not access the "fp" register due to an
> invalid address being returned.
Adding get_current_arch calls outside initial command evaluation
raises eyebrows.
/* Return "current" architecture. If the target is running, this is the
architecture of the selected frame. Otherwise, the "current" architecture
defaults to the target architecture.
This function should normally be called solely by the command interpreter
routines to determine the architecture to execute a command in. */
extern struct gdbarch *get_current_arch (void);
struct value *
probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
{
...
return evaluate_probe_argument (probe, n);
}
FRAME here is the context the probe is being evaluated
in (get_current_frame(), the frame/thread that hit the event,
passed from infrun.) It'd be better to use the frame's arch
explicitly (that is, pass down the frame or arch to
evaluate_probe_argument).
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...)
2013-12-05 13:04 ` Pedro Alves
@ 2013-12-05 22:12 ` Sergio Durigan Junior
2013-12-05 23:06 ` [PATCH] Sanitize gdbarch access on probe/SDT API Sergio Durigan Junior
2013-12-06 13:50 ` Pedro Alves
0 siblings, 2 replies; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-05 22:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches, Tom Tromey
On Thursday, December 05 2013, Pedro Alves wrote:
> Adding get_current_arch calls outside initial command evaluation
> raises eyebrows.
>
> /* Return "current" architecture. If the target is running, this is the
> architecture of the selected frame. Otherwise, the "current" architecture
> defaults to the target architecture.
>
> This function should normally be called solely by the command interpreter
> routines to determine the architecture to execute a command in. */
> extern struct gdbarch *get_current_arch (void);
Hm, thanks for the hint, I wasn't aware of such limitation/restriction.
> struct value *
> probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
> {
> ...
> return evaluate_probe_argument (probe, n);
> }
>
> FRAME here is the context the probe is being evaluated
> in (get_current_frame(), the frame/thread that hit the event,
> passed from infrun.) It'd be better to use the frame's arch
> explicitly (that is, pass down the frame or arch to
> evaluate_probe_argument).
Right, could you please take a look at the following approach and tell
me what you think?
I tried to use get_selected_frame whenever I could, and when I couldn't,
I used get_current_regcache.
Tom, you asked me to try to use the expression's gdbarch for the
compile_ax method, but as it turns out, the code would be too messy if I
did that, so I have chosen to simplify it and use the agent_expr's
gdbarch instead.
I tested this patch on x86_64 Fedora 17 and ARM Fedora 19, and it works
OK.
Thanks,
--
Sergio
2013-12-06 Sergio Durigan Junior <sergiodj@redhat.com>
* break-catch-throw.c (fetch_probe_arguments): Use gdbarch from
selected frame. Pass gdbarch to sym_get_probe_argument_count and
sym_evaluate_probe_argument.
* elfread.c (elf_get_probe_argument_count): Adjust declaration to
accept gdbarch. Pass gdbarch to get_probe_argument_count.
(elf_can_evaluate_probe_arguments): Likewise, but pass gdbarch to
can_evaluate_probe_arguments.
(elf_evaluate_probe_argument): Likewise, but pass gdbarch to
evaluate_probe_argument.
* probe.c (get_probe_argument_count): Use gdbarch from selected
frame. Pass gdbarch to sym_get_probe_argument_count.
(can_evaluate_probe_arguments): Likewise, but pass gdbarch to
can_evaluate_probe_arguments.
(evaluate_probe_argument): Likewise, but pass gdbarch to
sym_evaluate_probe_argument.
* probe.h (struct probe_ops) <get_probe_argument_count,
can_evaluate_probe_arguments, evaluate_probe_argument>: Adjust
declaration to accept gdbarch.
* stap-probe.c: Include "regcache.h".
(stap_parse_probe_arguments): Adjust declaration to accept
gdbarch.
(stap_get_probe_argument_count): Likewise. Pass gdbarch to
can_evaluate_probe_arguments and stap_parse_probe_arguments.
(stap_get_arg): Likewise, but pass gdbarch to
stap_parse_probe_arguments.
(stap_can_evaluate_probe_arguments): Adjust declaration to accept
gdbarch.
(stap_evaluate_probe_argument): Likewise. Pass gdbarch to
stap_get_arg.
(stap_compile_to_ax): Use agent_expr's gdbarch; pass it to
stap_get_arg.
(compute_probe_arg): Use gdbarch from selected frame. Pass
gdbarch to sym_get_probe_argument_count and
sym_evaluate_probe_argument.
(compile_probe_arg): Use gdbarch from agent_expr. Pass it to
sym_get_probe_argument_count.
(handle_stap_probe): Adjust declaration to accept gdbarch.
(stap_get_probes): Use gdbarch from curret regcache. Pass it to
handle_stap_probe.
(stap_gen_info_probes_table_values): Use gdbarch from selected
frame.
* symfile-debug.c (debug_sym_get_probe_argument_count): Adjust
declaration to accept gdbarch. Pass it to
sym_get_probe_argument_count.
(debug_can_evaluate_probe_arguments): Likewise, but pass gdbarch
to can_evaluate_probe_arguments.
(debug_sym_evaluate_probe_argument): Likewise, but pass gdbarch to
sym_evaluate_probe_argument.
* symfile.h (struct sym_probe_fns) <sym_get_probe_argument_count,
can_evaluate_probe_arguments, sym_evaluate_probe_argument>: Adjust
declaration to accept gdbarch.
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index fb24725..769559b 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -105,6 +105,7 @@ static void
fetch_probe_arguments (struct value **arg0, struct value **arg1)
{
struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
CORE_ADDR pc = get_frame_pc (frame);
struct probe *pc_probe;
const struct sym_probe_fns *pc_probe_fns;
@@ -123,13 +124,13 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1)
gdb_assert (pc_probe->objfile->sf->sym_probe_fns != NULL);
pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
- n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
+ n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
if (n_args < 2)
error (_("C++ exception catchpoint has too few arguments"));
if (arg0 != NULL)
- *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0);
- *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1);
+ *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0, gdbarch);
+ *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1, gdbarch);
if ((arg0 != NULL && *arg0 == NULL) || *arg1 == NULL)
error (_("error computing probe argument at c++ exception catchpoint"));
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 42ab18f..b72cd96 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1509,27 +1509,30 @@ elf_get_probes (struct objfile *objfile)
symfile.h. */
static unsigned
-elf_get_probe_argument_count (struct probe *probe)
+elf_get_probe_argument_count (struct probe *probe,
+ struct gdbarch *gdbarch)
{
- return probe->pops->get_probe_argument_count (probe);
+ return probe->pops->get_probe_argument_count (probe, gdbarch);
}
/* Implementation of `sym_can_evaluate_probe_arguments', as documented in
symfile.h. */
static int
-elf_can_evaluate_probe_arguments (struct probe *probe)
+elf_can_evaluate_probe_arguments (struct probe *probe,
+ struct gdbarch *gdbarch)
{
- return probe->pops->can_evaluate_probe_arguments (probe);
+ return probe->pops->can_evaluate_probe_arguments (probe, gdbarch);
}
/* Implementation of `sym_evaluate_probe_argument', as documented in
symfile.h. */
static struct value *
-elf_evaluate_probe_argument (struct probe *probe, unsigned n)
+elf_evaluate_probe_argument (struct probe *probe, unsigned n,
+ struct gdbarch *gdbarch)
{
- return probe->pops->evaluate_probe_argument (probe, n);
+ return probe->pops->evaluate_probe_argument (probe, n, gdbarch);
}
/* Implementation of `sym_compile_to_ax', as documented in symfile.h. */
diff --git a/gdb/probe.c b/gdb/probe.c
index 4046701..37314dd 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -617,6 +617,8 @@ unsigned
get_probe_argument_count (struct probe *probe)
{
const struct sym_probe_fns *probe_fns;
+ struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
gdb_assert (probe->objfile != NULL);
gdb_assert (probe->objfile->sf != NULL);
@@ -625,7 +627,7 @@ get_probe_argument_count (struct probe *probe)
gdb_assert (probe_fns != NULL);
- return probe_fns->sym_get_probe_argument_count (probe);
+ return probe_fns->sym_get_probe_argument_count (probe, gdbarch);
}
/* See comments in probe.h. */
@@ -634,6 +636,8 @@ int
can_evaluate_probe_arguments (struct probe *probe)
{
const struct sym_probe_fns *probe_fns;
+ struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
gdb_assert (probe->objfile != NULL);
gdb_assert (probe->objfile->sf != NULL);
@@ -642,7 +646,7 @@ can_evaluate_probe_arguments (struct probe *probe)
gdb_assert (probe_fns != NULL);
- return probe_fns->can_evaluate_probe_arguments (probe);
+ return probe_fns->can_evaluate_probe_arguments (probe, gdbarch);
}
/* See comments in probe.h. */
@@ -651,6 +655,8 @@ struct value *
evaluate_probe_argument (struct probe *probe, unsigned n)
{
const struct sym_probe_fns *probe_fns;
+ struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
gdb_assert (probe->objfile != NULL);
gdb_assert (probe->objfile->sf != NULL);
@@ -659,7 +665,7 @@ evaluate_probe_argument (struct probe *probe, unsigned n)
gdb_assert (probe_fns != NULL);
- return probe_fns->sym_evaluate_probe_argument (probe, n);
+ return probe_fns->sym_evaluate_probe_argument (probe, n, gdbarch);
}
/* See comments in probe.h. */
diff --git a/gdb/probe.h b/gdb/probe.h
index dd5387b..6a08f3d 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -71,19 +71,22 @@ struct probe_ops
/* Return the number of arguments of PROBE. */
- unsigned (*get_probe_argument_count) (struct probe *probe);
+ unsigned (*get_probe_argument_count) (struct probe *probe,
+ struct gdbarch *gdbarch);
/* Return 1 if the probe interface can evaluate the arguments of probe
PROBE, zero otherwise. See the comments on
sym_probe_fns:can_evaluate_probe_arguments for more details. */
- int (*can_evaluate_probe_arguments) (struct probe *probe);
+ int (*can_evaluate_probe_arguments) (struct probe *probe,
+ struct gdbarch *gdbarch);
/* Evaluate the Nth argument from the PROBE, returning a value
corresponding to it. The argument number is represented N. */
struct value *(*evaluate_probe_argument) (struct probe *probe,
- unsigned n);
+ unsigned n,
+ struct gdbarch *gdbarch);
/* Compile the Nth argument of the PROBE to an agent expression.
The argument number is represented by N. */
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index a734793..e8b5805 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -38,6 +38,7 @@
#include "parser-defs.h"
#include "language.h"
#include "elf-bfd.h"
+#include "regcache.h"
#include <ctype.h>
@@ -914,10 +915,9 @@ stap_parse_argument (const char **arg, struct type *atype,
this information. */
static void
-stap_parse_probe_arguments (struct stap_probe *probe)
+stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch)
{
const char *cur;
- struct gdbarch *gdbarch = get_objfile_arch (probe->p.objfile);
gdb_assert (!probe->args_parsed);
cur = probe->args_u.text;
@@ -1002,7 +1002,8 @@ stap_parse_probe_arguments (struct stap_probe *probe)
argument string. */
static unsigned
-stap_get_probe_argument_count (struct probe *probe_generic)
+stap_get_probe_argument_count (struct probe *probe_generic,
+ struct gdbarch *gdbarch)
{
struct stap_probe *probe = (struct stap_probe *) probe_generic;
@@ -1010,8 +1011,9 @@ stap_get_probe_argument_count (struct probe *probe_generic)
if (!probe->args_parsed)
{
- if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic))
- stap_parse_probe_arguments (probe);
+ if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic,
+ gdbarch))
+ stap_parse_probe_arguments (probe, gdbarch);
else
{
static int have_warned_stap_incomplete = 0;
@@ -1072,10 +1074,10 @@ stap_is_operator (const char *op)
}
static struct stap_probe_arg *
-stap_get_arg (struct stap_probe *probe, unsigned n)
+stap_get_arg (struct stap_probe *probe, unsigned n, struct gdbarch *gdbarch)
{
if (!probe->args_parsed)
- stap_parse_probe_arguments (probe);
+ stap_parse_probe_arguments (probe, gdbarch);
return VEC_index (stap_probe_arg_s, probe->args_u.vec, n);
}
@@ -1083,10 +1085,10 @@ stap_get_arg (struct stap_probe *probe, unsigned n)
/* Implement the `can_evaluate_probe_arguments' method of probe_ops. */
static int
-stap_can_evaluate_probe_arguments (struct probe *probe_generic)
+stap_can_evaluate_probe_arguments (struct probe *probe_generic,
+ struct gdbarch *gdbarch)
{
struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
- struct gdbarch *gdbarch = get_objfile_arch (stap_probe->p.objfile);
/* For SystemTap probes, we have to guarantee that the method
stap_is_single_operand is defined on gdbarch. If it is not, then it
@@ -1098,7 +1100,8 @@ stap_can_evaluate_probe_arguments (struct probe *probe_generic)
corresponding to it. Assertion is thrown if N does not exist. */
static struct value *
-stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
+stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
+ struct gdbarch *gdbarch)
{
struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
struct stap_probe_arg *arg;
@@ -1106,7 +1109,7 @@ stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
gdb_assert (probe_generic->pops == &stap_probe_ops);
- arg = stap_get_arg (stap_probe, n);
+ arg = stap_get_arg (stap_probe, n, gdbarch);
return evaluate_subexp_standard (arg->atype, arg->aexpr, &pos, EVAL_NORMAL);
}
@@ -1123,7 +1126,7 @@ stap_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
gdb_assert (probe_generic->pops == &stap_probe_ops);
- arg = stap_get_arg (stap_probe, n);
+ arg = stap_get_arg (stap_probe, n, expr->gdbarch);
pc = arg->aexpr->elts;
gen_expr (arg->aexpr, &pc, expr, value);
@@ -1165,6 +1168,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
{
struct frame_info *frame = get_selected_frame (_("No frame selected"));
CORE_ADDR pc = get_frame_pc (frame);
+ struct gdbarch *gdbarch = get_frame_arch (frame);
int sel = (int) (uintptr_t) data;
struct probe *pc_probe;
const struct sym_probe_fns *pc_probe_fns;
@@ -1183,7 +1187,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
- n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
+ n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
if (sel == -1)
return value_from_longest (builtin_type (arch)->builtin_int, n_args);
@@ -1191,7 +1195,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
error (_("Invalid probe argument %d -- probe has %u arguments available"),
sel, n_args);
- return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel);
+ return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel, gdbarch);
}
/* This is called to compile one of the $_probe_arg* convenience
@@ -1220,7 +1224,8 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
- n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
+ n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe,
+ expr->gdbarch);
if (sel == -1)
{
@@ -1333,11 +1338,11 @@ static const struct internalvar_funcs probe_funcs =
static void
handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
- VEC (probe_p) **probesp, CORE_ADDR base)
+ VEC (probe_p) **probesp, CORE_ADDR base,
+ struct gdbarch *gdbarch)
{
bfd *abfd = objfile->obfd;
int size = bfd_get_arch_size (abfd) / 8;
- struct gdbarch *gdbarch = get_objfile_arch (objfile);
struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
CORE_ADDR base_ref;
const char *probe_args = NULL;
@@ -1461,6 +1466,8 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
bfd_vma base;
struct sdt_note *iter;
unsigned save_probesp_len = VEC_length (probe_p, *probesp);
+ struct regcache *regcache = get_current_regcache ();
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
if (objfile->separate_debug_objfile_backlink != NULL)
{
@@ -1486,7 +1493,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
{
/* We first have to handle all the information about the
probe which is present in the section. */
- handle_stap_probe (objfile, iter, probesp, base);
+ handle_stap_probe (objfile, iter, probesp, base, gdbarch);
}
if (save_probesp_len == VEC_length (probe_p, *probesp))
@@ -1535,13 +1542,12 @@ stap_gen_info_probes_table_values (struct probe *probe_generic,
VEC (const_char_ptr) **ret)
{
struct stap_probe *probe = (struct stap_probe *) probe_generic;
- struct gdbarch *gdbarch;
+ struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
const char *val = NULL;
gdb_assert (probe_generic->pops == &stap_probe_ops);
- gdbarch = get_objfile_arch (probe->p.objfile);
-
if (probe->sem_addr)
val = print_core_address (gdbarch, probe->sem_addr);
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index b6e84d1..eb6f000 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -393,7 +393,8 @@ debug_sym_get_probes (struct objfile *objfile)
}
static unsigned
-debug_sym_get_probe_argument_count (struct probe *probe)
+debug_sym_get_probe_argument_count (struct probe *probe,
+ struct gdbarch *gdbarch)
{
struct objfile *objfile = probe->objfile;
const struct debug_sym_fns_data *debug_data =
@@ -401,7 +402,7 @@ debug_sym_get_probe_argument_count (struct probe *probe)
unsigned retval;
retval = debug_data->real_sf->sym_probe_fns->sym_get_probe_argument_count
- (probe);
+ (probe, gdbarch);
fprintf_filtered (gdb_stdlog,
"probes->sym_get_probe_argument_count (%s) = %u\n",
@@ -411,7 +412,8 @@ debug_sym_get_probe_argument_count (struct probe *probe)
}
static int
-debug_can_evaluate_probe_arguments (struct probe *probe)
+debug_can_evaluate_probe_arguments (struct probe *probe,
+ struct gdbarch *gdbarch)
{
struct objfile *objfile = probe->objfile;
const struct debug_sym_fns_data *debug_data =
@@ -419,7 +421,7 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
int retval;
retval = debug_data->real_sf->sym_probe_fns->can_evaluate_probe_arguments
- (probe);
+ (probe, gdbarch);
fprintf_filtered (gdb_stdlog,
"probes->can_evaluate_probe_arguments (%s) = %d\n",
@@ -429,7 +431,8 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
}
static struct value *
-debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
+debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n,
+ struct gdbarch *gdbarch)
{
struct objfile *objfile = probe->objfile;
const struct debug_sym_fns_data *debug_data =
@@ -441,7 +444,7 @@ debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
host_address_to_string (probe), n);
retval = debug_data->real_sf->sym_probe_fns->sym_evaluate_probe_argument
- (probe, n);
+ (probe, n, gdbarch);
fprintf_filtered (gdb_stdlog,
"probes->sym_evaluate_probe_argument (...) = %s\n",
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 8e5909b..4c4a0dd 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -309,7 +309,8 @@ struct sym_probe_fns
have come from a call to this objfile's sym_get_probes method.
If you provide an implementation of sym_get_probes, you must
implement this method as well. */
- unsigned (*sym_get_probe_argument_count) (struct probe *probe);
+ unsigned (*sym_get_probe_argument_count) (struct probe *probe,
+ struct gdbarch *gdbarch);
/* Return 1 if the probe interface can evaluate the arguments of probe
PROBE, zero otherwise. This function can be probe-specific, informing
@@ -317,7 +318,8 @@ struct sym_probe_fns
informing whether the probe interface is able to evaluate any kind of
argument. If you provide an implementation of sym_get_probes, you must
implement this method as well. */
- int (*can_evaluate_probe_arguments) (struct probe *probe);
+ int (*can_evaluate_probe_arguments) (struct probe *probe,
+ struct gdbarch *gdbarch);
/* Evaluate the Nth argument available to PROBE. PROBE will have
come from a call to this objfile's sym_get_probes method. N will
@@ -327,7 +329,8 @@ struct sym_probe_fns
implementation of sym_get_probes, you must implement this method
as well. */
struct value *(*sym_evaluate_probe_argument) (struct probe *probe,
- unsigned n);
+ unsigned n,
+ struct gdbarch *gdbarch);
/* Compile the Nth probe argument to an agent expression. PROBE
will have come from a call to this objfile's sym_get_probes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-05 22:12 ` [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...) Sergio Durigan Junior
@ 2013-12-05 23:06 ` Sergio Durigan Junior
2013-12-05 23:30 ` Tom Tromey
2013-12-06 13:50 ` Pedro Alves
1 sibling, 1 reply; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-05 23:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches, Tom Tromey
On Thursday, December 05 2013, I wrote:
> Right, could you please take a look at the following approach and tell
> me what you think?
As it turns out, I saw Tom's approach to make probes program-space
independent, and my patch touches many points that are also touched by
Tom's. Therefore, I will wait until he commits his patches, and then
I'll rebase this one on top of them and resubmit.
Thanks,
> I tried to use get_selected_frame whenever I could, and when I couldn't,
> I used get_current_regcache.
>
> Tom, you asked me to try to use the expression's gdbarch for the
> compile_ax method, but as it turns out, the code would be too messy if I
> did that, so I have chosen to simplify it and use the agent_expr's
> gdbarch instead.
>
> I tested this patch on x86_64 Fedora 17 and ARM Fedora 19, and it works
> OK.
>
> Thanks,
>
> --
> Sergio
>
> 2013-12-06 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * break-catch-throw.c (fetch_probe_arguments): Use gdbarch from
> selected frame. Pass gdbarch to sym_get_probe_argument_count and
> sym_evaluate_probe_argument.
> * elfread.c (elf_get_probe_argument_count): Adjust declaration to
> accept gdbarch. Pass gdbarch to get_probe_argument_count.
> (elf_can_evaluate_probe_arguments): Likewise, but pass gdbarch to
> can_evaluate_probe_arguments.
> (elf_evaluate_probe_argument): Likewise, but pass gdbarch to
> evaluate_probe_argument.
> * probe.c (get_probe_argument_count): Use gdbarch from selected
> frame. Pass gdbarch to sym_get_probe_argument_count.
> (can_evaluate_probe_arguments): Likewise, but pass gdbarch to
> can_evaluate_probe_arguments.
> (evaluate_probe_argument): Likewise, but pass gdbarch to
> sym_evaluate_probe_argument.
> * probe.h (struct probe_ops) <get_probe_argument_count,
> can_evaluate_probe_arguments, evaluate_probe_argument>: Adjust
> declaration to accept gdbarch.
> * stap-probe.c: Include "regcache.h".
> (stap_parse_probe_arguments): Adjust declaration to accept
> gdbarch.
> (stap_get_probe_argument_count): Likewise. Pass gdbarch to
> can_evaluate_probe_arguments and stap_parse_probe_arguments.
> (stap_get_arg): Likewise, but pass gdbarch to
> stap_parse_probe_arguments.
> (stap_can_evaluate_probe_arguments): Adjust declaration to accept
> gdbarch.
> (stap_evaluate_probe_argument): Likewise. Pass gdbarch to
> stap_get_arg.
> (stap_compile_to_ax): Use agent_expr's gdbarch; pass it to
> stap_get_arg.
> (compute_probe_arg): Use gdbarch from selected frame. Pass
> gdbarch to sym_get_probe_argument_count and
> sym_evaluate_probe_argument.
> (compile_probe_arg): Use gdbarch from agent_expr. Pass it to
> sym_get_probe_argument_count.
> (handle_stap_probe): Adjust declaration to accept gdbarch.
> (stap_get_probes): Use gdbarch from curret regcache. Pass it to
> handle_stap_probe.
> (stap_gen_info_probes_table_values): Use gdbarch from selected
> frame.
> * symfile-debug.c (debug_sym_get_probe_argument_count): Adjust
> declaration to accept gdbarch. Pass it to
> sym_get_probe_argument_count.
> (debug_can_evaluate_probe_arguments): Likewise, but pass gdbarch
> to can_evaluate_probe_arguments.
> (debug_sym_evaluate_probe_argument): Likewise, but pass gdbarch to
> sym_evaluate_probe_argument.
> * symfile.h (struct sym_probe_fns) <sym_get_probe_argument_count,
> can_evaluate_probe_arguments, sym_evaluate_probe_argument>: Adjust
> declaration to accept gdbarch.
>
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index fb24725..769559b 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -105,6 +105,7 @@ static void
> fetch_probe_arguments (struct value **arg0, struct value **arg1)
> {
> struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> CORE_ADDR pc = get_frame_pc (frame);
> struct probe *pc_probe;
> const struct sym_probe_fns *pc_probe_fns;
> @@ -123,13 +124,13 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1)
> gdb_assert (pc_probe->objfile->sf->sym_probe_fns != NULL);
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
> if (n_args < 2)
> error (_("C++ exception catchpoint has too few arguments"));
>
> if (arg0 != NULL)
> - *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0);
> - *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1);
> + *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0, gdbarch);
> + *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1, gdbarch);
>
> if ((arg0 != NULL && *arg0 == NULL) || *arg1 == NULL)
> error (_("error computing probe argument at c++ exception catchpoint"));
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 42ab18f..b72cd96 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1509,27 +1509,30 @@ elf_get_probes (struct objfile *objfile)
> symfile.h. */
>
> static unsigned
> -elf_get_probe_argument_count (struct probe *probe)
> +elf_get_probe_argument_count (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->get_probe_argument_count (probe);
> + return probe->pops->get_probe_argument_count (probe, gdbarch);
> }
>
> /* Implementation of `sym_can_evaluate_probe_arguments', as documented in
> symfile.h. */
>
> static int
> -elf_can_evaluate_probe_arguments (struct probe *probe)
> +elf_can_evaluate_probe_arguments (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->can_evaluate_probe_arguments (probe);
> + return probe->pops->can_evaluate_probe_arguments (probe, gdbarch);
> }
>
> /* Implementation of `sym_evaluate_probe_argument', as documented in
> symfile.h. */
>
> static struct value *
> -elf_evaluate_probe_argument (struct probe *probe, unsigned n)
> +elf_evaluate_probe_argument (struct probe *probe, unsigned n,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->evaluate_probe_argument (probe, n);
> + return probe->pops->evaluate_probe_argument (probe, n, gdbarch);
> }
>
> /* Implementation of `sym_compile_to_ax', as documented in symfile.h. */
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 4046701..37314dd 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -617,6 +617,8 @@ unsigned
> get_probe_argument_count (struct probe *probe)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -625,7 +627,7 @@ get_probe_argument_count (struct probe *probe)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->sym_get_probe_argument_count (probe);
> + return probe_fns->sym_get_probe_argument_count (probe, gdbarch);
> }
>
> /* See comments in probe.h. */
> @@ -634,6 +636,8 @@ int
> can_evaluate_probe_arguments (struct probe *probe)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -642,7 +646,7 @@ can_evaluate_probe_arguments (struct probe *probe)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->can_evaluate_probe_arguments (probe);
> + return probe_fns->can_evaluate_probe_arguments (probe, gdbarch);
> }
>
> /* See comments in probe.h. */
> @@ -651,6 +655,8 @@ struct value *
> evaluate_probe_argument (struct probe *probe, unsigned n)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -659,7 +665,7 @@ evaluate_probe_argument (struct probe *probe, unsigned n)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->sym_evaluate_probe_argument (probe, n);
> + return probe_fns->sym_evaluate_probe_argument (probe, n, gdbarch);
> }
>
> /* See comments in probe.h. */
> diff --git a/gdb/probe.h b/gdb/probe.h
> index dd5387b..6a08f3d 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -71,19 +71,22 @@ struct probe_ops
>
> /* Return the number of arguments of PROBE. */
>
> - unsigned (*get_probe_argument_count) (struct probe *probe);
> + unsigned (*get_probe_argument_count) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Return 1 if the probe interface can evaluate the arguments of probe
> PROBE, zero otherwise. See the comments on
> sym_probe_fns:can_evaluate_probe_arguments for more details. */
>
> - int (*can_evaluate_probe_arguments) (struct probe *probe);
> + int (*can_evaluate_probe_arguments) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Evaluate the Nth argument from the PROBE, returning a value
> corresponding to it. The argument number is represented N. */
>
> struct value *(*evaluate_probe_argument) (struct probe *probe,
> - unsigned n);
> + unsigned n,
> + struct gdbarch *gdbarch);
>
> /* Compile the Nth argument of the PROBE to an agent expression.
> The argument number is represented by N. */
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index a734793..e8b5805 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -38,6 +38,7 @@
> #include "parser-defs.h"
> #include "language.h"
> #include "elf-bfd.h"
> +#include "regcache.h"
>
> #include <ctype.h>
>
> @@ -914,10 +915,9 @@ stap_parse_argument (const char **arg, struct type *atype,
> this information. */
>
> static void
> -stap_parse_probe_arguments (struct stap_probe *probe)
> +stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch)
> {
> const char *cur;
> - struct gdbarch *gdbarch = get_objfile_arch (probe->p.objfile);
>
> gdb_assert (!probe->args_parsed);
> cur = probe->args_u.text;
> @@ -1002,7 +1002,8 @@ stap_parse_probe_arguments (struct stap_probe *probe)
> argument string. */
>
> static unsigned
> -stap_get_probe_argument_count (struct probe *probe_generic)
> +stap_get_probe_argument_count (struct probe *probe_generic,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *probe = (struct stap_probe *) probe_generic;
>
> @@ -1010,8 +1011,9 @@ stap_get_probe_argument_count (struct probe *probe_generic)
>
> if (!probe->args_parsed)
> {
> - if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic))
> - stap_parse_probe_arguments (probe);
> + if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic,
> + gdbarch))
> + stap_parse_probe_arguments (probe, gdbarch);
> else
> {
> static int have_warned_stap_incomplete = 0;
> @@ -1072,10 +1074,10 @@ stap_is_operator (const char *op)
> }
>
> static struct stap_probe_arg *
> -stap_get_arg (struct stap_probe *probe, unsigned n)
> +stap_get_arg (struct stap_probe *probe, unsigned n, struct gdbarch *gdbarch)
> {
> if (!probe->args_parsed)
> - stap_parse_probe_arguments (probe);
> + stap_parse_probe_arguments (probe, gdbarch);
>
> return VEC_index (stap_probe_arg_s, probe->args_u.vec, n);
> }
> @@ -1083,10 +1085,10 @@ stap_get_arg (struct stap_probe *probe, unsigned n)
> /* Implement the `can_evaluate_probe_arguments' method of probe_ops. */
>
> static int
> -stap_can_evaluate_probe_arguments (struct probe *probe_generic)
> +stap_can_evaluate_probe_arguments (struct probe *probe_generic,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
> - struct gdbarch *gdbarch = get_objfile_arch (stap_probe->p.objfile);
>
> /* For SystemTap probes, we have to guarantee that the method
> stap_is_single_operand is defined on gdbarch. If it is not, then it
> @@ -1098,7 +1100,8 @@ stap_can_evaluate_probe_arguments (struct probe *probe_generic)
> corresponding to it. Assertion is thrown if N does not exist. */
>
> static struct value *
> -stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
> +stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
> struct stap_probe_arg *arg;
> @@ -1106,7 +1109,7 @@ stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - arg = stap_get_arg (stap_probe, n);
> + arg = stap_get_arg (stap_probe, n, gdbarch);
> return evaluate_subexp_standard (arg->atype, arg->aexpr, &pos, EVAL_NORMAL);
> }
>
> @@ -1123,7 +1126,7 @@ stap_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - arg = stap_get_arg (stap_probe, n);
> + arg = stap_get_arg (stap_probe, n, expr->gdbarch);
>
> pc = arg->aexpr->elts;
> gen_expr (arg->aexpr, &pc, expr, value);
> @@ -1165,6 +1168,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> {
> struct frame_info *frame = get_selected_frame (_("No frame selected"));
> CORE_ADDR pc = get_frame_pc (frame);
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> int sel = (int) (uintptr_t) data;
> struct probe *pc_probe;
> const struct sym_probe_fns *pc_probe_fns;
> @@ -1183,7 +1187,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
>
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
> if (sel == -1)
> return value_from_longest (builtin_type (arch)->builtin_int, n_args);
>
> @@ -1191,7 +1195,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> error (_("Invalid probe argument %d -- probe has %u arguments available"),
> sel, n_args);
>
> - return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel);
> + return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel, gdbarch);
> }
>
> /* This is called to compile one of the $_probe_arg* convenience
> @@ -1220,7 +1224,8 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
>
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe,
> + expr->gdbarch);
>
> if (sel == -1)
> {
> @@ -1333,11 +1338,11 @@ static const struct internalvar_funcs probe_funcs =
>
> static void
> handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
> - VEC (probe_p) **probesp, CORE_ADDR base)
> + VEC (probe_p) **probesp, CORE_ADDR base,
> + struct gdbarch *gdbarch)
> {
> bfd *abfd = objfile->obfd;
> int size = bfd_get_arch_size (abfd) / 8;
> - struct gdbarch *gdbarch = get_objfile_arch (objfile);
> struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> CORE_ADDR base_ref;
> const char *probe_args = NULL;
> @@ -1461,6 +1466,8 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> bfd_vma base;
> struct sdt_note *iter;
> unsigned save_probesp_len = VEC_length (probe_p, *probesp);
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
>
> if (objfile->separate_debug_objfile_backlink != NULL)
> {
> @@ -1486,7 +1493,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> {
> /* We first have to handle all the information about the
> probe which is present in the section. */
> - handle_stap_probe (objfile, iter, probesp, base);
> + handle_stap_probe (objfile, iter, probesp, base, gdbarch);
> }
>
> if (save_probesp_len == VEC_length (probe_p, *probesp))
> @@ -1535,13 +1542,12 @@ stap_gen_info_probes_table_values (struct probe *probe_generic,
> VEC (const_char_ptr) **ret)
> {
> struct stap_probe *probe = (struct stap_probe *) probe_generic;
> - struct gdbarch *gdbarch;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> const char *val = NULL;
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - gdbarch = get_objfile_arch (probe->p.objfile);
> -
> if (probe->sem_addr)
> val = print_core_address (gdbarch, probe->sem_addr);
>
> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index b6e84d1..eb6f000 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -393,7 +393,8 @@ debug_sym_get_probes (struct objfile *objfile)
> }
>
> static unsigned
> -debug_sym_get_probe_argument_count (struct probe *probe)
> +debug_sym_get_probe_argument_count (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -401,7 +402,7 @@ debug_sym_get_probe_argument_count (struct probe *probe)
> unsigned retval;
>
> retval = debug_data->real_sf->sym_probe_fns->sym_get_probe_argument_count
> - (probe);
> + (probe, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->sym_get_probe_argument_count (%s) = %u\n",
> @@ -411,7 +412,8 @@ debug_sym_get_probe_argument_count (struct probe *probe)
> }
>
> static int
> -debug_can_evaluate_probe_arguments (struct probe *probe)
> +debug_can_evaluate_probe_arguments (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -419,7 +421,7 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
> int retval;
>
> retval = debug_data->real_sf->sym_probe_fns->can_evaluate_probe_arguments
> - (probe);
> + (probe, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->can_evaluate_probe_arguments (%s) = %d\n",
> @@ -429,7 +431,8 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
> }
>
> static struct value *
> -debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
> +debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -441,7 +444,7 @@ debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
> host_address_to_string (probe), n);
>
> retval = debug_data->real_sf->sym_probe_fns->sym_evaluate_probe_argument
> - (probe, n);
> + (probe, n, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->sym_evaluate_probe_argument (...) = %s\n",
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index 8e5909b..4c4a0dd 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -309,7 +309,8 @@ struct sym_probe_fns
> have come from a call to this objfile's sym_get_probes method.
> If you provide an implementation of sym_get_probes, you must
> implement this method as well. */
> - unsigned (*sym_get_probe_argument_count) (struct probe *probe);
> + unsigned (*sym_get_probe_argument_count) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Return 1 if the probe interface can evaluate the arguments of probe
> PROBE, zero otherwise. This function can be probe-specific, informing
> @@ -317,7 +318,8 @@ struct sym_probe_fns
> informing whether the probe interface is able to evaluate any kind of
> argument. If you provide an implementation of sym_get_probes, you must
> implement this method as well. */
> - int (*can_evaluate_probe_arguments) (struct probe *probe);
> + int (*can_evaluate_probe_arguments) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Evaluate the Nth argument available to PROBE. PROBE will have
> come from a call to this objfile's sym_get_probes method. N will
> @@ -327,7 +329,8 @@ struct sym_probe_fns
> implementation of sym_get_probes, you must implement this method
> as well. */
> struct value *(*sym_evaluate_probe_argument) (struct probe *probe,
> - unsigned n);
> + unsigned n,
> + struct gdbarch *gdbarch);
>
> /* Compile the Nth probe argument to an agent expression. PROBE
> will have come from a call to this objfile's sym_get_probes
--
Sergio
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-05 23:06 ` [PATCH] Sanitize gdbarch access on probe/SDT API Sergio Durigan Junior
@ 2013-12-05 23:30 ` Tom Tromey
2013-12-06 2:22 ` Sergio Durigan Junior
0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2013-12-05 23:30 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: Pedro Alves, GDB Patches
Sergio> As it turns out, I saw Tom's approach to make probes program-space
Sergio> independent, and my patch touches many points that are also touched by
Sergio> Tom's. Therefore, I will wait until he commits his patches, and then
Sergio> I'll rebase this one on top of them and resubmit.
Don't wait -- those patches depend on the minsym series.
I don't plan to check that in until 7.7, since it seems imminent,
since the minsym series is fairly large, and since the series could
probably use more testing on hosts that I don't have access to.
Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-05 23:30 ` Tom Tromey
@ 2013-12-06 2:22 ` Sergio Durigan Junior
0 siblings, 0 replies; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-06 2:22 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, GDB Patches
On Thursday, December 05 2013, Tom Tromey wrote:
> Don't wait -- those patches depend on the minsym series.
> I don't plan to check that in until 7.7, since it seems imminent,
> since the minsym series is fairly large, and since the series could
> probably use more testing on hosts that I don't have access to.
OK, thanks, then the patch is still valid :-).
--
Sergio
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-05 22:12 ` [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...) Sergio Durigan Junior
2013-12-05 23:06 ` [PATCH] Sanitize gdbarch access on probe/SDT API Sergio Durigan Junior
@ 2013-12-06 13:50 ` Pedro Alves
2013-12-06 15:49 ` Sergio Durigan Junior
1 sibling, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2013-12-06 13:50 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Tom Tromey
> I tried to use get_selected_frame whenever I could, and when I couldn't,
> I used get_current_regcache.
But what does "couldn't" mean?
Looking at the function that uses get_current_regcache:
On 12/05/2013 10:12 PM, Sergio Durigan Junior wrote:
> @@ -1461,6 +1466,8 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> bfd_vma base;
> struct sdt_note *iter;
> unsigned save_probesp_len = VEC_length (probe_p, *probesp);
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
AFAICS, stap_get_probes isn't called when evaluating a probe,
but instead when getting the list of probes out of an objfile.
Seems to me like this function is reachable even if the inferior
is not running yet. Is that why you couldn't use
get_selected_frame here? (because there's no frame?) If so,
using get_current_regcache is wrong. If there's no thread at
all, then what does the regcache of the current thread mean?
It sounds like you just managed to use it becauese
get_current_regcache doesn't error out when inferior_ptid
is pointing nowhere. As this is listing the probes in
the objfile, and not using the target's registers (afaics),
can you use the objfile's arch here? With that out of the way,
would it work to pass the frame pointer down instead of the
gdbarch?
> if (objfile->separate_debug_objfile_backlink != NULL)
> {
> @@ -1486,7 +1493,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> {
> /* We first have to handle all the information about the
> probe which is present in the section. */
> - handle_stap_probe (objfile, iter, probesp, base);
> + handle_stap_probe (objfile, iter, probesp, base, gdbarch);
> }
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-06 13:50 ` Pedro Alves
@ 2013-12-06 15:49 ` Sergio Durigan Junior
2013-12-06 16:00 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-06 15:49 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches, Tom Tromey
On Friday, December 06 2013, Pedro Alves wrote:
> On 12/05/2013 10:12 PM, Sergio Durigan Junior wrote:
>> @@ -1461,6 +1466,8 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
>> bfd_vma base;
>> struct sdt_note *iter;
>> unsigned save_probesp_len = VEC_length (probe_p, *probesp);
>> + struct regcache *regcache = get_current_regcache ();
>> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
>
> AFAICS, stap_get_probes isn't called when evaluating a probe,
> but instead when getting the list of probes out of an objfile.
Right.
> Seems to me like this function is reachable even if the inferior
> is not running yet. Is that why you couldn't use
> get_selected_frame here? (because there's no frame?)
Right.
> If so, using get_current_regcache is wrong. If there's no thread at
> all, then what does the regcache of the current thread mean?
Indeed. Now that you put it that way, it makes total sense.
> It sounds like you just managed to use it becauese
> get_current_regcache doesn't error out when inferior_ptid is pointing
> nowhere.
OK, interesting.
> As this is listing the probes in the objfile, and not using the
>target's registers (afaics), can you use the objfile's arch here?
Yes, I guess that's the best solution indeed.
> With that out of the way, would it work to pass the frame pointer down
>instead of the gdbarch?
You mean that the callers should pass the frame pointers, instead of the
relying on the callees to get it by themselves?
--
Sergio
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-06 15:49 ` Sergio Durigan Junior
@ 2013-12-06 16:00 ` Pedro Alves
2013-12-06 16:04 ` Sergio Durigan Junior
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2013-12-06 16:00 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Tom Tromey
On 12/06/2013 03:49 PM, Sergio Durigan Junior wrote:
>> > With that out of the way, would it work to pass the frame pointer down
>> >instead of the gdbarch?
> You mean that the callers should pass the frame pointers, instead of the
> relying on the callees to get it by themselves?
Something like that. I meant, the patch added a gdbarch parameter to
a few functions, and then passes get_frame_arch (frame) down.
fetch_probe_arguments (struct value **arg0, struct value **arg1)
{
struct frame_info *frame = get_selected_frame (_("No frame selected"));
+ struct gdbarch *gdbarch = get_frame_arch (frame);
CORE_ADDR pc = get_frame_pc (frame);
struct probe *pc_probe;
const struct sym_probe_fns *pc_probe_fns;
...
+ *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0, gdbarch);
+ *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1, gdbarch);
etc. etc. I'm wondering whether after making that spot discussed
use the probe's obfile's arch, which I think already it has handy
through the probe pointer, we can pass down the frame pointer instead
of the gdbarch pointer:
fetch_probe_arguments (struct value **arg0, struct value **arg1)
{
struct frame_info *frame = get_selected_frame (_("No frame selected"));
CORE_ADDR pc = get_frame_pc (frame);
struct probe *pc_probe;
const struct sym_probe_fns *pc_probe_fns;
...
+ *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0, frame);
+ *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1, frame);
etc. That would make the intention of the code clearer, I think, as
that way we don't need to explain that much what the gdbarch is for,
and where it must come from.
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Sanitize gdbarch access on probe/SDT API
2013-12-06 16:00 ` Pedro Alves
@ 2013-12-06 16:04 ` Sergio Durigan Junior
0 siblings, 0 replies; 10+ messages in thread
From: Sergio Durigan Junior @ 2013-12-06 16:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches, Tom Tromey
On Friday, December 06 2013, Pedro Alves wrote:
> On 12/06/2013 03:49 PM, Sergio Durigan Junior wrote:
>>> > With that out of the way, would it work to pass the frame pointer down
>>> >instead of the gdbarch?
>> You mean that the callers should pass the frame pointers, instead of the
>> relying on the callees to get it by themselves?
>
> Something like that. I meant, the patch added a gdbarch parameter to
> a few functions, and then passes get_frame_arch (frame) down.
Sure, it can be done. I will wait for Tom to commit that cleanup patch
of his, and then I'll rebase my modifications on top of it and do this
tweak you're asking.
Thanks,
--
Sergio
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-12-06 16:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-05 6:34 [PATCH] Use "get_current_arch" instead of "get_objfile_arch" on SystemTap SDT code (and fix ARM bug) Sergio Durigan Junior
2013-12-05 13:04 ` Pedro Alves
2013-12-05 22:12 ` [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...) Sergio Durigan Junior
2013-12-05 23:06 ` [PATCH] Sanitize gdbarch access on probe/SDT API Sergio Durigan Junior
2013-12-05 23:30 ` Tom Tromey
2013-12-06 2:22 ` Sergio Durigan Junior
2013-12-06 13:50 ` Pedro Alves
2013-12-06 15:49 ` Sergio Durigan Junior
2013-12-06 16:00 ` Pedro Alves
2013-12-06 16:04 ` Sergio Durigan Junior
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).