public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [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).