This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[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")
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Gary Benson <gbenson at redhat dot com>
- Date: Thu, 25 Sep 2014 16:47:28 -0400
- 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")
- Authentication-results: sourceware.org; auth=none
- References: <1411581801-19126-1-git-send-email-sergiodj at redhat dot com> <5423F08B dot 1040409 at redhat dot 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);