[RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete"
Pedro Alves
palves@redhat.com
Thu Sep 25 10:38:00 GMT 2014
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
More information about the Gdb-patches
mailing list