This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Implement way of checking if probe interface can evaluate arguments


Hi,

According to the discussion on:

          http://sourceware.org/ml/gdb-patches/2013-06/msg00745.html

(Please see the follow-ups for that message)

I am implementing a way for probe interface users to check if probes's
arguments can be evaluated.  Let me explain why I have chosen this path.

Currently, we only have SystemTap SDT probe support implement on GDB,
although we offer a generic probe.h interface that can be used to
implement other kinds of probes.  And for the SDT probes, there are two
things to consider:

1) How GDB parses the probe from the objfile;

2) How GDB parses the probe's arguments.

Item (1) is totally self-contained inside GDB, it does not need any kind
of external support nor internal, target-dependent support.  It only
depends on ELF, and the code is fully implemented on gdb/stap-probe.c.
So, even if we create a new architecture called XYZ today, GDB will be
fully able to extract the probes from the objfiles on this arch,
assuming that it uses ELF, of course.

Item (2) is the problematic thing which led to the creation of this
patch.  While one can surely create probes without arguments, in the
most majority of the cases the users (and ourselves, for that matter)
will want to use probes *with* arguments.  And to parse the arguments,
GDB depends on target-specific support (remember that we're dealing with
asm here).  So, what was happening with Gary's patch was:

- GDB found the probes it needed on the loader;
- It correctly inserted breakpoints on those probes;
- But when it tried to evaluate the arguments of those probes, an error
  was issued and the warning was printed.

We can do better.  So I've implemented this patch, which exports one
more function to the probe interface, responsible for telling whether
the ability to evaluate arguments (target-specific) is present.  If it
is not, then we shouldn't even bother to use the probes, because we will
eventually need to evaluate the arguments and get an error in the face.

I have also modified the three places that currently use this probe
feature: Gary's patch, along with longjmp and unwind probes.

Regtested on x86_64, Fedora 17, without regressions.  I don't have
access to an architecture which (a) can use probes and (b) does not have
an implementation for it inside GDB, so if anyone has and can test it, I
would really appreciate.

OK to apply?

-- 
Sergio

2013-07-10  Sergio Durigan Junior  <sergiodj@redhat.com>

	* breakpoint.c (create_longjmp_master_breakpoint): Check if probe
	interface can evaluate arguments.  Fallback to the old mode if it
	cannot.
	(create_exception_master_breakpoint): Likewise.
	* elfread.c (elf_can_evaluate_probe_arguments): New function.
	(struct sym_probe_fns elf_probe_fns): Export function above to the
	probe interface.
	* probe.c (can_evaluate_probe_arguments): New function.
	* probe.h (struct probe_ops) <can_evaluate_probe_arguments>: New
	function pointer.
	(can_evaluate_probe_arguments): New function prototype.
	* solib-svr4.c (svr4_create_solib_event_breakpoints): Check if
	probe interface can evaluate arguments.  Fallback to the old mode
	if it cannot.
	* stap-probe.c (stap_get_probe_argument_count): Check if probe
	interface can evaluate arguments.  Warning the user if it cannot.
	(stap_can_evaluate_probe_arguments): New function.
	(struct probe_ops stap_probe_ops): Export function above to the
	probe interface.
	* symfile.h (struct sym_probe_fns) <can_evaluate_probe_arguments>:
	New function pointer.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4d09b30..1e89407 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3194,8 +3194,23 @@ create_longjmp_master_breakpoint (void)
 
       if (!bp_objfile_data->longjmp_searched)
 	{
-	  bp_objfile_data->longjmp_probes
-	    = find_probes_in_objfile (objfile, "libc", "longjmp");
+	  VEC (probe_p) *ret;
+
+	  ret = find_probes_in_objfile (objfile, "libc", "longjmp");
+	  if (ret != NULL)
+	    {
+	      /* We are only interested in checking one element.  */
+	      struct probe *p = VEC_index (probe_p, ret, 0);
+
+	      if (!can_evaluate_probe_arguments (p))
+		{
+		  /* We cannot use the probe interface here, because it does
+		     not know how to evaluate arguments.  */
+		  VEC_free (probe_p, ret);
+		  ret = NULL;
+		}
+	    }
+	  bp_objfile_data->longjmp_probes = ret;
 	  bp_objfile_data->longjmp_searched = 1;
 	}
 
@@ -3336,8 +3351,24 @@ create_exception_master_breakpoint (void)
       /* We prefer the SystemTap probe point if it exists.  */
       if (!bp_objfile_data->exception_searched)
 	{
-	  bp_objfile_data->exception_probes
-	    = find_probes_in_objfile (objfile, "libgcc", "unwind");
+	  VEC (probe_p) *ret;
+
+	  ret = find_probes_in_objfile (objfile, "libgcc", "unwind");
+
+	  if (ret != NULL)
+	    {
+	      /* We are only interested in checking one element.  */
+	      struct probe *p = VEC_index (probe_p, ret, 0);
+
+	      if (!can_evaluate_probe_arguments (p))
+		{
+		  /* We cannot use the probe interface here, because it does
+		     not know how to evaluate arguments.  */
+		  VEC_free (probe_p, ret);
+		  ret = NULL;
+		}
+	    }
+	  bp_objfile_data->exception_probes = ret;
 	  bp_objfile_data->exception_searched = 1;
 	}
 
diff --git a/gdb/elfread.c b/gdb/elfread.c
index cfdfe45..1aa10d1 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1643,6 +1643,15 @@ elf_get_probe_argument_count (struct probe *probe)
   return probe->pops->get_probe_argument_count (probe);
 }
 
+/* Implementation of `sym_can_evaluate_probe_arguments', as documented in
+   symfile.h.  */
+
+static int
+elf_can_evaluate_probe_arguments (struct probe *probe)
+{
+  return probe->pops->can_evaluate_probe_arguments (probe);
+}
+
 /* Implementation of `sym_evaluate_probe_argument', as documented in
    symfile.h.  */
 
@@ -1700,11 +1709,12 @@ probe_key_free (struct objfile *objfile, void *d)
 
 static const struct sym_probe_fns elf_probe_fns =
 {
-  elf_get_probes,		/* sym_get_probes */
-  elf_get_probe_argument_count,	/* sym_get_probe_argument_count */
-  elf_evaluate_probe_argument,	/* sym_evaluate_probe_argument */
-  elf_compile_to_ax,		/* sym_compile_to_ax */
-  elf_symfile_relocate_probe,	/* sym_relocate_probe */
+  elf_get_probes,		    /* sym_get_probes */
+  elf_get_probe_argument_count,	    /* sym_get_probe_argument_count */
+  elf_can_evaluate_probe_arguments, /* sym_can_evaluate_probe_arguments */
+  elf_evaluate_probe_argument,	    /* sym_evaluate_probe_argument */
+  elf_compile_to_ax,		    /* sym_compile_to_ax */
+  elf_symfile_relocate_probe,	    /* sym_relocate_probe */
 };
 
 /* Register that we are able to handle ELF object file formats.  */
diff --git a/gdb/probe.c b/gdb/probe.c
index e650892..c313c38 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -628,6 +628,23 @@ get_probe_argument_count (struct probe *probe)
 
 /* See comments in probe.h.  */
 
+int
+can_evaluate_probe_arguments (struct probe *probe)
+{
+  const struct sym_probe_fns *probe_fns;
+
+  gdb_assert (probe->objfile != NULL);
+  gdb_assert (probe->objfile->sf != NULL);
+
+  probe_fns = probe->objfile->sf->sym_probe_fns;
+
+  gdb_assert (probe_fns != NULL);
+
+  return probe_fns->can_evaluate_probe_arguments (probe);
+}
+
+/* See comments in probe.h.  */
+
 struct value *
 evaluate_probe_argument (struct probe *probe, unsigned n)
 {
diff --git a/gdb/probe.h b/gdb/probe.h
index de07f50..dd5387b 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -73,6 +73,12 @@ struct probe_ops
 
     unsigned (*get_probe_argument_count) (struct probe *probe);
 
+    /* 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);
+
     /* Evaluate the Nth argument from the PROBE, returning a value
        corresponding to it.  The argument number is represented N.  */
 
@@ -218,6 +224,12 @@ extern struct cmd_list_element **info_probes_cmdlist_get (void);
 
 extern unsigned get_probe_argument_count (struct probe *probe);
 
+/* Return 1 if the probe interface associated with PROBE can evaluate
+   arguments, zero otherwise.  See the comments on the definition of
+   sym_probe_fns:can_evaluate_probe_arguments for more details.  */
+
+extern int can_evaluate_probe_arguments (struct probe *probe);
+
 /* Evaluate argument N of the specified probe.  N must be between 0
    inclusive and get_probe_argument_count exclusive.  */
 
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6f8d0ef..9aeebaf 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1984,6 +1984,7 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
 	  for (i = 0; i < NUM_PROBES; i++)
 	    {
 	      const char *name = probe_info[i].name;
+	      struct probe *p;
 	      char buf[32];
 
 	      /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4
@@ -2001,6 +2002,9 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
 
 	      probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
 
+	      /* Checking whether the probe interface can evaluate
+		 arguments.  */
+
 	      /* The "map_failed" probe did not exist in early
 		 versions of the probes code in which the probes'
 		 names were prefixed with "rtld_".  */
@@ -2012,6 +2016,15 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
 		  all_probes_found = 0;
 		  break;
 		}
+
+	      /* Checking to see if the probe interface can evaluate
+		 arguments.  If not, we cannot use it.  */
+	      p = VEC_index (probe_p, probes[i], 0);
+	      if (!can_evaluate_probe_arguments (p))
+		{
+		  all_probes_found = 0;
+		  break;
+		}
 	    }
 
 	  if (all_probes_found)
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index 1079e3b..55ed796 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1009,7 +1009,28 @@ stap_get_probe_argument_count (struct probe *probe_generic)
   gdb_assert (probe_generic->pops == &stap_probe_ops);
 
   if (!probe->args_parsed)
-    stap_parse_probe_arguments (probe);
+    {
+      if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic))
+	stap_parse_probe_arguments (probe);
+      else
+	{
+	  static int have_warned_stap_incomplete = 0;
+
+	  if (!have_warned_stap_incomplete)
+	    {
+	      warning (_(
+"The SystemTap SDT probe support is not fully implemented on this target;\n"
+"you will not be able to inspect probes's arguments.\n"
+"Please, report a bug against GDB requesting for the implementation to be\n"
+"ported to this target."));
+	      have_warned_stap_incomplete = 1;
+	    }
+
+	  /* Marking the arguments as "already parsed".  */
+	  probe->args_u.vec = NULL;
+	  probe->args_parsed = 1;
+	}
+    }
 
   gdb_assert (probe->args_parsed);
   return VEC_length (stap_probe_arg_s, probe->args_u.vec);
@@ -1060,6 +1081,20 @@ stap_get_arg (struct stap_probe *probe, unsigned n)
   return VEC_index (stap_probe_arg_s, probe->args_u.vec, n);
 }
 
+/* Implement the `can_evaluate_probe_arguments' method of probe_ops.  */
+
+static int
+stap_can_evaluate_probe_arguments (struct probe *probe_generic)
+{
+  struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
+  struct gdbarch *gdbarch = stap_probe->p.objfile->gdbarch;
+
+  /* For SystemTap probes, we have to guarantee that the method
+     stap_is_single_operand is defined on gdbarch.  If it is not, then it
+     means that argument evaluation is not implemented on this target.  */
+  return gdbarch_stap_is_single_operand_p (gdbarch);
+}
+
 /* Evaluate the probe's argument N (indexed from 0), returning a value
    corresponding to it.  Assertion is thrown if N does not exist.  */
 
@@ -1520,6 +1555,7 @@ static const struct probe_ops stap_probe_ops =
   stap_get_probes,
   stap_relocate,
   stap_get_probe_argument_count,
+  stap_can_evaluate_probe_arguments,
   stap_evaluate_probe_argument,
   stap_compile_to_ax,
   stap_set_semaphore,
diff --git a/gdb/symfile.h b/gdb/symfile.h
index c0e367d..c36e6b3 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -315,6 +315,14 @@ struct sym_probe_fns
      implement this method as well.  */
   unsigned (*sym_get_probe_argument_count) (struct probe *probe);
 
+  /* Return 1 if the probe interface can evaluate the arguments of probe
+     PROBE, zero otherwise.  This function can be probe-specific, informing
+     whether only the arguments of PROBE can be evaluated, of generic,
+     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);
+
   /* Evaluate the Nth argument available to PROBE.  PROBE will have
      come from a call to this objfile's sym_get_probes method.  N will
      be between 0 and the number of arguments available to this probe.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]