* [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" @ 2014-09-24 18:03 Sergio Durigan Junior 2014-09-25 9:41 ` Gary Benson 2014-09-25 10:38 ` Pedro Alves 0 siblings, 2 replies; 11+ messages in thread From: Sergio Durigan Junior @ 2014-09-24 18:03 UTC (permalink / raw) To: GDB Patches; +Cc: Pedro Alves, Gary Benson, Sergio Durigan Junior 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(-) 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 $" { -- 1.9.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" 2014-09-24 18:03 [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" Sergio Durigan Junior @ 2014-09-25 9:41 ` Gary Benson 2014-09-25 10:38 ` Pedro Alves 1 sibling, 0 replies; 11+ messages in thread From: Gary Benson @ 2014-09-25 9:41 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Pedro Alves 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 -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" 2014-09-24 18:03 [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" Sergio Durigan Junior 2014-09-25 9:41 ` Gary Benson @ 2014-09-25 10:38 ` Pedro Alves 2014-09-25 20:47 ` [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") Sergio Durigan Junior 1 sibling, 1 reply; 11+ messages in thread From: Pedro Alves @ 2014-09-25 10:38 UTC (permalink / raw) To: Sergio Durigan Junior, GDB Patches; +Cc: Gary Benson 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [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") 2014-09-25 10:38 ` Pedro Alves @ 2014-09-25 20:47 ` Sergio Durigan Junior 2014-09-25 21:13 ` [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes Pedro Alves 0 siblings, 1 reply; 11+ messages in thread From: Sergio Durigan Junior @ 2014-09-25 20:47 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Gary Benson 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); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 20:47 ` [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") Sergio Durigan Junior @ 2014-09-25 21:13 ` Pedro Alves 2014-09-25 21:23 ` Sergio Durigan Junior 0 siblings, 1 reply; 11+ messages in thread From: Pedro Alves @ 2014-09-25 21:13 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Gary Benson On 09/25/2014 09:47 PM, Sergio Durigan Junior wrote: > 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). There's RHEL (at least, per the comment) 6.4 too, which isn't EOL'ed, though. It's reasonable to expect that people may still want to build/test upstream gdb on those? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 21:13 ` [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes Pedro Alves @ 2014-09-25 21:23 ` Sergio Durigan Junior 2014-09-25 21:44 ` Pedro Alves 0 siblings, 1 reply; 11+ messages in thread From: Sergio Durigan Junior @ 2014-09-25 21:23 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Gary Benson On Thursday, September 25 2014, Pedro Alves wrote: > There's RHEL (at least, per the comment) 6.4 too, which isn't EOL'ed, > though. It's reasonable to expect that people may still want to > build/test upstream gdb on those? Damn, I forgot to talk about RHEL. But yeah, RHEL-6.x is not EOL'ed, but the GDB that comes with it is from Red Hat as well, and contains all the necessary patches to deal with whatever "internal idiosyncrasy" that it may need. OTOH, I know Gary has been using RHEL-6.5 to test his upstream patches, so to answer your question, it is still possible that people may still want to buid/test upstream GDB there. My opinion is that we shouldn't need to worry about internals of each distro (I know we *have to do that* sometimes, but that's not an excuse), so I still hold the cleanup patch for approval :-). -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 21:23 ` Sergio Durigan Junior @ 2014-09-25 21:44 ` Pedro Alves 2014-09-25 21:53 ` Sergio Durigan Junior 0 siblings, 1 reply; 11+ messages in thread From: Pedro Alves @ 2014-09-25 21:44 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Gary Benson On 09/25/2014 10:23 PM, Sergio Durigan Junior wrote: > On Thursday, September 25 2014, Pedro Alves wrote: > >> There's RHEL (at least, per the comment) 6.4 too, which isn't EOL'ed, >> though. It's reasonable to expect that people may still want to >> build/test upstream gdb on those? > > Damn, I forgot to talk about RHEL. > > But yeah, RHEL-6.x is not EOL'ed, but the GDB that comes with it is from > Red Hat as well, and contains all the necessary patches to deal with > whatever "internal idiosyncrasy" that it may need. OTOH, I know Gary > has been using RHEL-6.5 to test his upstream patches, so to answer your > question, it is still possible that people may still want to buid/test > upstream GDB there. > > My opinion is that we shouldn't need to worry about internals of each > distro (I know we *have to do that* sometimes, but that's not an > excuse), so I still hold the cleanup patch for approval :-). There's the system GDB, that is usually maintained by the distro, but then it's quite often the case that people build and ship their own tools on top of the distro, bypassing the system tools. I tend to view supporting older-ish distros that people might still be using like the proprietary OSs we "support" (in a sense). I think that just as we'd accept a patch that makes GDB work better on Windows 7 OOTB (e.g., to work around some debug API issue), even though there's already Windows 8 out there, I think patches that make GDB work better OOTB on a bit older (but still in use) distros are fine, as long as they don't get in the way of progress and don't impose a big maintenance burden. IMHO, there's no harm in leaving this particular bit in a while longer. But I certainly won't cry over this. I'm not personally affected. If others are fine with yanking this out, I'll be fine with it too. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 21:44 ` Pedro Alves @ 2014-09-25 21:53 ` Sergio Durigan Junior 2014-09-25 22:07 ` Pedro Alves 0 siblings, 1 reply; 11+ messages in thread From: Sergio Durigan Junior @ 2014-09-25 21:53 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Gary Benson On Thursday, September 25 2014, Pedro Alves wrote: > There's the system GDB, that is usually maintained by the > distro, but then it's quite often the case that people build > and ship their own tools on top of the distro, bypassing the > system tools. Yeah. > I tend to view supporting older-ish distros that people might > still be using like the proprietary OSs we "support" (in a sense). > I think that just as we'd accept a patch that makes GDB work better > on Windows 7 OOTB (e.g., to work around some debug API issue), even > though there's already Windows 8 out there, I think patches that make > GDB work better OOTB on a bit older (but still in use) distros are > fine, as long as they don't get in the way of progress and don't > impose a big maintenance burden. Heh, in my personal opinion GDB should not support proprietary OSes OOTB. But I certainly don't want to start a flamewar. As for support a bit older distro that might still be out there, I totally agree with you. The problem is that we (as a community) don't usually track those things very well, and code here tends to be forgot until someone stumbles on it because of some bug... > IMHO, there's no harm in leaving this particular bit in > a while longer. Me too, definitely, but there's the issue I raised in the sentence above... > But I certainly won't cry over this. I'm not personally affected. > If others are fine with yanking this out, I'll be fine with it too. Thanks. I will wait a few more days, and if nobody else objects, I will go ahead and push this patch in. BTW, if I push this in, I believe my other patch to adjust the testsuite becomes obvious, right? (Assuming that the test is indeed needed, as you already pointed out). Thanks, -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 21:53 ` Sergio Durigan Junior @ 2014-09-25 22:07 ` Pedro Alves 2014-09-25 22:21 ` Sergio Durigan Junior 0 siblings, 1 reply; 11+ messages in thread From: Pedro Alves @ 2014-09-25 22:07 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Gary Benson On 09/25/2014 10:53 PM, Sergio Durigan Junior wrote: >> I tend to view supporting older-ish distros that people might >> still be using like the proprietary OSs we "support" (in a sense). >> I think that just as we'd accept a patch that makes GDB work better >> on Windows 7 OOTB (e.g., to work around some debug API issue), even >> though there's already Windows 8 out there, I think patches that make >> GDB work better OOTB on a bit older (but still in use) distros are >> fine, as long as they don't get in the way of progress and don't >> impose a big maintenance burden. > > Heh, in my personal opinion GDB should not support proprietary OSes > OOTB. But I certainly don't want to start a flamewar. I don't either. But I'd rather a user stuck on such a OS be able to use a free debugger, than drive him towards a proprietary debugger. That's part of how I got involved into GDB in the first place. I was forced to used Windows at work. I worked around that by using Cygwin, to be able to use the free tools I preferred. At the same time I needed to build a tool that would run on Windows CE. So I worked on the GNU toolchain in order to target that OS. Then I wanted to make Cygwin GDB better too, because it was similar to CE, and I was using it at work too. And then somehow I ended up working on GDB full time. :-P It's a trap, I tells ya! The real point was that the user building GDB may have no control over the system bits of the distro it is building GDB for (in this case glibc's loader), just like when building for a proprietary OS, even though GNU/Linux distros are based (mostly) on free sources. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 22:07 ` Pedro Alves @ 2014-09-25 22:21 ` Sergio Durigan Junior 2014-09-26 8:23 ` Gary Benson 0 siblings, 1 reply; 11+ messages in thread From: Sergio Durigan Junior @ 2014-09-25 22:21 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Gary Benson On Thursday, September 25 2014, Pedro Alves wrote: > I don't either. But I'd rather a user stuck on such a OS be able to > use a free debugger, than drive him towards a proprietary debugger. > That's part of how I got involved into GDB in the first place. I was > forced to used Windows at work. I worked around that by using Cygwin, > to be able to use the free tools I preferred. At the same time > I needed to build a tool that would run on Windows CE. So I worked on > the GNU toolchain in order to target that OS. Then I wanted to make Cygwin > GDB better too, because it was similar to CE, and I was using it > at work too. And then somehow I ended up working on GDB full > time. :-P It's a trap, I tells ya! Haha, thanks for sharing your experience :-). > The real point was that the user building GDB may have no control > over the system bits of the distro it is building GDB for (in this > case glibc's loader), just like when building for a proprietary OS, > even though GNU/Linux distros are based (mostly) on free sources. Yeah, that is a fair point, and it is very valid when we talk about things that might make GDB break badly when removed. But in this case, we are talking about a very specific Fedora/RHEL thing, which is itself intended to improve something (i.e., GDB will still work without it on old Fedora/RHEL systems), so I think most of the concerns don't apply here. -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes 2014-09-25 22:21 ` Sergio Durigan Junior @ 2014-09-26 8:23 ` Gary Benson 0 siblings, 0 replies; 11+ messages in thread From: Gary Benson @ 2014-09-26 8:23 UTC (permalink / raw) To: GDB Patches Sergio Durigan Junior wrote: > On Thursday, September 25 2014, Pedro Alves wrote: > > The real point was that the user building GDB may have no control > > over the system bits of the distro it is building GDB for (in this > > case glibc's loader), just like when building for a proprietary > > OS, even though GNU/Linux distros are based (mostly) on free > > sources. > > Yeah, that is a fair point, and it is very valid when we talk about > things that might make GDB break badly when removed. But in this > case, we are talking about a very specific Fedora/RHEL thing, which > is itself intended to improve something (i.e., GDB will still work > without it on old Fedora/RHEL systems), so I think most of the > concerns don't apply here. The code is dead, but only recently so. It's still warm :) Removing the prefixed-probes code would reintroduce this: https://sourceware.org/bugzilla/show_bug.cgi?id=2328 My preference is to leave the prefixes in, at least for now; you don't know what people are using out there. The comment lists affected OS versions so future reviewers will be able to decide how relevant it is. I don't think we need to bother with prefix support in the testsuite, but we should probably switch to a probe that GDB is using like Pedro mentioned earlier. "reloc_complete" would be good. Cheers, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-26 8:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-24 18:03 [RFA] Fix PR gdb/17016: Expect for probe "map_complete" instead of "rtld_map_complete" Sergio Durigan Junior 2014-09-25 9:41 ` Gary Benson 2014-09-25 10:38 ` Pedro Alves 2014-09-25 20:47 ` [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") Sergio Durigan Junior 2014-09-25 21:13 ` [PATCH] Remove support for "rtld_" prefix on solib-svr4 probes Pedro Alves 2014-09-25 21:23 ` Sergio Durigan Junior 2014-09-25 21:44 ` Pedro Alves 2014-09-25 21:53 ` Sergio Durigan Junior 2014-09-25 22:07 ` Pedro Alves 2014-09-25 22:21 ` Sergio Durigan Junior 2014-09-26 8:23 ` Gary Benson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).