[RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete"

  This commit fixes PR gdb/17016.  It could be considered to be obvious,
but I am sending to the mailing list for approval either way...

The problem, described by Pedro, is that a specific test on
gdb.threads/dlopen-libpthread.exp is XFAILing.  The test is:

  XFAIL: gdb.threads/dlopen-libpthread.exp: info probes all rtld rtld_map_complete

According to Pedro, he was not seeing this failure on Fedora 17, but
is now seeing it on Fedora 20.  This has been caused by a difference
between a Fedora-local glibc patch and an upstream glibc patch,
submitted by Gary in 2012.

Back then, Gary submitted this patch to glibc upstream:

  <https://sourceware.org/ml/libc-alpha/2012-06/msg00690.html>

Which implemented the SDT probes on the runtime linker.  Note that the
initial patch included the "rtld_" prefix on the probes' names.
Roland McGrath asked him to remove this prefix, and this is what was
pushed to the repo:

  <https://sourceware.org/ml/libc-alpha/2012-06/msg00723.html>

  commit 815e6fa3e010628f77838abec18692cbfeb60809
  Author: Gary Benson <gbenson@redhat.com>
  Date:   Thu Jul 26 11:03:35 2012 +0100

      Add SystemTap static probes to the runtime linker.  [BZ #14298]

Gary and Jan also added the gdb.threads/dlopen-libpthread.exp file later on GDB:

  commit a29a3fb7a350b70ec755b1964d2830094314dba8
  Author: Gary Benson <gary@redhat.com>
  Date:   Tue Jun 4 13:23:32 2013 +0000

      2013-06-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
	      Gary Benson  <gbenson@redhat.com>

	  * lib/gdb.exp (build_executable_from_specs): Use gdb_compile_pthread,
	  gdb_compile_shlib or gdb_compile_shlib_pthreads where appropriate.
	  * lib/prelink-support.exp (build_executable_own_libs): Allow INTERP
	  to be set to "no" to indicate that no ld.so copy should be made.
	  * gdb.base/break-interp.exp (solib_bp): New constant.
	  (reach_1): Use the above instead of "_dl_debug_state".
	  (test_attach): Likewise.
	  (test_ld): Likewise.
	  * gdb.threads/dlopen-libpthread.exp: New file.
	  * gdb.threads/dlopen-libpthread.c: Likewise.
	  * gdb.threads/dlopen-libpthread-lib.c: Likewise.
	  * gdb.base/solib-corrupted.exp: Disable test if GDB is using probes.

And this file is also using the "rtld_" prefix when trying to match
the glibc probe (probably because they were using a Fedora glibc with
the first patch applied, see below).

However, on the Fedora land, we included Gary's first patch on the
glibc package for Fedora 17:

  <http://pkgs.fedoraproject.org/cgit/glibc.git/tree/glibc-rh179072.patch?h=f17>

And now, 2 years later, this local patch is obviously not needed
anymore, because upstream glibc already has the necessary patch in
it.  But we forgot to update our local testcase, so that is what this
patch does.

OK to apply?

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

	PR gdb/17016
	* gdb.threads/dlopen-libpthread.exp: Adjust match to probe
	"map_complete" instead of "rtld_map_complete.
---
 gdb/testsuite/gdb.threads/dlopen-libpthread.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Sergio Durigan Junior wrote:
> This commit fixes PR gdb/17016.  It could be considered to be obvious,
> but I am sending to the mailing list for approval either way...
[snip]
> gdb/testsuite/ChangeLog:
> 2014-09-24  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	PR gdb/17016
> 	* gdb.threads/dlopen-libpthread.exp: Adjust match to probe
> 	"map_complete" instead of "rtld_map_complete.
[snip]
> OK to apply?

Definitely :)

Thanks,
Gary
  

On 09/24/2014 07:03 PM, Sergio Durigan Junior wrote:
> This commit fixes PR gdb/17016.  It could be considered to be obvious,
> but I am sending to the mailing list for approval either way...
> 
> The problem, described by Pedro, is that a specific test on
> gdb.threads/dlopen-libpthread.exp is XFAILing.  The test is:
> 
>   XFAIL: gdb.threads/dlopen-libpthread.exp: info probes all rtld rtld_map_complete
> 
> According to Pedro, he was not seeing this failure on Fedora 17, but
> is now seeing it on Fedora 20.  This has been caused by a difference
> between a Fedora-local glibc patch and an upstream glibc patch,
> submitted by Gary in 2012.
> 
> Back then, Gary submitted this patch to glibc upstream:
> 
>   <https://sourceware.org/ml/libc-alpha/2012-06/msg00690.html>
> 
> Which implemented the SDT probes on the runtime linker.  Note that the
> initial patch included the "rtld_" prefix on the probes' names.
> Roland McGrath asked him to remove this prefix, and this is what was
> pushed to the repo:
> 
>   <https://sourceware.org/ml/libc-alpha/2012-06/msg00723.html>
> 
>   commit 815e6fa3e010628f77838abec18692cbfeb60809
>   Author: Gary Benson <gbenson@redhat.com>
>   Date:   Thu Jul 26 11:03:35 2012 +0100
> 
>       Add SystemTap static probes to the runtime linker.  [BZ #14298]
> 
> Gary and Jan also added the gdb.threads/dlopen-libpthread.exp file later on GDB:
> 
>   commit a29a3fb7a350b70ec755b1964d2830094314dba8
>   Author: Gary Benson <gary@redhat.com>
>   Date:   Tue Jun 4 13:23:32 2013 +0000
> 
>       2013-06-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 	      Gary Benson  <gbenson@redhat.com>
> 
> 	  * lib/gdb.exp (build_executable_from_specs): Use gdb_compile_pthread,
> 	  gdb_compile_shlib or gdb_compile_shlib_pthreads where appropriate.
> 	  * lib/prelink-support.exp (build_executable_own_libs): Allow INTERP
> 	  to be set to "no" to indicate that no ld.so copy should be made.
> 	  * gdb.base/break-interp.exp (solib_bp): New constant.
> 	  (reach_1): Use the above instead of "_dl_debug_state".
> 	  (test_attach): Likewise.
> 	  (test_ld): Likewise.
> 	  * gdb.threads/dlopen-libpthread.exp: New file.
> 	  * gdb.threads/dlopen-libpthread.c: Likewise.
> 	  * gdb.threads/dlopen-libpthread-lib.c: Likewise.
> 	  * gdb.base/solib-corrupted.exp: Disable test if GDB is using probes.
> 
> And this file is also using the "rtld_" prefix when trying to match
> the glibc probe (probably because they were using a Fedora glibc with
> the first patch applied, see below).
> 
> However, on the Fedora land, we included Gary's first patch on the
> glibc package for Fedora 17:
> 
>   <http://pkgs.fedoraproject.org/cgit/glibc.git/tree/glibc-rh179072.patch?h=f17>
> 
> And now, 2 years later, this local patch is obviously not needed
> anymore, because upstream glibc already has the necessary patch in
> it.  But we forgot to update our local testcase, so that is what this
> patch does.
> 
> OK to apply?

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]


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

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 },
};

Thanks,
Pedro Alves