From patchwork Thu Sep 25 20:47:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 2976 Received: (qmail 26777 invoked by alias); 25 Sep 2014 20:47:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26761 invoked by uid 89); 25 Sep 2014 20:47:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_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; Thu, 25 Sep 2014 20:47:30 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8PKlS2n009291 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 25 Sep 2014 16:47:29 -0400 Received: from localhost (dhcp-10-15-16-169.yyz.redhat.com [10.15.16.169]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8PKlSk7005468 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Thu, 25 Sep 2014 16:47:28 -0400 From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Gary Benson Subject: [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") References: <1411581801-19126-1-git-send-email-sergiodj@redhat.com> <5423F08B.1040409@redhat.com> X-URL: http://www.redhat.com Date: Thu, 25 Sep 2014 16:47:28 -0400 In-Reply-To: <5423F08B.1040409@redhat.com> (Pedro Alves's message of "Thu, 25 Sep 2014 11:38:03 +0100") Message-ID: <87bnq3h1gf.fsf_-_@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes 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: 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? 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);