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

Sergio Durigan Junior sergiodj@redhat.com
Thu Jul 11 17:57:00 GMT 2013


On Thursday, July 11 2013, Gary Benson wrote:

> Sergio Durigan Junior wrote:
>> 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 @@
>>  	  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 @@
>>  
>>  	      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_".  */
>
> Duplicated comment.

Ops, fixed.

>> @@ -2012,6 +2016,15 @@
>>  		  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;
>> +		}
>>  	    }
>
> This is checking one probe from each list (ie there will be 7 checks).
> Is this what you wanted to do?  I would have thought you would either
> want to check one probe or all probes.

Yes, I wanted to check only one probe.  I still prefer to do this check
inside the loop, because we avoid gathering all probes before being sure
that we can use them.  I've rewritten this part, let me see what you
think.

> Also I would comment "/* Ensure probe arguments can be evaluated.  */"
> but that is probably bikeshedding on my part :)

Sure, comment fixed.

Thanks for taking a look.

-- 
Sergio

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..c86bc6f 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1978,12 +1978,14 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
 	{
 	  VEC (probe_p) *probes[NUM_PROBES];
 	  int all_probes_found = 1;
+	  int checked_can_use_probe_arguments = 0;
 	  int i;
 
 	  memset (probes, 0, sizeof (probes));
 	  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
@@ -2012,6 +2014,18 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
 		  all_probes_found = 0;
 		  break;
 		}
+
+	      /* Ensure probe arguments can be evaluated.  */
+	      if (!checked_can_use_probe_arguments)
+		{
+		  p = VEC_index (probe_p, probes[i], 0);
+		  if (!can_evaluate_probe_arguments (p))
+		    {
+		      all_probes_found = 0;
+		      break;
+		    }
+		  checked_can_use_probe_arguments = 1;
+		}
 	    }
 
 	  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.



More information about the Gdb-patches mailing list