Message ID | 1411581801-19126-1-git-send-email-sergiodj@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 9936 invoked by alias); 24 Sep 2014 18:03:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 9793 invoked by uid 89); 24 Sep 2014 18:03:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 24 Sep 2014 18:03:31 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OI3Twa027656 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for <gdb-patches@sourceware.org>; Wed, 24 Sep 2014 14:03:29 -0400 Received: from psique.yyz.redhat.com (dhcp-10-15-16-169.yyz.redhat.com [10.15.16.169]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8OI3SsN016261; Wed, 24 Sep 2014 14:03:28 -0400 From: Sergio Durigan Junior <sergiodj@redhat.com> To: GDB Patches <gdb-patches@sourceware.org> Cc: Pedro Alves <palves@redhat.com>, Gary Benson <gbenson@redhat.com>, Sergio Durigan Junior <sergiodj@redhat.com> Subject: [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" Date: Wed, 24 Sep 2014 14:03:21 -0400 Message-Id: <1411581801-19126-1-git-send-email-sergiodj@redhat.com> X-IsSubscribed: yes |
Commit Message
Sergio Durigan Junior
Sept. 24, 2014, 6:03 p.m. UTC
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(-)
Comments
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
diff --git a/gdb/testsuite/gdb.threads/dlopen-libpthread.exp b/gdb/testsuite/gdb.threads/dlopen-libpthread.exp index f0e3358..eaff8ca 100644 --- a/gdb/testsuite/gdb.threads/dlopen-libpthread.exp +++ b/gdb/testsuite/gdb.threads/dlopen-libpthread.exp @@ -41,9 +41,9 @@ if { ![runto_main] } { return -1 } -set test "info probes all rtld rtld_map_complete" +set test "info probes all rtld map_complete" gdb_test_multiple $test $test { - -re "\[ \t\]rtld_map_complete\[ \t\]+0x\[0-9a-f\]+.*\r\n$gdb_prompt $" { + -re "\[ \t\]map_complete\[ \t\]+0x\[0-9a-f\]+.*\r\n$gdb_prompt $" { pass $test } -re "No probes matched\\.\r\n$gdb_prompt $" {