time to workaround libc/13097 in fsf gdb?

time to workaround libc/13097 in fsf gdb?

[Doug Evans]

Hi.

It's been three years and various people are doing similar things to
deal with glibc's that are out there (regardless of whatever glibc
decides to do).

http://sourceware.org/ml/gdb-patches/2011-08/msg00331.html

As for the patch itself, I wonder if the "fix" belongs in solib-svr4.c
instead of solib.c.  E.g., where we compare the so name with "",
also check for linux-vdso.so.1 and linux-gate.so.1 ?

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/11/2014 05:25 PM, Doug Evans wrote:
> Hi.
> 
> It's been three years and various people are doing similar things to
> deal with glibc's that are out there (regardless of whatever glibc
> decides to do).
> 
> http://sourceware.org/ml/gdb-patches/2011-08/msg00331.html
> 
> As for the patch itself, I wonder if the "fix" belongs in solib-svr4.c
> instead of solib.c.  E.g., where we compare the so name with "",
> also check for linux-vdso.so.1 and linux-gate.so.1 ?

Also, we know the address of the vDSO/gate (symfile-mem.c).  Can't
we use that to match instead of the name?

I think that's what Ulrich meant in
  https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c1
with
 "You must already have magic code in gdb to handle this DSO".

ISTR having seen a patch that does that, but I can't seem to find it.

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Thu, 11 Sep 2014 18:37:02 +0200, Pedro Alves wrote:
> Also, we know the address of the vDSO/gate (symfile-mem.c).  Can't
> we use that to match instead of the name?
[...]
> ISTR having seen a patch that does that, but I can't seem to find it.

Re: [patch+7.5.1] Work around PR libc/13097 "linux-vdso.so.1" #3
https://sourceware.org/ml/gdb-patches/2012-11/msg00636.html
Message-ID: <20121125181505.GA26194@host2.jankratochvil.net>

It is pending/unreviewed/unapproved.

Also I am not sure it is really still an issue on latest upstream glibc, it is
not an issue on Fedora 21 x86_64 glibc with the reproducer from:
	https://sourceware.org/bugzilla/show_bug.cgi?id=13097

(That is old Fedoras workarounded it in glibc, then some Fedoras exposed the
issue in GDB but now it is not visible - so either Fedoras workaround it again
or just upstream glibc switched/workarounds it also.)


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/12/2014 12:54 PM, Jan Kratochvil wrote:
> On Thu, 11 Sep 2014 18:37:02 +0200, Pedro Alves wrote:
>> Also, we know the address of the vDSO/gate (symfile-mem.c).  Can't
>> we use that to match instead of the name?
> [...]
>> ISTR having seen a patch that does that, but I can't seem to find it.
> 
> Re: [patch+7.5.1] Work around PR libc/13097 "linux-vdso.so.1" #3
> https://sourceware.org/ml/gdb-patches/2012-11/msg00636.html
> Message-ID: <20121125181505.GA26194@host2.jankratochvil.net>
> 
> It is pending/unreviewed/unapproved.

Ah, yeah, I think that was it.

I was more inclined to leave the vdso in the shared library list
though, like ldd does, than filtering it out.  Like, similar to
your gdbarch_solib_file_not_found_is_ok patch, but look at the
addresses rather than filenames in the hook.  I'm not sure
whether that'd complicate things too much.

> Also I am not sure it is really still an issue on latest upstream glibc, it is
> not an issue on Fedora 21 x86_64 glibc with the reproducer from:
> 	https://sourceware.org/bugzilla/show_bug.cgi?id=13097
> 
> (That is old Fedoras workarounded it in glibc, then some Fedoras exposed the
> issue in GDB but now it is not visible - so either Fedoras workaround it again
> or just upstream glibc switched/workarounds it also.)

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
> I was more inclined to leave the vdso in the shared library list
> though, like ldd does, than filtering it out.  Like, similar to
> your gdbarch_solib_file_not_found_is_ok patch, but look at the
> addresses rather than filenames in the hook.  I'm not sure
> whether that'd complicate things too much.

Everything can be done but this is again changing a direction/behavior of GDB
upon receiving a fix of current behavior.  So far GDB has not been including
vDSO in the library list and the patch was fixing that behavior.  One can go
very far from doing one fix up to rewriting GDB from scratch.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/12/2014 01:33 PM, Jan Kratochvil wrote:
> On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
>> I was more inclined to leave the vdso in the shared library list
>> though, like ldd does, than filtering it out.  Like, similar to
>> your gdbarch_solib_file_not_found_is_ok patch, but look at the
>> addresses rather than filenames in the hook.  I'm not sure
>> whether that'd complicate things too much.
> 
> Everything can be done but this is again changing a direction/behavior of GDB
> upon receiving a fix of current behavior.  So far GDB has not been including
> vDSO in the library list and the patch was fixing that behavior.  One can go
> very far from doing one fix up to rewriting GDB from scratch.

I think that's a bit uncalled for and unfair -- AIUI, your original patch
even did that; it left vdso.so in the list.

I had said:

 "Alternatively to hard coding the names, maybe we could match the vdso address
  found through that with the addresses found iterating the dynamic linker list, to
  know which dynamic linker entry is the vdso."

And your new patch said:

 "But now it discards any shared libraries which match a symbol file loaded via
  add-symbol-file-from-memory.  Which may be OK but it is more widespread change
  than before."

I was only clarifying what I had already said in the message
you replied to.  I have no idea what problems you found in
the original patch that led to redesigning the patch to filter
out instead, or what you saw that would suggest that doing that
change would require tilting so much in the "rewriting GDB from
scratch" direction.

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Doug Evans]

Pedro Alves writes:
 > On 09/12/2014 12:54 PM, Jan Kratochvil wrote:
 > > On Thu, 11 Sep 2014 18:37:02 +0200, Pedro Alves wrote:
 > >> Also, we know the address of the vDSO/gate (symfile-mem.c).  Can't
 > >> we use that to match instead of the name?
 > > [...]
 > >> ISTR having seen a patch that does that, but I can't seem to find it.
 > > 
 > > Re: [patch+7.5.1] Work around PR libc/13097 "linux-vdso.so.1" #3
 > > https://sourceware.org/ml/gdb-patches/2012-11/msg00636.html
 > > Message-ID: <20121125181505.GA26194@host2.jankratochvil.net>
 > > 
 > > It is pending/unreviewed/unapproved.
 > 
 > Ah, yeah, I think that was it.
 > 
 > I was more inclined to leave the vdso in the shared library list
 > though, like ldd does, than filtering it out.

I like this too.
No point in hiding it from the user.

 > Like, similar to
 > your gdbarch_solib_file_not_found_is_ok patch, but look at the
 > addresses rather than filenames in the hook.  I'm not sure
 > whether that'd complicate things too much.

I'm not sure either.

It *would* be nice to connect the processing of vdso when seen
by svr4_read_so_list (called during shared library loading)
with add_syscall_page (called as an observer when an inferior
is created).  The same file (vdso) is processed in two disparate places
with no linkage for the human reader between them - a bit confusing.

 > > Also I am not sure it is really still an issue on latest upstream glibc, it is
 > > not an issue on Fedora 21 x86_64 glibc with the reproducer from:
 > > 	https://sourceware.org/bugzilla/show_bug.cgi?id=13097
 > > 
 > > (That is old Fedoras workarounded it in glibc, then some Fedoras exposed the
 > > issue in GDB but now it is not visible - so either Fedoras workaround it again
 > > or just upstream glibc switched/workarounds it also.)

I downloaded glibc 2.20 and tested it with gdb 7.8.
Still an issue.
[I just did a simple build, if there's a configure option
that will fix things I didn't try one.]
I think FSF gdb 7.8[.1?] (the latest) should work with FSF glibc 2.20
(the latest).

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Fri, 12 Sep 2014 14:46:23 +0200, Pedro Alves wrote:
> On 09/12/2014 01:33 PM, Jan Kratochvil wrote:
> > On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
> >> I was more inclined to leave the vdso in the shared library list
> >> though, like ldd does, than filtering it out.  Like, similar to
> >> your gdbarch_solib_file_not_found_is_ok patch, but look at the
> >> addresses rather than filenames in the hook.  I'm not sure
> >> whether that'd complicate things too much.
> > 
> > Everything can be done but this is again changing a direction/behavior of GDB
> > upon receiving a fix of current behavior.  So far GDB has not been including
> > vDSO in the library list and the patch was fixing that behavior.  One can go
> > very far from doing one fix up to rewriting GDB from scratch.
> 
> I think that's a bit uncalled for and unfair -- AIUI, your original patch
> even did that; it left vdso.so in the list.
> 
> I had said:
> 
>  "Alternatively to hard coding the names, maybe we could match the vdso address
>   found through that with the addresses found iterating the dynamic linker list, to
>   know which dynamic linker entry is the vdso."
> 
> And your new patch said:
> 
>  "But now it discards any shared libraries which match a symbol file loaded via
>   add-symbol-file-from-memory.  Which may be OK but it is more widespread change
>   than before."
> 
> I was only clarifying what I had already said in the message
> you replied to.

Do you mean that "Alternatively to hard coding the names, maybe we could" was
only an idea which I should have ignored?  I had the title global maintainer
that time so I probably should have checked in a patch after a timeout but
I always found the timeout rule (if there is any such rule) broken relying on
a luck, compared to Gerrit's flagging of patches by multiple maintainers.


> I have no idea what problems you found in the original patch that led to
> redesigning the patch to filter out instead,

As there was the discussion whether the list of names of former patch
	Message-ID: <20121122201737.GA32172@host2.jankratochvil.net>
is complete and whether one should not use the address match instead.

The new patch
	Message-ID: <20121125181505.GA26194@host2.jankratochvil.net>
filters-out the vDSO from library list as is done by FSF GDB HEAD.

Each of the patches is considered to have some disadvantages, IIUC.
But all of them are just theoretical in the usual GDB nitpicking style.


> or what you saw that would suggest that doing that change would require
> tilting so much in the "rewriting GDB from scratch" direction.

For each fix of GDB one has to consider how widespread the fix should be.

Any fix present on gdb-patches is wrong as it is not written in an
effective/maintainable language (C++/Java/C#/others).  Therefore a full fix
would be always to rewrite GDB into C++/Java/C#/others first - that I call
"rewriting GDB from scratch".  But that I consider as a too large task to do
for any of the fixes, particularly because is then more effective to rather
start extending LLDB instead.

So a metric of a best fix does not work.  Another possible metric is to make
a fix in minimal time/effort/cost so that the user is no longer affected by
the specific bug being addressed.  This metric I try to follow.

You seem to evaluate the patches by some other metric which I cannot guess
myself in advance to coding a patch.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Sat, 13 Sep 2014 03:06:12 +0200, Doug Evans wrote:
> > On 09/12/2014 12:54 PM, Jan Kratochvil wrote:
> > > Also I am not sure it is really still an issue on latest upstream glibc, it is
> > > not an issue on Fedora 21 x86_64 glibc with the reproducer from:
> > > 	https://sourceware.org/bugzilla/show_bug.cgi?id=13097
> > > 
> > > (That is old Fedoras workarounded it in glibc, then some Fedoras exposed the
> > > issue in GDB but now it is not visible - so either Fedoras workaround it again
> > > or just upstream glibc switched/workarounds it also.)
> 
> I downloaded glibc 2.20 and tested it with gdb 7.8.
> Still an issue.
> [I just did a simple build, if there's a configure option
> that will fix things I didn't try one.]

I have then explained this unreproducibility for me by:
	[bfd patch] Regression for Linux vDSO in GDB
	https://sourceware.org/ml/binutils/2014-09/msg00140.html


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/17/2014 09:10 PM, Jan Kratochvil wrote:
> On Fri, 12 Sep 2014 14:46:23 +0200, Pedro Alves wrote:
>> On 09/12/2014 01:33 PM, Jan Kratochvil wrote:
>>> On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
>>>> I was more inclined to leave the vdso in the shared library list
>>>> though, like ldd does, than filtering it out.  Like, similar to
>>>> your gdbarch_solib_file_not_found_is_ok patch, but look at the
>>>> addresses rather than filenames in the hook.  I'm not sure
>>>> whether that'd complicate things too much.
>>>
>>> Everything can be done but this is again changing a direction/behavior of GDB
>>> upon receiving a fix of current behavior.  So far GDB has not been including
>>> vDSO in the library list and the patch was fixing that behavior.  One can go
>>> very far from doing one fix up to rewriting GDB from scratch.
>>
>> I think that's a bit uncalled for and unfair -- AIUI, your original patch
>> even did that; it left vdso.so in the list.
>>
>> I had said:
>>
>>  "Alternatively to hard coding the names, maybe we could match the vdso address
>>   found through that with the addresses found iterating the dynamic linker list, to
>>   know which dynamic linker entry is the vdso."
>>
>> And your new patch said:
>>
>>  "But now it discards any shared libraries which match a symbol file loaded via
>>   add-symbol-file-from-memory.  Which may be OK but it is more widespread change
>>   than before."
>>
>> I was only clarifying what I had already said in the message
>> you replied to.
> 
> Do you mean that "Alternatively to hard coding the names, maybe we could" was
> only an idea which I should have ignored?

No, it was an idea that should have been further discussed.  I failed
to follow up back then, sorry about that.

>> I have no idea what problems you found in the original patch that led to
>> redesigning the patch to filter out instead,
> 
> As there was the discussion whether the list of names of former patch
> 	Message-ID: <20121122201737.GA32172@host2.jankratochvil.net>
> is complete and whether one should not use the address match instead.
> 
> The new patch
> 	Message-ID: <20121125181505.GA26194@host2.jankratochvil.net>
> filters-out the vDSO from library list as is done by FSF GDB HEAD.
> 
> Each of the patches is considered to have some disadvantages, IIUC.

OK, I've investigated this further.  I think that we're a couple steps
away from being able to list the vdso without causing other issues.
I'll reply again with a patch based on your "new" patch, but that matches
the vdso by address independently of add-symbol-file-from-memory.

> But all of them are just theoretical in the usual GDB nitpicking style.

That is just not helpful.  :-/

>> or what you saw that would suggest that doing that change would require
>> tilting so much in the "rewriting GDB from scratch" direction.
> 
> For each fix of GDB one has to consider how widespread the fix should be.
> 
> Any fix present on gdb-patches is wrong as it is not written in an
> effective/maintainable language (C++/Java/C#/others).  Therefore a full fix
> would be always to rewrite GDB into C++/Java/C#/others first - that I call
> "rewriting GDB from scratch".  But that I consider as a too large task to do
> for any of the fixes, particularly because is then more effective to rather
> start extending LLDB instead.

Perhaps not surprisingly, I disagree.

> Another possible metric is to make
> a fix in minimal time/effort/cost so that the user is no longer affected by
> the specific bug being addressed.  This metric I try to follow.

I'd call that the "put on life support" metric...  I don't agree with it
(for GDB), and I'd guess most on this list don't.

> You seem to evaluate the patches by some other metric which I cannot guess
> myself in advance to coding a patch.

It's simply the metric of someone who believes that GDB is here
to stay, and therefore weighs impact of changes both in the present
and in the future.  Yes, GDB does have its issues, then again which
project doesn't?

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/19/2014 03:38 PM, Pedro Alves wrote:
> OK, I've investigated this further.  I think that we're a couple steps
> away from being able to list the vdso without causing other issues.
> I'll reply again with a patch based on your "new" patch, but that matches
> the vdso by address independently of add-symbol-file-from-memory.

Here it is.  WDYT?

-------------
[PATCH] Subject: Work around PR libc/13097 "linux-vdso.so.1"

With upstream glibc, GDB prints:

	warning: Could not load shared library symbols for linux-vdso.so.1.
	Do you need "set solib-search-path" or "set sysroot"?

A bug's been filed for glibc a few years back:

  http://sourceware.org/bugzilla/show_bug.cgi?id=13097

but it's still not resolved.  It's not clear whether there's even
consensus that this is indeed a glibc bug.  It would actually be nice
if GDB also listed the vdso in the shared library list, but there are
some design considerations with that:

 - the vDSO is mapped by the kernel, not userspace, therefore we
   should load its symbols right from the process's start of life,
   even before glibc / the userspace loader sets up the initial DSO
   list.  The program might even be using a custom loader or no
   loader.

 - that kind of hints at that solib.c should handle retrieving shared
   library lists from more than one source, and that symfile-mem.c's
   loading of the vdso would be converted to load and relocate the
   vdso's bfd behind the target_so_ops interface.

 - and then, once glibc links in the vdso to its DSO list, we'd need
   to either:

    a) somehow hand over the vdso from one target_so_ops to the
      other
    b) or simply keep hiding glibc's entry.

And then b) seems the simplest.

With that in mind, this patch simply discards the vDSO from glibc's
reported shared library list.

We can match the vdso address found through AT_SYSINFO_EHDR with the
addresses found iterating the dynamic linker list, to know which
dynamic linker entry is the vdso.

Note that symfile-mem.c is not present in every configuration that
uses solib-svr4.c.  It actually probably should always be linked in
GDB (added to COMMON_OBS), so that the "add-symbol-file-from-memory"
command is always available for all targets.  But even if we did that,
even though harmless, it's unnecessary or a bit wrong in principle
even, to try the target_auxv_search lookup against all targets (even
though harmless; at least currently).

So solve that, this moves the target_auxv_search lookup to a gdbarch
hook, registered for all Linux architectures, and then both
solib-svr4.c and symfile-mem.c can both use it.

Tested on x86_64 Fedora 20.

gdb/
2014-09-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	PR 14466
	* arch-utils.c (default_vsyscall_address): New function.
	* arch-utils.h (default_vsyscall_address): New declaration.
	* gdbarch.sh (vsyscall_address): New hook.
	* gdbarch.h, gdbarch.c: Regenerate.
	* linux-tdep.c (linux_vsyscall_address): New function.
	(linux_init_abi): Install linux_vsyscall_address as
	vsyscall_address gdbarch hook.
	* solib-svr4.c (svr4_read_so_list): Rename to ...
	(svr4_current_sos_1): ... this and change the function comment.
	(svr4_current_sos): New function.
	* symfile-mem.c (add_vsyscall_page): Use gdbarch_vsyscall_address.

gdb/testsuite/
2014-09-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	PR 14466
	* gdb.base/vdso-warning.c: New file.
	* gdb.base/vdso-warning.exp: New file.
---
 gdb/arch-utils.c                        |  8 +++++
 gdb/arch-utils.h                        |  4 +++
 gdb/gdbarch.c                           | 23 ++++++++++++++
 gdb/gdbarch.h                           |  4 +++
 gdb/gdbarch.sh                          |  3 ++
 gdb/linux-tdep.c                        | 13 ++++++++
 gdb/solib-svr4.c                        | 41 +++++++++++++++++++++++--
 gdb/symfile-mem.c                       |  5 ++-
 gdb/testsuite/gdb.base/vdso-warning.c   | 22 ++++++++++++++
 gdb/testsuite/gdb.base/vdso-warning.exp | 54 +++++++++++++++++++++++++++++++++
 10 files changed, 172 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vdso-warning.c
 create mode 100644 gdb/testsuite/gdb.base/vdso-warning.exp

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 5ae9fb3..b7ccb32 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -243,6 +243,14 @@ default_remote_register_number (struct gdbarch *gdbarch,
   return regno;
 }

+/* See arch-utils.h.  */
+
+CORE_ADDR
+default_vsyscall_address (struct gdbarch *gdbarch)
+{
+  return 0;
+}
+
 \f
 /* Functions to manipulate the endianness of the target.  */

diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 46d1573..fc0ce53 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -174,4 +174,8 @@ extern int default_return_in_first_hidden_param_p (struct gdbarch *,
 extern int default_insn_is_call (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_ret (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_jump (struct gdbarch *, CORE_ADDR);
+
+/* Do-nothing version of vsyscall_address.  Returns 0.  */
+
+extern CORE_ADDR default_vsyscall_address (struct gdbarch *);
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index b0ee79d..68b7a3e 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -316,6 +316,7 @@ struct gdbarch
   gdbarch_insn_is_ret_ftype *insn_is_ret;
   gdbarch_insn_is_jump_ftype *insn_is_jump;
   gdbarch_auxv_parse_ftype *auxv_parse;
+  gdbarch_vsyscall_address_ftype *vsyscall_address;
 };

 /* Create a new ``struct gdbarch'' based on information provided by
@@ -408,6 +409,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->insn_is_call = default_insn_is_call;
   gdbarch->insn_is_ret = default_insn_is_ret;
   gdbarch->insn_is_jump = default_insn_is_jump;
+  gdbarch->vsyscall_address = default_vsyscall_address;
   /* gdbarch_alloc() */

   return gdbarch;
@@ -627,6 +629,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of insn_is_ret, invalid_p == 0 */
   /* Skip verify of insn_is_jump, invalid_p == 0 */
   /* Skip verify of auxv_parse, has predicate.  */
+  /* Skip verify of vsyscall_address, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1296,6 +1299,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: virtual_frame_pointer = <%s>\n",
                       host_address_to_string (gdbarch->virtual_frame_pointer));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: vsyscall_address = <%s>\n",
+                      host_address_to_string (gdbarch->vsyscall_address));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: vtable_function_descriptors = %s\n",
                       plongest (gdbarch->vtable_function_descriptors));
   fprintf_unfiltered (file,
@@ -4403,6 +4409,23 @@ set_gdbarch_auxv_parse (struct gdbarch *gdbarch,
   gdbarch->auxv_parse = auxv_parse;
 }

+CORE_ADDR
+gdbarch_vsyscall_address (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->vsyscall_address != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_vsyscall_address called\n");
+  return gdbarch->vsyscall_address (gdbarch);
+}
+
+void
+set_gdbarch_vsyscall_address (struct gdbarch *gdbarch,
+                              gdbarch_vsyscall_address_ftype vsyscall_address)
+{
+  gdbarch->vsyscall_address = vsyscall_address;
+}
+

 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0303b2e..8f54405 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1317,6 +1317,10 @@ typedef int (gdbarch_auxv_parse_ftype) (struct gdbarch *gdbarch, gdb_byte **read
 extern int gdbarch_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp);
 extern void set_gdbarch_auxv_parse (struct gdbarch *gdbarch, gdbarch_auxv_parse_ftype *auxv_parse);

+typedef CORE_ADDR (gdbarch_vsyscall_address_ftype) (struct gdbarch *gdbarch);
+extern CORE_ADDR gdbarch_vsyscall_address (struct gdbarch *gdbarch);
+extern void set_gdbarch_vsyscall_address (struct gdbarch *gdbarch, gdbarch_vsyscall_address_ftype *vsyscall_address);
+
 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)

diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 2a8bca8..b6f0550 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1029,6 +1029,9 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0
 # Return -1 if there is insufficient buffer for a whole entry.
 # Return 1 if an entry was read into *TYPEP and *VALP.
 M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp
+
+# Return the address of the current inferior's vsyscall/vDSO.
+m:CORE_ADDR:vsyscall_address:void:::default_vsyscall_address::0
 EOF
 }

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index dae59c5..00f0ed5 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1782,6 +1782,18 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }

+/* Implementation of the "vsyscall_address" gdbarch hook.  */
+
+static CORE_ADDR
+linux_vsyscall_address (struct gdbarch *gdbarch)
+{
+  CORE_ADDR sysinfo_ehdr;
+
+  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0)
+    return sysinfo_ehdr;
+  return 0;
+}
+
 /* To be called from the various GDB_OSABI_LINUX handlers for the
    various GNU/Linux architectures and machine types.  */

@@ -1799,6 +1811,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 				      linux_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch,
 				    linux_gdb_signal_to_target);
+  set_gdbarch_vsyscall_address (gdbarch, linux_vsyscall_address);
 }

 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 3deef20..af9a4ca 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1462,10 +1462,11 @@ svr4_current_sos_direct (struct svr4_info *info)
   return head;
 }

-/* Implement the "current_sos" target_so_ops method.  */
+/* Implement the main part of the "current_sos" target_so_ops
+   method.  */

 static struct so_list *
-svr4_current_sos (void)
+svr4_current_sos_1 (void)
 {
   struct svr4_info *info = get_svr4_info ();

@@ -1478,6 +1479,42 @@ svr4_current_sos (void)
   return svr4_current_sos_direct (info);
 }

+/* Implement the "current_sos" target_so_ops method.  */
+
+static struct so_list *
+svr4_current_sos (void)
+{
+  struct so_list *so_head = svr4_current_sos_1 ();
+  struct objfile *objfile;
+  struct obj_section *osect;
+  CORE_ADDR vsyscall_addr = gdbarch_vsyscall_address (target_gdbarch ());
+
+  /* Filter out the vDSO module, if present.  Its symbol file would
+     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
+     managed by symfile-mem.c:add_vsyscall_page.  */
+  if (vsyscall_addr != 0)
+    {
+      struct so_list **sop;
+
+      sop = &so_head;
+      while (*sop != NULL)
+	{
+	  struct so_list *so = *sop;
+
+	  if (so->lm_info->l_addr_inferior == vsyscall_addr)
+	    {
+	      *sop = so->next;
+	      free_so (so);
+	      break;
+	    }
+
+	  sop = &so->next;
+	}
+    }
+
+  return so_head;
+}
+
 /* Get the address of the link_map for a given OBJFILE.  */

 CORE_ADDR
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index ef48f7d..073dec3 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -211,10 +211,9 @@ find_vdso_size (CORE_ADDR vaddr, unsigned long size,
 static void
 add_vsyscall_page (struct target_ops *target, int from_tty)
 {
-  CORE_ADDR sysinfo_ehdr;
+  CORE_ADDR sysinfo_ehdr = gdbarch_vsyscall_address (target_gdbarch ());

-  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
-      && sysinfo_ehdr != (CORE_ADDR) 0)
+  if (sysinfo_ehdr != 0)
     {
       struct bfd *bfd;
       struct symbol_file_add_from_memory_args args;
diff --git a/gdb/testsuite/gdb.base/vdso-warning.c b/gdb/testsuite/gdb.base/vdso-warning.c
new file mode 100644
index 0000000..4b94803
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vdso-warning.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/vdso-warning.exp b/gdb/testsuite/gdb.base/vdso-warning.exp
new file mode 100644
index 0000000..1f538fa
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vdso-warning.exp
@@ -0,0 +1,54 @@
+# Copyright 2012-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile] } {
+    return -1
+}
+
+gdb_breakpoint "main"
+
+# At least some versions of Fedora/RHEL glibc have local patches that
+# hide the vDSO.  This lines re-exposes it.  See PR libc/13097,
+# comment 2.  There's no support for passing environment variables in
+# the remote protocol, but that's OK -- if we're testing against a
+# glibc that doesn't list the vDSO without this, the test should still
+# pass.
+gdb_test_no_output "set environment LD_DEBUG=unused"
+
+gdb_run_cmd
+
+set test "stop without warning"
+gdb_test_multiple "" $test {
+    -re "Could not load shared library symbols .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\nBreakpoint \[0-9\]+, main .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+# Extra testing in case the warning changes and we miss updating the
+# above.
+set test "no vdso without symbols is listed"
+gdb_test_multiple "info shared" $test {
+    -re "No\[^\r\n\]+linux-(vdso|gate).*$gdb_prompt $" {
+	fail $test
+    }
+    -re "$gdb_prompt $" {
+	pass $test
+    }
+}
-- 
1.9.3

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Fri, 19 Sep 2014 16:38:07 +0200, Pedro Alves wrote:
> On 09/17/2014 09:10 PM, Jan Kratochvil wrote:
> > You seem to evaluate the patches by some other metric which I cannot guess
> > myself in advance to coding a patch.
> 
> It's simply the metric of someone who believes that GDB is here
> to stay, and therefore weighs impact of changes both in the present
> and in the future.

Then it is (IMO) most time effective to rewrite GDB to C++ first.  But it has
some organizational issues as the improved stability, speed and maintenance
cost may (or may not?) be lower priority than specific fixes/improvements
requested by users.  Which leads to short time vs. long time goals.
I also can't forget to mention there is also LLDB.


On Fri, 19 Sep 2014 16:38:07 +0200, Pedro Alves wrote:
> Perhaps not surprisingly, I disagree.

We therefore both agree on our disagreement.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Fri, 19 Sep 2014 16:41:34 +0200, Pedro Alves wrote:
> Here it is.  WDYT?
[...]
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
[...]
> @@ -1478,6 +1479,42 @@ svr4_current_sos (void)
>    return svr4_current_sos_direct (info);
>  }
> 
> +/* Implement the "current_sos" target_so_ops method.  */
> +
> +static struct so_list *
> +svr4_current_sos (void)
> +{
> +  struct so_list *so_head = svr4_current_sos_1 ();
> +  struct objfile *objfile;
> +  struct obj_section *osect;
> +  CORE_ADDR vsyscall_addr = gdbarch_vsyscall_address (target_gdbarch ());
> +
> +  /* Filter out the vDSO module, if present.  Its symbol file would
> +     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
> +     managed by symfile-mem.c:add_vsyscall_page.  */
> +  if (vsyscall_addr != 0)
> +    {
> +      struct so_list **sop;
> +
> +      sop = &so_head;
> +      while (*sop != NULL)
> +	{
> +	  struct so_list *so = *sop;
> +
> +	  if (so->lm_info->l_addr_inferior == vsyscall_addr)

This won't work as l_addr_inferior (and also l_addr) do not necessarily
represent the real starting address of the ELF if the ELF itself is
"prelinked".  For some reason vDSOs on some kernels look like prelinked.

kernel-3.16.2-200.fc20.x86_64 appears sane, vDSO is 0-based.

But for example kernel-2.6.32-220.el6.x86_64 is "prelinked", see below.


That's the pain of solib-svr4.c which is OS-agnostic and so it cannot find out
start of the ELF file just from link map.  gdbserver can find it as it can
depend on /proc/PID/{s,}maps as its linux-low.c is Linux-specific.
That was implemented in unfinished/pending:
	[PATCH v5 0/8] Validate binary before use
	Message-ID: <20140319223004.14668.20989.stgit@host1.jankratochvil.net>


Thanks,
Jan


kernel-2.6.32-220.el6.x86_64
(gdb) p *_r_debug.r_map.l_next
$4 = {l_addr = 140737363566592, l_name = 0x3deba1ade4 "", l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
(gdb) p/x *_r_debug.r_map.l_next
$5 = {l_addr = 0x7ffff88fe000, l_name = 0x3deba1ade4, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
# (gdb) p/x *new.lm_info
# $5 = {l_addr = 0x0, l_addr_inferior = 0x7ffff88fe000, l_addr_p = 0x0, lm_addr = 0x3debc21718, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188, l_name = 0x3deba1ade4}
(gdb) info auxv
33   AT_SYSINFO_EHDR      System-supplied DSO's ELF header 0x7ffff7ffe000
(gdb) info proc mappings
      0x7ffff7ffe000     0x7ffff7fff000     0x1000          0                           [vdso]
(gdb) dump memory vdso.bin 0x7ffff7ffe000 0x7ffff7fff000
# readelf -Wa vdso.bin
[...]
  Entry point address:               0xffffffffff700700
[...]
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .hash             HASH            ffffffffff700120 000120 000038 04   A  2   0  8
  [ 2] .dynsym           DYNSYM          ffffffffff700158 000158 0000d8 18   A  3   2  8
[...]
  [ 9] .dynamic          DYNAMIC         ffffffffff700580 000580 0000f0 10  WA  3   0  8

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/20/2014 10:30 PM, Jan Kratochvil wrote:
> On Fri, 19 Sep 2014 16:41:34 +0200, Pedro Alves wrote:
>> Here it is.  WDYT?
> [...]
>> --- a/gdb/solib-svr4.c
>> +++ b/gdb/solib-svr4.c
> [...]
>> @@ -1478,6 +1479,42 @@ svr4_current_sos (void)
>>    return svr4_current_sos_direct (info);
>>  }
>>
>> +/* Implement the "current_sos" target_so_ops method.  */
>> +
>> +static struct so_list *
>> +svr4_current_sos (void)
>> +{
>> +  struct so_list *so_head = svr4_current_sos_1 ();
>> +  struct objfile *objfile;
>> +  struct obj_section *osect;
>> +  CORE_ADDR vsyscall_addr = gdbarch_vsyscall_address (target_gdbarch ());
>> +
>> +  /* Filter out the vDSO module, if present.  Its symbol file would
>> +     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
>> +     managed by symfile-mem.c:add_vsyscall_page.  */
>> +  if (vsyscall_addr != 0)
>> +    {
>> +      struct so_list **sop;
>> +
>> +      sop = &so_head;
>> +      while (*sop != NULL)
>> +	{
>> +	  struct so_list *so = *sop;
>> +
>> +	  if (so->lm_info->l_addr_inferior == vsyscall_addr)
> 
> This won't work as l_addr_inferior (and also l_addr) do not necessarily
> represent the real starting address of the ELF if the ELF itself is
> "prelinked".  For some reason vDSOs on some kernels look like prelinked.
> 
> kernel-3.16.2-200.fc20.x86_64 appears sane, vDSO is 0-based.
> 
> But for example kernel-2.6.32-220.el6.x86_64 is "prelinked", see below.

Ah, didn't know that.  That's the sort of thing we should have in
comments in the code, or at least in the commit log.

> That's the pain of solib-svr4.c which is OS-agnostic and so it cannot find out
> start of the ELF file just from link map.  gdbserver can find it as it can
> depend on /proc/PID/{s,}maps as its linux-low.c is Linux-specific.

Is it really a pain though?  We can just put things behind gdbarch hooks,
like my patch was doing.  In fact, symfile-mem.c is already looking
at /proc/PID/maps to find the vdso mapping size.  That's exactly done
behind a gdbarch hook:

      args.size = 0;
      if (gdbarch_find_memory_regions_p (target_gdbarch ()))
	(void) gdbarch_find_memory_regions (target_gdbarch (),
					    find_vdso_size, &args);

> 
> kernel-2.6.32-220.el6.x86_64
> (gdb) p *_r_debug.r_map.l_next
> $4 = {l_addr = 140737363566592, l_name = 0x3deba1ade4 "", l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
> (gdb) p/x *_r_debug.r_map.l_next
> $5 = {l_addr = 0x7ffff88fe000, l_name = 0x3deba1ade4, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
> # (gdb) p/x *new.lm_info
> # $5 = {l_addr = 0x0, l_addr_inferior = 0x7ffff88fe000, l_addr_p = 0x0, lm_addr = 0x3debc21718, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188, l_name = 0x3deba1ade4}
> (gdb) info auxv
> 33   AT_SYSINFO_EHDR      System-supplied DSO's ELF header 0x7ffff7ffe000
> (gdb) info proc mappings
>       0x7ffff7ffe000     0x7ffff7fff000     0x1000          0                           [vdso]

Sounds like a predicate like this would work then?

	  if (vsyscall_start <= so->lm_info->l_ld && so->lm_info->l_ld < vsyscall_end)

We would move the bit that finds the vdso size to the gdbarch_vsyscall_address hook,
and make that new hook return the vsyscall's size too in addition to
the starting address.  We can also cache the result somewhere instead
of constantly reopening /proc/PID/maps if necessary.

We can also add a custom linux-specific target_so_ops implementation that
extends svr4's if we want.  The target_so_ops to use is also registered
in the gdbarch.

  CORE_ADDR vsyscall_start;
  CORE_ADDR vsyscall_end;

  vsyscall_start = gdbarch_vsyscall_address (target_gdbarch (), &vsyscall_end);

  /* Filter out the vDSO module, if present.  Its symbol file would
     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
     managed by symfile-mem.c:add_vsyscall_page.  */
  if (vsyscall_start != 0)
    {
       struct so_list **sop;

       sop = &so_head;
       while (*sop != NULL)
         {
           struct so_list *so = *sop;

    	   if (vsyscall_start <= so->lm_info->l_ld
               && so->lm_info->l_ld < vsyscall_end)
             {
               ... found vdso ...

> (gdb) dump memory vdso.bin 0x7ffff7ffe000 0x7ffff7fff000
> # readelf -Wa vdso.bin
> [...]
>   Entry point address:               0xffffffffff700700
> [...]
> Section Headers:
>   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>   [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
>   [ 1] .hash             HASH            ffffffffff700120 000120 000038 04   A  2   0  8
>   [ 2] .dynsym           DYNSYM          ffffffffff700158 000158 0000d8 18   A  3   2  8
> [...]
>   [ 9] .dynamic          DYNAMIC         ffffffffff700580 000580 0000f0 10  WA  3   0  8

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/21/2014 08:12 PM, Pedro Alves wrote:
> On 09/20/2014 10:30 PM, Jan Kratochvil wrote:
>> On Fri, 19 Sep 2014 16:41:34 +0200, Pedro Alves wrote:
>>> Here it is.  WDYT?
>> [...]
>>> --- a/gdb/solib-svr4.c
>>> +++ b/gdb/solib-svr4.c
>> [...]
>>> @@ -1478,6 +1479,42 @@ svr4_current_sos (void)
>>>    return svr4_current_sos_direct (info);
>>>  }
>>>
>>> +/* Implement the "current_sos" target_so_ops method.  */
>>> +
>>> +static struct so_list *
>>> +svr4_current_sos (void)
>>> +{
>>> +  struct so_list *so_head = svr4_current_sos_1 ();
>>> +  struct objfile *objfile;
>>> +  struct obj_section *osect;
>>> +  CORE_ADDR vsyscall_addr = gdbarch_vsyscall_address (target_gdbarch ());
>>> +
>>> +  /* Filter out the vDSO module, if present.  Its symbol file would
>>> +     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
>>> +     managed by symfile-mem.c:add_vsyscall_page.  */
>>> +  if (vsyscall_addr != 0)
>>> +    {
>>> +      struct so_list **sop;
>>> +
>>> +      sop = &so_head;
>>> +      while (*sop != NULL)
>>> +	{
>>> +	  struct so_list *so = *sop;
>>> +
>>> +	  if (so->lm_info->l_addr_inferior == vsyscall_addr)
>>
>> This won't work as l_addr_inferior (and also l_addr) do not necessarily
>> represent the real starting address of the ELF if the ELF itself is
>> "prelinked".  For some reason vDSOs on some kernels look like prelinked.
>>
>> kernel-3.16.2-200.fc20.x86_64 appears sane, vDSO is 0-based.
>>
>> But for example kernel-2.6.32-220.el6.x86_64 is "prelinked", see below.
> 
> Ah, didn't know that.  That's the sort of thing we should have in
> comments in the code, or at least in the commit log.
> 
>> That's the pain of solib-svr4.c which is OS-agnostic and so it cannot find out
>> start of the ELF file just from link map.  gdbserver can find it as it can
>> depend on /proc/PID/{s,}maps as its linux-low.c is Linux-specific.
> 
> Is it really a pain though?  We can just put things behind gdbarch hooks,
> like my patch was doing.  In fact, symfile-mem.c is already looking
> at /proc/PID/maps to find the vdso mapping size.  That's exactly done
> behind a gdbarch hook:
> 
>       args.size = 0;
>       if (gdbarch_find_memory_regions_p (target_gdbarch ()))
> 	(void) gdbarch_find_memory_regions (target_gdbarch (),
> 					    find_vdso_size, &args);
> 
>>
>> kernel-2.6.32-220.el6.x86_64
>> (gdb) p *_r_debug.r_map.l_next
>> $4 = {l_addr = 140737363566592, l_name = 0x3deba1ade4 "", l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
>> (gdb) p/x *_r_debug.r_map.l_next
>> $5 = {l_addr = 0x7ffff88fe000, l_name = 0x3deba1ade4, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188}
>> # (gdb) p/x *new.lm_info
>> # $5 = {l_addr = 0x0, l_addr_inferior = 0x7ffff88fe000, l_addr_p = 0x0, lm_addr = 0x3debc21718, l_ld = 0x7ffff7ffe580, l_next = 0x7ffff7ffd658, l_prev = 0x3debc21188, l_name = 0x3deba1ade4}
>> (gdb) info auxv
>> 33   AT_SYSINFO_EHDR      System-supplied DSO's ELF header 0x7ffff7ffe000
>> (gdb) info proc mappings
>>       0x7ffff7ffe000     0x7ffff7fff000     0x1000          0                           [vdso]
> 
> Sounds like a predicate like this would work then?
> 
> 	  if (vsyscall_start <= so->lm_info->l_ld && so->lm_info->l_ld < vsyscall_end)
> 
> We would move the bit that finds the vdso size to the gdbarch_vsyscall_address hook,
> and make that new hook return the vsyscall's size too in addition to
> the starting address.  We can also cache the result somewhere instead
> of constantly reopening /proc/PID/maps if necessary.
> 
> We can also add a custom linux-specific target_so_ops implementation that
> extends svr4's if we want.  The target_so_ops to use is also registered
> in the gdbarch.
> 
>   CORE_ADDR vsyscall_start;
>   CORE_ADDR vsyscall_end;
> 
>   vsyscall_start = gdbarch_vsyscall_address (target_gdbarch (), &vsyscall_end);
> 
>   /* Filter out the vDSO module, if present.  Its symbol file would
>      not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
>      managed by symfile-mem.c:add_vsyscall_page.  */
>   if (vsyscall_start != 0)
>     {
>        struct so_list **sop;
> 
>        sop = &so_head;
>        while (*sop != NULL)
>          {
>            struct so_list *so = *sop;
> 
>     	   if (vsyscall_start <= so->lm_info->l_ld
>                && so->lm_info->l_ld < vsyscall_end)
>              {
>                ... found vdso ...


Here's a quick update to the patch that does (just) that, for
testing.  Doesn't update comments, etc.  It works on f20 here.

WDYT ?

-------
From f64fc9d4bf3909570c6047ba0c6e78ce8ea8d8c6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 19 Sep 2014 15:08:42 +0100
Subject: [PATCH] Subject: Work around PR libc/13097 "linux-vdso.so.1"

With upstream glibc, GDB prints:

	warning: Could not load shared library symbols for linux-vdso.so.1.
	Do you need "set solib-search-path" or "set sysroot"?

A bug's been filed for glibc a few years back:

  http://sourceware.org/bugzilla/show_bug.cgi?id=13097

but it's still not resolved.  It's not clear whether there's even
consensus that this is indeed a glibc bug.  It would actually be nice
if GDB also listed the vdso in the shared library list, but there are
some design considerations with that:

 - the vDSO is mapped by the kernel, not userspace, therefore we
   should load its symbols right from the process's start of life,
   even before glibc / the userspace loader sets up the initial DSO
   list.  The program might even be using a custom loader or no
   loader.

 - that kind of hints at that solib.c should handle retrieving shared
   library lists from more than one source, and that symfile-mem.c's
   loading of the vdso would be converted to load and relocate the
   vdso's bfd behind the target_so_ops interface.

 - and then, once glibc links in the vdso to its DSO list, we'd need
   to either:

    a) somehow hand over the vdso from one target_so_ops to the
      other
    b) or simply keep hiding glibc's entry.

And then b) seems the simplest.

With that in mind, this patch simply discards the vDSO from glibc's
reported shared library list.

We can match the vdso address found through AT_SYSINFO_EHDR with the
addresses found iterating the dynamic linker list, to know which
dynamic linker entry is the vdso.

Note that symfile-mem.c is not present in every configuration that
uses solib-svr4.c.  It actually probably should always be linked in
GDB (added to COMMON_OBS), so that the "add-symbol-file-from-memory"
command is always available for all targets.  But even if we did that,
even though harmless, it's unnecessary or a bit wrong in principle
even, to try the target_auxv_search lookup against all targets (even
though harmless; at least currently).

So solve that, this moves the target_auxv_search lookup to a gdbarch
hook, registered for all Linux architectures, and then both
solib-svr4.c and symfile-mem.c can both use it.

Tested on x86_64 Fedora 20.

gdb/
2014-09-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	PR 14466
	* arch-utils.c (default_vsyscall_address): New function.
	* arch-utils.h (default_vsyscall_address): New declaration.
	* gdbarch.sh (vsyscall_address): New hook.
	* gdbarch.h, gdbarch.c: Regenerate.
	* linux-tdep.c (linux_vsyscall_address): New function.
	(linux_init_abi): Install linux_vsyscall_address as
	vsyscall_address gdbarch hook.
	* solib-svr4.c (svr4_read_so_list): Rename to ...
	(svr4_current_sos_1): ... this and change the function comment.
	(svr4_current_sos): New function.
	* symfile-mem.c (add_vsyscall_page): Use gdbarch_vsyscall_address.

gdb/testsuite/
2014-09-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	PR 14466
	* gdb.base/vdso-warning.c: New file.
	* gdb.base/vdso-warning.exp: New file.
---
 gdb/arch-utils.c                        |  8 +++++
 gdb/arch-utils.h                        |  4 +++
 gdb/gdbarch.c                           | 23 ++++++++++++++
 gdb/gdbarch.h                           |  7 +++++
 gdb/gdbarch.sh                          |  4 +++
 gdb/linux-tdep.c                        | 49 ++++++++++++++++++++++++++++++
 gdb/solib-svr4.c                        | 43 ++++++++++++++++++++++++--
 gdb/symfile-mem.c                       | 32 ++++---------------
 gdb/testsuite/gdb.base/vdso-warning.c   | 22 ++++++++++++++
 gdb/testsuite/gdb.base/vdso-warning.exp | 54 +++++++++++++++++++++++++++++++++
 10 files changed, 218 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vdso-warning.c
 create mode 100644 gdb/testsuite/gdb.base/vdso-warning.exp

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 5ae9fb3..ecaa773 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -243,6 +243,14 @@ default_remote_register_number (struct gdbarch *gdbarch,
   return regno;
 }

+/* See arch-utils.h.  */
+
+int
+default_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
+{
+  return 0;
+}
+
 \f
 /* Functions to manipulate the endianness of the target.  */

diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 46d1573..d9f49b9 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -174,4 +174,8 @@ extern int default_return_in_first_hidden_param_p (struct gdbarch *,
 extern int default_insn_is_call (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_ret (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_jump (struct gdbarch *, CORE_ADDR);
+
+/* Do-nothing version of vsyscall_address.  Returns 0.  */
+
+extern int default_vsyscall_range (struct gdbarch *, CORE_ADDR *, CORE_ADDR *);
 #endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index b0ee79d..54b2434 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -316,6 +316,7 @@ struct gdbarch
   gdbarch_insn_is_ret_ftype *insn_is_ret;
   gdbarch_insn_is_jump_ftype *insn_is_jump;
   gdbarch_auxv_parse_ftype *auxv_parse;
+  gdbarch_vsyscall_range_ftype *vsyscall_range;
 };

 /* Create a new ``struct gdbarch'' based on information provided by
@@ -408,6 +409,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->insn_is_call = default_insn_is_call;
   gdbarch->insn_is_ret = default_insn_is_ret;
   gdbarch->insn_is_jump = default_insn_is_jump;
+  gdbarch->vsyscall_range = default_vsyscall_range;
   /* gdbarch_alloc() */

   return gdbarch;
@@ -627,6 +629,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of insn_is_ret, invalid_p == 0 */
   /* Skip verify of insn_is_jump, invalid_p == 0 */
   /* Skip verify of auxv_parse, has predicate.  */
+  /* Skip verify of vsyscall_range, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &length);
   make_cleanup (xfree, buf);
   if (length > 0)
@@ -1296,6 +1299,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: virtual_frame_pointer = <%s>\n",
                       host_address_to_string (gdbarch->virtual_frame_pointer));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: vsyscall_range = <%s>\n",
+                      host_address_to_string (gdbarch->vsyscall_range));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: vtable_function_descriptors = %s\n",
                       plongest (gdbarch->vtable_function_descriptors));
   fprintf_unfiltered (file,
@@ -4403,6 +4409,23 @@ set_gdbarch_auxv_parse (struct gdbarch *gdbarch,
   gdbarch->auxv_parse = auxv_parse;
 }

+int
+gdbarch_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->vsyscall_range != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_vsyscall_range called\n");
+  return gdbarch->vsyscall_range (gdbarch, start, end);
+}
+
+void
+set_gdbarch_vsyscall_range (struct gdbarch *gdbarch,
+                            gdbarch_vsyscall_range_ftype vsyscall_range)
+{
+  gdbarch->vsyscall_range = vsyscall_range;
+}
+

 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0303b2e..5613ce4 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1317,6 +1317,13 @@ typedef int (gdbarch_auxv_parse_ftype) (struct gdbarch *gdbarch, gdb_byte **read
 extern int gdbarch_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp);
 extern void set_gdbarch_auxv_parse (struct gdbarch *gdbarch, gdbarch_auxv_parse_ftype *auxv_parse);

+/* Find the address range of the current inferior's vsyscall/vDSO, and
+   write it to *START, *END.  Returns true if found, false otherwise. */
+
+typedef int (gdbarch_vsyscall_range_ftype) (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end);
+extern int gdbarch_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end);
+extern void set_gdbarch_vsyscall_range (struct gdbarch *gdbarch, gdbarch_vsyscall_range_ftype *vsyscall_range);
+
 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)

diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 2a8bca8..e3bbf82 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1029,6 +1029,10 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0
 # Return -1 if there is insufficient buffer for a whole entry.
 # Return 1 if an entry was read into *TYPEP and *VALP.
 M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp
+
+# Find the address range of the current inferior's vsyscall/vDSO, and
+# write it to *START, *END.  Returns true if found, false otherwise.
+m:int:vsyscall_range:CORE_ADDR *start, CORE_ADDR *end:start, end::default_vsyscall_range::0
 EOF
 }

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index dae59c5..3f28fc9 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1782,6 +1782,54 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }

+/* Arguments for symbol_file_add_from_memory_wrapper.  */
+
+struct find_mapping_size_args
+{
+  CORE_ADDR vaddr;
+  size_t size;
+};
+
+/* Rummage through mappings to find a mapping size.  */
+
+static int
+find_mapping_size (CORE_ADDR vaddr, unsigned long size,
+		   int read, int write, int exec, int modified,
+		   void *data)
+{
+  struct find_mapping_size_args *args = data;
+
+  if (vaddr == args->vaddr)
+    {
+      args->size = size;
+      return 1;
+    }
+  return 0;
+}
+
+/* Implementation of the "vsyscall_range" gdbarch hook.  */
+
+static int
+linux_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
+{
+  struct find_mapping_size_args args;
+
+  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &args.vaddr) <= 0)
+    return 0;
+
+  /* This is installed by linux_init_abi below, so should always be
+     available.  */
+  gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ()));
+
+  args.size = 0;
+  gdbarch_find_memory_regions (target_gdbarch (),
+			       find_mapping_size, &args);
+
+  *start = args.vaddr;
+  *end = *start + args.size;
+  return 1;
+}
+
 /* To be called from the various GDB_OSABI_LINUX handlers for the
    various GNU/Linux architectures and machine types.  */

@@ -1799,6 +1847,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 				      linux_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch,
 				    linux_gdb_signal_to_target);
+  set_gdbarch_vsyscall_range (gdbarch, linux_vsyscall_range);
 }

 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 3deef20..cc38979 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1462,10 +1462,11 @@ svr4_current_sos_direct (struct svr4_info *info)
   return head;
 }

-/* Implement the "current_sos" target_so_ops method.  */
+/* Implement the main part of the "current_sos" target_so_ops
+   method.  */

 static struct so_list *
-svr4_current_sos (void)
+svr4_current_sos_1 (void)
 {
   struct svr4_info *info = get_svr4_info ();

@@ -1478,6 +1479,44 @@ svr4_current_sos (void)
   return svr4_current_sos_direct (info);
 }

+/* Implement the "current_sos" target_so_ops method.  */
+
+static struct so_list *
+svr4_current_sos (void)
+{
+  struct so_list *so_head = svr4_current_sos_1 ();
+  struct objfile *objfile;
+  struct obj_section *osect;
+  CORE_ADDR vsyscall_start, vsyscall_end;
+
+  /* Filter out the vDSO module, if present.  Its symbol file would
+     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
+     managed by symfile-mem.c:add_vsyscall_page.  */
+  if (gdbarch_vsyscall_range (target_gdbarch (),
+			      &vsyscall_start, &vsyscall_end))
+    {
+      struct so_list **sop;
+
+      sop = &so_head;
+      while (*sop != NULL)
+	{
+	  struct so_list *so = *sop;
+
+	  if (vsyscall_start <= so->lm_info->l_ld
+	      && so->lm_info->l_ld < vsyscall_end)
+	    {
+	      *sop = so->next;
+	      free_so (so);
+	      break;
+	    }
+
+	  sop = &so->next;
+	}
+    }
+
+  return so_head;
+}
+
 /* Get the address of the link_map for a given OBJFILE.  */

 CORE_ADDR
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index ef48f7d..6b5fa7a 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -188,33 +188,16 @@ symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
   return 0;
 }

-/* Rummage through mappings to find the vsyscall page size.  */
-
-static int
-find_vdso_size (CORE_ADDR vaddr, unsigned long size,
-		int read, int write, int exec, int modified,
-		void *data)
-{
-  struct symbol_file_add_from_memory_args *args = data;
-
-  if (vaddr == args->sysinfo_ehdr)
-    {
-      args->size = size;
-      return 1;
-    }
-  return 0;
-}
-
 /* Try to add the symbols for the vsyscall page, if there is one.
    This function is called via the inferior_created observer.  */

 static void
 add_vsyscall_page (struct target_ops *target, int from_tty)
 {
-  CORE_ADDR sysinfo_ehdr;
+  CORE_ADDR vsyscall_start, vsyscall_end;

-  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
-      && sysinfo_ehdr != (CORE_ADDR) 0)
+  if (gdbarch_vsyscall_range (target_gdbarch (),
+			      &vsyscall_start, &vsyscall_end))
     {
       struct bfd *bfd;
       struct symbol_file_add_from_memory_args args;
@@ -237,14 +220,11 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	  return;
 	}
       args.bfd = bfd;
-      args.sysinfo_ehdr = sysinfo_ehdr;
-      args.size = 0;
-      if (gdbarch_find_memory_regions_p (target_gdbarch ()))
-	(void) gdbarch_find_memory_regions (target_gdbarch (),
-					    find_vdso_size, &args);
+      args.sysinfo_ehdr = vsyscall_start;
+      args.size = vsyscall_end - vsyscall_start;

       args.name = xstrprintf ("system-supplied DSO at %s",
-			      paddress (target_gdbarch (), sysinfo_ehdr));
+			      paddress (target_gdbarch (), vsyscall_start));
       /* Pass zero for FROM_TTY, because the action of loading the
 	 vsyscall DSO was not triggered by the user, even if the user
 	 typed "run" at the TTY.  */
diff --git a/gdb/testsuite/gdb.base/vdso-warning.c b/gdb/testsuite/gdb.base/vdso-warning.c
new file mode 100644
index 0000000..4b94803
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vdso-warning.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/vdso-warning.exp b/gdb/testsuite/gdb.base/vdso-warning.exp
new file mode 100644
index 0000000..1f538fa
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vdso-warning.exp
@@ -0,0 +1,54 @@
+# Copyright 2012-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile] } {
+    return -1
+}
+
+gdb_breakpoint "main"
+
+# At least some versions of Fedora/RHEL glibc have local patches that
+# hide the vDSO.  This lines re-exposes it.  See PR libc/13097,
+# comment 2.  There's no support for passing environment variables in
+# the remote protocol, but that's OK -- if we're testing against a
+# glibc that doesn't list the vDSO without this, the test should still
+# pass.
+gdb_test_no_output "set environment LD_DEBUG=unused"
+
+gdb_run_cmd
+
+set test "stop without warning"
+gdb_test_multiple "" $test {
+    -re "Could not load shared library symbols .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\nBreakpoint \[0-9\]+, main .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+# Extra testing in case the warning changes and we miss updating the
+# above.
+set test "no vdso without symbols is listed"
+gdb_test_multiple "info shared" $test {
+    -re "No\[^\r\n\]+linux-(vdso|gate).*$gdb_prompt $" {
+	fail $test
+    }
+    -re "$gdb_prompt $" {
+	pass $test
+    }
+}
-- 
1.9.3



Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
> Is it really a pain though?

95 lines of gdbarch.* patch + its ChangeLog is really a pain compared to
1 line of C++ virtual override.


> Sounds like a predicate like this would work then?
> 
> 	  if (vsyscall_start <= so->lm_info->l_ld && so->lm_info->l_ld < vsyscall_end)

Yes, I find this test correct.


On Sun, 21 Sep 2014 21:46:38 +0200, Pedro Alves wrote:
> Here's a quick update to the patch that does (just) that, for
> testing.  Doesn't update comments, etc.  It works on f20 here.
> 
> WDYT ?

Yes, it works for me on kernel-2.6.32-220.el6.x86_64 (also verified your
previous patch still displayed the warning).

Clarifying it does not fulfill this your suggestion:

On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
# I was more inclined to leave the vdso in the shared library list
# though, like ldd does, than filtering it out.

But I understand it was more a suggestion than requirement for patch acceptance.
IMO it was request for an unrelated feature.


> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1029,6 +1029,10 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0
>  # Return -1 if there is insufficient buffer for a whole entry.
>  # Return 1 if an entry was read into *TYPEP and *VALP.
>  M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp
> +
> +# Find the address range of the current inferior's vsyscall/vDSO, and
> +# write it to *START, *END.  Returns true if found, false otherwise.

I find unclear the description whether *END is the last byte address or the
after-the-last byte address.


> +m:int:vsyscall_range:CORE_ADDR *start, CORE_ADDR *end:start, end::default_vsyscall_range::0
>  EOF
>  }
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index dae59c5..3f28fc9 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1782,6 +1782,54 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
>    return -1;
>  }
> 
> +/* Arguments for symbol_file_add_from_memory_wrapper.  */
> +
> +struct find_mapping_size_args
> +{
> +  CORE_ADDR vaddr;
> +  size_t size;

size_t and not CORE_ADDR?  (This patch uses also unsigned long for this value.)


> +};
> +
> +/* Rummage through mappings to find a mapping size.  */
> +
> +static int
> +find_mapping_size (CORE_ADDR vaddr, unsigned long size,
> +		   int read, int write, int exec, int modified,
> +		   void *data)
> +{
> +  struct find_mapping_size_args *args = data;
> +
> +  if (vaddr == args->vaddr)
> +    {
> +      args->size = size;
> +      return 1;
> +    }
> +  return 0;
> +}
> +
> +/* Implementation of the "vsyscall_range" gdbarch hook.  */
> +
> +static int
> +linux_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
> +{
> +  struct find_mapping_size_args args;
> +
> +  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &args.vaddr) <= 0)
> +    return 0;
> +
> +  /* This is installed by linux_init_abi below, so should always be
> +     available.  */
> +  gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ()));

Is there a reason for target_gdbarch () and not gdbarch?


> +
> +  args.size = 0;
> +  gdbarch_find_memory_regions (target_gdbarch (),
> +			       find_mapping_size, &args);
> +
> +  *start = args.vaddr;
> +  *end = *start + args.size;
> +  return 1;

Here it returns 1 even if the vDSO was not found.
It will return *start == *end so the current only caller behaves correctly.
But I do not find it fully compliant to its gdbarch.sh documentation.


> +}
> +
>  /* To be called from the various GDB_OSABI_LINUX handlers for the
>     various GNU/Linux architectures and machine types.  */
> 
> @@ -1799,6 +1847,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  				      linux_gdb_signal_from_target);
>    set_gdbarch_gdb_signal_to_target (gdbarch,
>  				    linux_gdb_signal_to_target);
> +  set_gdbarch_vsyscall_range (gdbarch, linux_vsyscall_range);
>  }
> 
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
[...]
> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -188,33 +188,16 @@ symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
>    return 0;
>  }
> 
> -/* Rummage through mappings to find the vsyscall page size.  */
> -
> -static int
> -find_vdso_size (CORE_ADDR vaddr, unsigned long size,
> -		int read, int write, int exec, int modified,
> -		void *data)
> -{
> -  struct symbol_file_add_from_memory_args *args = data;
> -
> -  if (vaddr == args->sysinfo_ehdr)
> -    {
> -      args->size = size;
> -      return 1;
> -    }
> -  return 0;
> -}
> -
>  /* Try to add the symbols for the vsyscall page, if there is one.
>     This function is called via the inferior_created observer.  */
> 
>  static void
>  add_vsyscall_page (struct target_ops *target, int from_tty)
>  {
> -  CORE_ADDR sysinfo_ehdr;
> +  CORE_ADDR vsyscall_start, vsyscall_end;
> 
> -  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
> -      && sysinfo_ehdr != (CORE_ADDR) 0)
> +  if (gdbarch_vsyscall_range (target_gdbarch (),
> +			      &vsyscall_start, &vsyscall_end))

This is a code cleanup part of the patch which could be separate.


>      {
>        struct bfd *bfd;
>        struct symbol_file_add_from_memory_args args;


Thanks,
Jan

automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Jan Kratochvil]

On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
> On 09/20/2014 10:30 PM, Jan Kratochvil wrote:
> > But for example kernel-2.6.32-220.el6.x86_64 is "prelinked", see below.
> 
> Ah, didn't know that.  That's the sort of thing we should have in
> comments in the code, or at least in the commit log.

That apparently would not work, comment is added only after the problem is
discovered.

It would be found by automated testing upon submitting patch for reviews, such
as I have seen done through Jenkins connected to Gerrit.

At least assuming the same "prelinked vDSO" (which is definitely not done by
the prelink program) issue also happens on some non-Fedora/non-RHEL OSes
(as Fedora/RHEL OSes have patched glibc where the vDSO is not listed in the
link map at all and so the patch being discussed is not needed there).

And sure deploying automated testing with the current GDB testsuite as is
would not work now automatically as the testsuite has fuzzy results.  Although
at least running new testcases (from the patch under review) would work which
would be sufficient in this case (but not in other cases - regression cases).


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/20/2014 08:50 PM, Jan Kratochvil wrote:
> On Fri, 19 Sep 2014 16:38:07 +0200, Pedro Alves wrote:
>> On 09/17/2014 09:10 PM, Jan Kratochvil wrote:
>>> You seem to evaluate the patches by some other metric which I cannot guess
>>> myself in advance to coding a patch.
>>
>> It's simply the metric of someone who believes that GDB is here
>> to stay, and therefore weighs impact of changes both in the present
>> and in the future.
> 
> Then it is (IMO) most time effective to rewrite GDB to C++ first.

What I don't think that we should halt all development and
"rewrite" GDB to anything _first_.  Instead, let that be done in
parallel.  Let me remind you that I'm still on the C++ camp.
Last we discussed this, I suggested that we should have a wiki page
describing the project, summarizing previous discussions,
previously identified obstacles, proposed solutions for same,
listing a suggested conversion roadmap ("gcc -Wc++-compat" -> "still C but
built with g++", etc.),  etc.  That hasn't happened yet.

> But it has
> some organizational issues as the improved stability, speed and maintenance
> cost may (or may not?) be lower priority than specific fixes/improvements
> requested by users.  Which leads to short time vs. long time goals.
> I also can't forget to mention there is also LLDB.
> 
> 
> On Fri, 19 Sep 2014 16:38:07 +0200, Pedro Alves wrote:
>> Perhaps not surprisingly, I disagree.
> 
> We therefore both agree on our disagreement.

TBC, what I don't agree with is the view that anything other
than converting to C++ first is wrong:

 "Any fix present on gdb-patches is _wrong_
  as it is not written in an effective/maintainable language"

(emphasis above is mine.)

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/22/2014 07:35 PM, Jan Kratochvil wrote:
> On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
>> Is it really a pain though?
> 
> 95 lines of gdbarch.* patch + its ChangeLog is really a pain compared to 1 line of C++ virtual override.

That is a different complain from the original "That's the
pain of solib-svr4.c which is OS-agnostic and so it cannot find out start
of the ELF file just from link map.  gdbserver can find it as it can
depend on /proc/PID/{s,}maps as its linux-low.c is Linux-specific."

But still, well, that's a bogus comparison and you know that.  You wouldn't
get very far with just a 1 line...   Even if GDB was written in C++, I'd
probably still want to hook this through the gdbarch object, as it's
a lower level thing than target_so_ops, which is consumed by
add_vsyscall_table too.  Most of those 95 lines include generated
boilerplace that you'd need in C++ too.  You'd need to count debug
dump code, validation code, and the new entry point in the base object,
and both the declaration and the definition of the override in the
new class.

It's actually quite possible to add a linux-specific target_so_ops that
inherits svr4_so_ops, and we even do that for a few targets/archs).
That's very much like the C++ virtual override, but even if we did
that, istm we'd still make the new "class" use the new gdbarch method in
its current_sos() method.  I didn't do that as it doesn't seem worth it
to add a new class for this, given that the gdbarch method returns false
on the exact same set of targets that wouldn't use the new class, and
then again, at some point, we may actually want to move the filtering
of the vdso out close to where we already filter out a dso whose name
is the same as of the executable, assuming it's the vdso.

Yes, I know that C++ makes writing some of these things easier.
No need to keep repeating it to me.  The thing is that most of
the design issues here are orthogonal to the C/C++ axis.  And that
shouldn't surprise, exactly because GDB is written in what some would
call poor-man's C++-in-C style.

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/22/2014 07:35 PM, Jan Kratochvil wrote:
> Clarifying it does not fulfill this your suggestion:
> 
> On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote:
> # I was more inclined to leave the vdso in the shared library list
> # though, like ldd does, than filtering it out.
> 
> But I understand it was more a suggestion than requirement for patch acceptance.
> IMO it was request for an unrelated feature.

I don't think of it as unrelated (your original patch did that even, after all),
but also not required for acceptance.

An important goal of review and maintenance IMO is checking whether we're
taking steps in the right direction.  I hadn't identified the issues
with solib-svr4.c vs symfile-mem.c and DSO lifetimes at that point.

As I said, I've investigated this further now.  I now believe
that we're still a couple steps away from being able to list the vdso
without causing other issues.  In the patch'es commit log, I wrote:

"It would actually be nice if GDB also listed the vdso in the shared library
list, but there are some design considerations with that:

 - the vDSO is mapped by the kernel, not userspace, therefore we
   should load its symbols right from the process's start of life,
   even before glibc / the userspace loader sets up the initial DSO
   list.  The program might even be using a custom loader or no
   loader.

 - that kind of hints at that solib.c should handle retrieving shared
   library lists from more than one source, and that symfile-mem.c's
   loading of the vdso would be converted to load and relocate the
   vdso's bfd behind the target_so_ops interface.

 - and then, once glibc links in the vdso to its DSO list, we'd need
   to either:

    a) somehow hand over the vdso from one target_so_ops to the
      other
    b) or simply keep hiding glibc's entry.

And then b) seems the simplest."

IOW, I'm now convinced that making solib-svr4.c hide the vDSO is
_not_ a step backwards from making "info shared" list the vDSO.

I didn't like much the way the address matching was done though,
hence the new patch revision.

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/23/2014 11:58 AM, Jan Kratochvil wrote:
> On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
>> On 09/20/2014 10:30 PM, Jan Kratochvil wrote:
>>> But for example kernel-2.6.32-220.el6.x86_64 is "prelinked", see below.
>>
>> Ah, didn't know that.  That's the sort of thing we should have in
>> comments in the code, or at least in the commit log.
> 
> That apparently would not work, comment is added only after the problem is
> discovered.

:-)  Ah, OK.  I thought that that was already the reason you
didn't match the vDSO using AT_SYSINFO_EHDR in your original patch.

> It would be found by automated testing upon submitting patch for reviews, such
> as I have seen done through Jenkins connected to Gerrit.

Or even after the patch is in, and we can revert if build / test bots
find a problem.  Seems like a simpler step that I don't think anyone
would object to ...

Seems like Jan-Benedict Glaw is running a buildbot that includes GDB:

  http://toolchain.lug-owl.de/buildbot/

No idea what system that runs on, and whether he runs the testsuite
as well.

Sergio was also interested in setting up a GDB build bot.

There's the gcc compile farm too.

> And sure deploying automated testing with the current GDB testsuite as is
> would not work now automatically as the testsuite has fuzzy results.

We should be able to filter those out though.  Of course ideally we'd
just fix them to not be fuzzy...

> Although at least running new testcases (from the patch under review) would work which
> would be sufficient in this case (but not in other cases - regression cases).

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Jan Kratochvil]

On Tue, 23 Sep 2014 14:32:32 +0200, Pedro Alves wrote:
> :-)  Ah, OK.  I thought that that was already the reason you
> didn't match the vDSO using AT_SYSINFO_EHDR in your original patch.

OK, true, I could make some such patch already in the original patch but
I found that part obvious. :-)


> > It would be found by automated testing upon submitting patch for reviews, such
> > as I have seen done through Jenkins connected to Gerrit.
> 
> Or even after the patch is in, and we can revert if build / test bots
> find a problem.  Seems like a simpler step that I don't think anyone
> would object to ...

I find the primary advantage that it tests the patches already before review.
The same way the patches are automatically checked for proper code formatting.
So one no longer has to lose time on those mechanical parts during reviews,
which is one of the reasons I stopped doing them regularly (if I ever did).


> Seems like Jan-Benedict Glaw is running a buildbot that includes GDB:
> Sergio was also interested in setting up a GDB build bot.
> There's the gcc compile farm too.

I do not know who is running which bots but so far it seems to me I am the
only one paying some attention to their results - or are there other
regression bugreports I miss on the list?


> We should be able to filter those out though.  Of course ideally we'd
> just fix them to not be fuzzy...

I do not see how to filter them automatically.  The gdb.mi/mi-nsintrall.exp
regression today looks exactly like one of the many nightly fuzzy results but
in the end it has proven to be a real regression.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Gary Benson]

Pedro Alves wrote:
> On 09/20/2014 08:50 PM, Jan Kratochvil wrote:
> > Then it is (IMO) most time effective to rewrite GDB to C++
> > first.
[snip]
> Let me remind you that I'm still on the C++ camp.  Last we
> discussed this, I suggested that we should have a wiki page
> describing the project, summarizing previous discussions,
> previously identified obstacles, proposed solutions for same,
> listing a suggested conversion roadmap ("gcc -Wc++-compat"
> -> "still C but built with g++", etc.), etc.  That hasn't
> happened yet.

This is my fault, it was on my task list and I haven't done
it.

Sorry,
Gary

-- 
http://gbenson.net/

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/23/2014 01:45 PM, Jan Kratochvil wrote:

>> Seems like Jan-Benedict Glaw is running a buildbot that includes GDB:
>> Sergio was also interested in setting up a GDB build bot.
>> There's the gcc compile farm too.
> 
> I do not know who is running which bots but so far it seems to me I am the
> only one paying some attention to their results - or are there other
> regression bugreports I miss on the list?

On testing regressions, indeed you may the only one doing
something like that currently.  Is your build/test bot some
place public others can look at results?

I saw Jan-Benedict acting on a build regression caught by the
build bot here:

  https://sourceware.org/ml/gdb-patches/2014-09/msg00658.html

Seems like we should at least have a central place that lists
the existing public bots / auto testers.  Like, a wiki page.  :-)

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Jan Kratochvil]

On Tue, 23 Sep 2014 15:30:14 +0200, Pedro Alves wrote:
> On testing regressions, indeed you may the only one doing
> something like that currently.  Is your build/test bot some
> place public others can look at results?

Sanimir Agovic asked for results subscription in the past, it had no public
access so far as I am still waiting till some more "business ran" testing bot
gets deployed.  So I made accessible what I have now ...


> Seems like we should at least have a central place that lists
> the existing public bots / auto testers.  Like, a wiki page.  :-)

... at the wiki page:
	https://sourceware.org/gdb/wiki/BuildBotResults
	https://sourceware.org/gdb/wiki/HomePageDevelopingColumn?action=diff&rev2=33&rev1=32


> I saw Jan-Benedict acting on a build regression caught by the
> build bot here:
> 
>   https://sourceware.org/ml/gdb-patches/2014-09/msg00658.html

OK, great, I see there is running second testbot.


Jan

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Doug Evans]

On Tue, Sep 23, 2014 at 3:58 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> And sure deploying automated testing with the current GDB testsuite as is
> would not work now automatically as the testsuite has fuzzy results.  Although
> at least running new testcases (from the patch under review) would work which
> would be sufficient in this case (but not in other cases - regression cases).

Sometimes tests are flaky and one isn't aware of it, and not
everything gets caught by patch review.

IWBN if automated testing reran everything failing test N times (10?)
and reported those results as something like "fail" (always failed) or
"flaky" (inconsistent).

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/23/2014 02:57 PM, Jan Kratochvil wrote:
> On Tue, 23 Sep 2014 15:30:14 +0200, Pedro Alves wrote:
>> On testing regressions, indeed you may the only one doing
>> something like that currently.  Is your build/test bot some
>> place public others can look at results?
> 
> Sanimir Agovic asked for results subscription in the past, it had no public
> access so far as I am still waiting till some more "business ran" testing bot
> gets deployed.  So I made accessible what I have now ...

Thanks!

(fyi, http://host2.jankratochvil.net/gdbmail seems to not be working for me atm.)

I think it'd be fine to send the periodic email results/alerts/whatever to:

 https://sourceware.org/ml/gdb-testresults/

That list hasn't been active in a while, but it's still alive, afaics,
and the point of that list was to collect auto testers' test results.

>> Seems like we should at least have a central place that lists
>> the existing public bots / auto testers.  Like, a wiki page.  :-)
> 
> ... at the wiki page:
> 	https://sourceware.org/gdb/wiki/BuildBotResults
> 	https://sourceware.org/gdb/wiki/HomePageDevelopingColumn?action=diff&rev2=33&rev1=32

Awesome, thank you.

>> I saw Jan-Benedict acting on a build regression caught by the
>> build bot here:
>>
>>   https://sourceware.org/ml/gdb-patches/2014-09/msg00658.html
> 
> OK, great, I see there is running second testbot.

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Doug Evans]

On Tue, Sep 23, 2014 at 5:45 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> I find the primary advantage that it tests the patches already before review.

That would indeed be awesome.

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/23/2014 03:48 PM, Doug Evans wrote:
> On Tue, Sep 23, 2014 at 3:58 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> And sure deploying automated testing with the current GDB testsuite as is
>> would not work now automatically as the testsuite has fuzzy results.  Although
>> at least running new testcases (from the patch under review) would work which
>> would be sufficient in this case (but not in other cases - regression cases).
> 
> Sometimes tests are flaky and one isn't aware of it, and not
> everything gets caught by patch review.
> 
> IWBN if automated testing reran everything failing test N times (10?)
> and reported those results as something like "fail" (always failed) or
> "flaky" (inconsistent).

Indeed.  I think that would be doable as part of gdb's own
testsuite machinery even, which would make it easily available
to every one.

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Simon Marchi]

On 2014-09-23 08:32 AM, Pedro Alves wrote:
>> And sure deploying automated testing with the current GDB testsuite as is
>> would not work now automatically as the testsuite has fuzzy results.
> 
> We should be able to filter those out though.  Of course ideally we'd
> just fix them to not be fuzzy...

If we adopt automated testing as part of the process and those fuzzy tests are
in the way, it will help identify them and it will give some a motivation to fix
them.

What is bugging me is also the failing test cases in master. In order to make
Jenkins (or similar) useful, you need a baseline where everything is green.
Otherwise all patches that will get tested will fail and it will be difficult
to weed out the actually failing patches from the good ones.

Unless there is actually a way to make a pass/fail/unresolved "diff" that
compares the patch's results to those of the latest master commit's results.
Last time I checked, it didn't seem obvious.

Simon

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Jan Kratochvil]

On Tue, 23 Sep 2014 16:48:50 +0200, Pedro Alves wrote:
https://sourceware.org/gdb/wiki/BuildBotResults
->
> (fyi, http://host2.jankratochvil.net/gdbmail seems to not be working for me atm.)

The only idea I have is you still do not have IPv6 connectivity.


> I think it'd be fine to send the periodic email results/alerts/whatever to:
> 
>  https://sourceware.org/ml/gdb-testresults/

I have set up that address, we'll see tomorrow.  I did not know that list.


Jan

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/23/2014 04:53 PM, Jan Kratochvil wrote:
> On Tue, 23 Sep 2014 16:48:50 +0200, Pedro Alves wrote:
> https://sourceware.org/gdb/wiki/BuildBotResults
> ->
>> (fyi, http://host2.jankratochvil.net/gdbmail seems to not be working for me atm.)
> 
> The only idea I have is you still do not have IPv6 connectivity.

Yes, that's right.

>> I think it'd be fine to send the periodic email results/alerts/whatever to:
>>
>>  https://sourceware.org/ml/gdb-testresults/
> 
> I have set up that address, we'll see tomorrow.  I did not know that list.

Awesome, progress.  :-)

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Doug Evans]

On Fri, Sep 12, 2014 at 6:06 PM, Doug Evans <dje@google.com> wrote:
> I downloaded glibc 2.20 and tested it with gdb 7.8.
> Still an issue.
> [I just did a simple build, if there's a configure option
> that will fix things I didn't try one.]
> I think FSF gdb 7.8[.1?] (the latest) should work with FSF glibc 2.20
> (the latest).

Data point: Evidently people on Arch Linux are tripping over this, so
there are real users affected here.

They're also affected by another bug where we're not reading their
vdso properly and getting bfd error "File truncated".

Re: time to workaround libc/13097 in fsf gdb?

[Doug Evans]

On Sun, Sep 21, 2014 at 12:46 PM, Pedro Alves <palves@redhat.com> wrote:
>[...]
> @@ -1478,6 +1479,44 @@ svr4_current_sos (void)
>    return svr4_current_sos_direct (info);
>  }
>
> +/* Implement the "current_sos" target_so_ops method.  */
> +
> +static struct so_list *
> +svr4_current_sos (void)
> +{
> +  struct so_list *so_head = svr4_current_sos_1 ();
> +  struct objfile *objfile;
> +  struct obj_section *osect;
> +  CORE_ADDR vsyscall_start, vsyscall_end;
> +
> +  /* Filter out the vDSO module, if present.  Its symbol file would
> +     not be found on disk.  The vDSO/vsyscall's OBJFILE is instead
> +     managed by symfile-mem.c:add_vsyscall_page.  */
> +  if (gdbarch_vsyscall_range (target_gdbarch (),
> +                             &vsyscall_start, &vsyscall_end))
> +    {
> +      struct so_list **sop;
> +
> +      sop = &so_head;
> +      while (*sop != NULL)
> +       {
> +         struct so_list *so = *sop;
> +
> +         if (vsyscall_start <= so->lm_info->l_ld
> +             && so->lm_info->l_ld < vsyscall_end)

This test is subtle enough that I'd appreciate a full explanation here
of what's going on.
Thanks.

> +           {
> +             *sop = so->next;
> +             free_so (so);
> +             break;
> +           }
> +
> +         sop = &so->next;
> +       }
> +    }
> +
> +  return so_head;
> +}
> +

I may have a few more comments, but I think Jan has covered them, so
I'm going to wait until the next version for further review.

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Andreas Arnez]

On Tue, Sep 23 2014, Pedro Alves wrote:

> I think it'd be fine to send the periodic email results/alerts/whatever to:
>
>  https://sourceware.org/ml/gdb-testresults/
>
> That list hasn't been active in a while, but it's still alive, afaics,
> and the point of that list was to collect auto testers' test results.

Interesting.  Even before I started working on GDB, Andreas Krebbel had
set up a bot that sends test results to a different list, and we're
still continuing to do so:

  https://sourceware.org/ml/gdb-testers/

Knowing now that there's also gdb-testresults, I wonder whether that
ever was a good choice.  We could certainly change that, so gdb-testers
is freed up for discussions like this one ;-)

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Ulrich Weigand]

Andreas Arnez wrote:
> On Tue, Sep 23 2014, Pedro Alves wrote:
> 
> > I think it'd be fine to send the periodic email results/alerts/whatever to:
> >
> >  https://sourceware.org/ml/gdb-testresults/
> >
> > That list hasn't been active in a while, but it's still alive, afaics,
> > and the point of that list was to collect auto testers' test results.
> 
> Interesting.  Even before I started working on GDB, Andreas Krebbel had
> set up a bot that sends test results to a different list, and we're
> still continuing to do so:
> 
>   https://sourceware.org/ml/gdb-testers/
> 
> Knowing now that there's also gdb-testresults, I wonder whether that
> ever was a good choice.  We could certainly change that, so gdb-testers
> is freed up for discussions like this one ;-)

Well, I guess that would be because the main web page:
  https://www.sourceware.org/gdb/mailing-lists/
says:

gdb-testers
    is a list for the announcement of development snapshots and the reporting of test results.

and does not mention gdb-testresults at all.

I think we should agree on one of them, and document it on the web page.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Andreas Arnez]

On Wed, Sep 24 2014, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>> On Tue, Sep 23 2014, Pedro Alves wrote:
>> 
>> > I think it'd be fine to send the periodic email results/alerts/whatever to:
>> >
>> >  https://sourceware.org/ml/gdb-testresults/
>> >
>> > That list hasn't been active in a while, but it's still alive, afaics,
>> > and the point of that list was to collect auto testers' test results.
>> 
>> Interesting.  Even before I started working on GDB, Andreas Krebbel had
>> set up a bot that sends test results to a different list, and we're
>> still continuing to do so:
>> 
>>   https://sourceware.org/ml/gdb-testers/
>> 
>> Knowing now that there's also gdb-testresults, I wonder whether that
>> ever was a good choice.  We could certainly change that, so gdb-testers
>> is freed up for discussions like this one ;-)
>
> Well, I guess that would be because the main web page:
>   https://www.sourceware.org/gdb/mailing-lists/
> says:
>
> gdb-testers
>     is a list for the announcement of development snapshots and the reporting of test results.
>
> and does not mention gdb-testresults at all.

Right, I also noticed that gdb-testers is mirrored by Gmane (as
comp.gdb.testing) and gdb-testresults is not.  I'm puzzled why
gdb-testresults was ever created...

> I think we should agree on one of them, and document it on the web page.

For instance by leaving everything as-is?  Or maybe, for clarity, we
should at least drop the obsolete "announcement of development
snapshots" from the list description?

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Pedro Alves]

On 09/24/2014 04:23 PM, Ulrich Weigand wrote:
> Andreas Arnez wrote:
>> On Tue, Sep 23 2014, Pedro Alves wrote:
>>
>>> I think it'd be fine to send the periodic email results/alerts/whatever to:
>>>
>>>  https://sourceware.org/ml/gdb-testresults/
>>>
>>> That list hasn't been active in a while, but it's still alive, afaics,
>>> and the point of that list was to collect auto testers' test results.
>>
>> Interesting.  Even before I started working on GDB, Andreas Krebbel had
>> set up a bot that sends test results to a different list, and we're
>> still continuing to do so:
>>
>>   https://sourceware.org/ml/gdb-testers/
>>
>> Knowing now that there's also gdb-testresults, I wonder whether that
>> ever was a good choice.  We could certainly change that, so gdb-testers
>> is freed up for discussions like this one ;-)
> 
> Well, I guess that would be because the main web page:
>   https://www.sourceware.org/gdb/mailing-lists/
> says:
> 
> gdb-testers
>     is a list for the announcement of development snapshots and the reporting of test results.
> 
> and does not mention gdb-testresults at all.
> 
> I think we should agree on one of them, and document it on the web page.

Certainly fine with me.  I completely missed that we had two
lists for this.  The only reason I suggested gdb-testresults was that
I knew there was a test results mailing list, but, I wasn't subscribed
to it, and then when I went looking for the archives url, gdb-testresults
was what Firefox url history bar autocompleted.  :-)

Thanks,
Pedro Alves

Re: automated testing comment [Re: time to workaround libc/13097 in fsf gdb?]

[Jan Kratochvil]

On Thu, 25 Sep 2014 10:20:29 +0200, Pedro Alves wrote:
> On 09/24/2014 04:23 PM, Ulrich Weigand wrote:
> > I think we should agree on one of them, and document it on the web page.
> 
> Certainly fine with me.  I completely missed that we had two
> lists for this.

Changed:
	https://sourceware.org/gdb/wiki/BuildBotResults?action=diff&rev2=3&rev1=2


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/22/2014 07:35 PM, Jan Kratochvil wrote:

> Yes, it works for me on kernel-2.6.32-220.el6.x86_64 (also verified your
> previous patch still displayed the warning).

Thanks for the testing.

>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -1029,6 +1029,10 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0
>>  # Return -1 if there is insufficient buffer for a whole entry.
>>  # Return 1 if an entry was read into *TYPEP and *VALP.
>>  M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp
>> +
>> +# Find the address range of the current inferior's vsyscall/vDSO, and
>> +# write it to *START, *END.  Returns true if found, false otherwise.
> 
> I find unclear the description whether *END is the last byte address or the
> after-the-last byte address.

OK.  Best just use "struct mem_range" instead and avoid this
ambiguity whenever we need a range.  That struct uses "int" for
length.  Although that should be fixed, that can be done
independently.  int is obviously sufficient for the vDSO.

>> +/* Arguments for symbol_file_add_from_memory_wrapper.  */
>> +
>> +struct find_mapping_size_args
>> +{
>> +  CORE_ADDR vaddr;
>> +  size_t size;
> 
> size_t and not CORE_ADDR?  (This patch uses also unsigned long for this value.)

I assume you means ULONGEST or some such.  But, I just moved that
code (and renamed it).  IIRC, it's size_t because that's what the
"bfd from remote memory" interface uses.  This structure disappears in
the next version.  We can work with struct mem_range here too.

>> +
>> +static int
>> +linux_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end)
>> +{
>> +  struct find_mapping_size_args args;
>> +
>> +  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &args.vaddr) <= 0)
>> +    return 0;
>> +
>> +  /* This is installed by linux_init_abi below, so should always be
>> +     available.  */
>> +  gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ()));
> 
> Is there a reason for target_gdbarch () and not gdbarch?

No reason.

> 
> 
>> +
>> +  args.size = 0;
>> +  gdbarch_find_memory_regions (target_gdbarch (),
>> +			       find_mapping_size, &args);
>> +
>> +  *start = args.vaddr;
>> +  *end = *start + args.size;
>> +  return 1;
> 
> Here it returns 1 even if the vDSO was not found.
> It will return *start == *end so the current only caller behaves correctly.
> But I do not find it fully compliant to its gdbarch.sh documentation.

Yeah.  The next version will make that clear in the documentation.

>>
>>  static void
>>  add_vsyscall_page (struct target_ops *target, int from_tty)
>>  {
>> -  CORE_ADDR sysinfo_ehdr;
>> +  CORE_ADDR vsyscall_start, vsyscall_end;
>>
>> -  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
>> -      && sysinfo_ehdr != (CORE_ADDR) 0)
>> +  if (gdbarch_vsyscall_range (target_gdbarch (),
>> +			      &vsyscall_start, &vsyscall_end))
> 
> This is a code cleanup part of the patch which could be separate.

OK.  The next version splits this out to a preparatory patch.

Thanks.  I'll be posting the new series in a bit.

Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 09/24/2014 12:05 AM, Doug Evans wrote:
> On Sun, Sep 21, 2014 at 12:46 PM, Pedro Alves <palves@redhat.com> wrote:
>> [...]
>> Here's a quick update to the patch that does (just) that, for
>> testing.  Doesn't update comments, etc.  It works on f20 here.

>> +         if (vsyscall_start <= so->lm_info->l_ld
>> +             && so->lm_info->l_ld < vsyscall_end)
> 
> This test is subtle enough that I'd appreciate a full explanation here
> of what's going on.
> Thanks.
> 

Yeah, as I mentioned, that was just a quick patch for testing.

I'll be sending out the next version in a new thread, which adds
a big comment here.

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Tue, 23 Sep 2014 13:49:41 +0200, Pedro Alves wrote:
> On 09/22/2014 07:35 PM, Jan Kratochvil wrote:
> > On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote:
> >> Is it really a pain though?
> > 
> > 95 lines of gdbarch.* patch + its ChangeLog is really a pain compared to 1 line of C++ virtual override.
[...]
> But still, well, that's a bogus comparison and you know that.

No; or in part - just that 95 was counted with diff context, 1 without context.

> Even if GDB was written in C++, I'd
> probably still want to hook this through the gdbarch object,

Irrelevant, "gdbarch" probably would not exist with cheap-OO language.

> Most of those 95 lines include generated
> boilerplace that you'd need in C++ too.

No.

> You'd need to count debug dump code,

No.

> validation code,

No.

> and the new entry point in the base object, and both the declaration and the
> definition of the override in the new class.

Maybe 2-3 lines, not 1. That is not important.

> The thing is that most of the design issues here are orthogonal to the C/C++
> axis.

No.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

[snip silly yes/no/yes/no game.]

I've started a proposal for C++ conversion wiki page:

 https://sourceware.org/gdb/wiki/cxx-conversion

I've shamelessly based it on both Tromey's plan and GCC's conversion
wiki, then stripped it down to bare essentials, just to get
something going.

Thanks,
Pedro Alves

Re: time to workaround libc/13097 in fsf gdb?

[Gary Benson]

Pedro Alves wrote:
> I've started a proposal for C++ conversion wiki page:
> 
>  https://sourceware.org/gdb/wiki/cxx-conversion
> 
> I've shamelessly based it on both Tromey's plan and GCC's conversion
> wiki, then stripped it down to bare essentials, just to get
> something going.

Thanks Pedro.

Cheers,
Gary

-- 
http://gbenson.net/

Re: time to workaround libc/13097 in fsf gdb?

[Doug Evans]

On Mon, Sep 29, 2014 at 6:35 AM, Pedro Alves <palves@redhat.com> wrote:
> [snip silly yes/no/yes/no game.]
>
> I've started a proposal for C++ conversion wiki page:
>
>  https://sourceware.org/gdb/wiki/cxx-conversion
>
> I've shamelessly based it on both Tromey's plan and GCC's conversion
> wiki, then stripped it down to bare essentials, just to get
> something going.

Now you need to announce this with a subject line that gets the
community's attention. :-)

Re: time to workaround libc/13097 in fsf gdb?

[Jan Kratochvil]

On Tue, 23 Sep 2014 13:18:24 +0200, Pedro Alves wrote:
> Let me remind you that I'm still on the C++ camp.

You are newly in the C++ camp.  You were blocking C++ for GDB before.


> Last we discussed this, I suggested that we should have a wiki page
> describing the project,

You suggested something different:
	"I do believe we're better off staying in C land, at least for
	a couple years more."
	https://sourceware.org/ml/gdb/2012-04/msg00063.html


I am aware you changed your opinion but this mail looked as if you were always
C++ proponent which is not true.


Jan

Re: time to workaround libc/13097 in fsf gdb?

[Pedro Alves]

On 10/09/2014 09:09 PM, Jan Kratochvil wrote:
> On Tue, 23 Sep 2014 13:18:24 +0200, Pedro Alves wrote:
>> Let me remind you that I'm still on the C++ camp.
> 
> You are newly in the C++ camp.  You were blocking C++ for GDB before.

Yes.  Fact.

For anyone else following at home, this is where I disclosed
that I had changed my mind:

  https://sourceware.org/ml/gdb/2013-08/msg00026.html

I reserve the right to discuss, learn from past mistakes, and
change my mind.

>> Last we discussed this, I suggested that we should have a wiki page
>> describing the project,
> 
> You suggested something different:
> 	"I do believe we're better off staying in C land, at least for
> 	a couple years more."
> 	https://sourceware.org/ml/gdb/2012-04/msg00063.html

Well, that's not the "last we discussed this" I was referring to.

We last discussed this in person, this last January, in the
team meeting which you attended as well, where (among many
things) we discussed what would need to be done to move
along with the GDB-in-C++ project.

> I am aware you changed your opinion but this mail looked as if you were always
> C++ proponent which is not true.

I never tried to hide that.

Adding more info to the wiki, like e.g., pointers to
existing branches and patches would be much more useful than
this "you said I said" finger-pointing.

I've extended it now some more with some info I wanted to add,
but hadn't found time to yet.

Thanks,
Pedro Alves