[PATCH] Remove support for "rtld_" prefix on solib-svr4 probes (was: Re: [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete")

[Sergio Durigan Junior <sergiodj@redhat.com>]

Thanks for the review.

On Thursday, September 25 2014, Pedro Alves wrote:

> Well, AFAICS, upstream GDB still supports F17's probes.  See
> svr4_create_solib_event_breakpoints:
>
> 	  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
> 		 shipped with an early version of the probes code in
> 		 which the probes' names were prefixed with "rtld_"
> 		 and the "map_failed" probe did not exist.  The
> 		 locations of the probes are otherwise the same, so
> 		 we check for probes with prefixed names if probes
> 		 with unprefixed names are not present.  */
> 	      if (with_prefix)
> 		{
> 		  xsnprintf (buf, sizeof (buf), "rtld_%s", name);
> 		  name = buf;
> 		}
>
> 	      probes[i]

Indeed it does, thanks for catching this.

> So it seems to me the test should cope with both variants.

Or maybe we should simplify this code and remove this support.

Really, Fedora 17 was EOL'ed more than 1 year ago:

  <https://lists.fedoraproject.org/pipermail/announce/2013-July/003177.html>

And we are already on Fedora 20, moving towards Fedora 21.  Also, this
code was needed because a patch present in Fedora 17's glibc, so I think
it is fair to leave this to be handled by Fedora GDB if needed (but it
won't be, because the upstream glibc patches already made into Fedora
too).

I am sending a patch to remove the support, tell me what you think.  I'm
in the "let's clean GDB code" mode, in order to avoid having to handle
with dead code all around...

> But, on a cursory look, I can't see what is being tested by this
> test that wouldn't work without probes?  If the test would pass just
> the same without probes, I think we should just remove the
> probe probing.   OTOH, if this is testing functionally that only
> works if probes are available, then I still think the test is
> lacking a comment.
>
> I also find it a bit odd to check for a probe point that GDB
> doesn't seem to be using:
>
> static const struct probe_info probe_info[] =
> {
>   { "init_start", DO_NOTHING },
>   { "init_complete", FULL_RELOAD },
>   { "map_start", DO_NOTHING },
>   { "map_failed", DO_NOTHING },
>   { "reloc_complete", UPDATE_OR_RELOAD },
>   { "unmap_start", DO_NOTHING },
>   { "unmap_complete", FULL_RELOAD },
> };

I will leave these comments to Gary, because he wrote the code.  But I
can look at it if needed, of course.

So, what do you think of this patch?

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

gdb/ChangeLog:
2014-09-25  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR gdb/17016
	* solib-svr4.c (svr4_create_solib_event_breakpoints): Remove
	support for "rtld_" prefix when looking for probes.  This prefix
	was used on a Fedora 17 specific patch, which is now EOL'ed.

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 3deef20..b5ea9bb 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1983,71 +1983,47 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch,
   os = find_pc_section (address);
   if (os != NULL)
     {
-      int with_prefix;
+      VEC (probe_p) *probes[NUM_PROBES];
+      int all_probes_found = 1;
+      int checked_can_use_probe_arguments = 0;
+      int i;
 
-      for (with_prefix = 0; with_prefix <= 1; with_prefix++)
+      memset (probes, 0, sizeof (probes));
+      for (i = 0; i < NUM_PROBES; i++)
 	{
-	  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
-		 shipped with an early version of the probes code in
-		 which the probes' names were prefixed with "rtld_"
-		 and the "map_failed" probe did not exist.  The
-		 locations of the probes are otherwise the same, so
-		 we check for probes with prefixed names if probes
-		 with unprefixed names are not present.  */
-	      if (with_prefix)
-		{
-		  xsnprintf (buf, sizeof (buf), "rtld_%s", name);
-		  name = buf;
-		}
+	  const char *name = probe_info[i].name;
+	  struct probe *p;
+	  char buf[32];
 
-	      probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
+	  probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
 
-	      /* The "map_failed" probe did not exist in early
-		 versions of the probes code in which the probes'
-		 names were prefixed with "rtld_".  */
-	      if (strcmp (name, "rtld_map_failed") == 0)
-		continue;
+	  if (VEC_empty (probe_p, probes[i]))
+	    {
+	      all_probes_found = 0;
+	      break;
+	    }
 
-	      if (VEC_empty (probe_p, probes[i]))
+	  /* 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;
 		}
-
-	      /* 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;
-		}
+	      checked_can_use_probe_arguments = 1;
 	    }
+	}
 
-	  if (all_probes_found)
-	    svr4_create_probe_breakpoints (gdbarch, probes, os->objfile);
+      if (all_probes_found)
+	svr4_create_probe_breakpoints (gdbarch, probes, os->objfile);
 
-	  for (i = 0; i < NUM_PROBES; i++)
-	    VEC_free (probe_p, probes[i]);
+      for (i = 0; i < NUM_PROBES; i++)
+	VEC_free (probe_p, probes[i]);
 
-	  if (all_probes_found)
-	    return;
-	}
+      if (all_probes_found)
+	return;
     }
 
   create_solib_event_breakpoint (gdbarch, address);