time to workaround libc/13097 in fsf gdb?

Message ID 541C409E.6010408@redhat.com
State Superseded
Headers

Commit Message

Pedro Alves Sept. 19, 2014, 2:41 p.m. UTC
  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
  

Comments

Jan Kratochvil Sept. 20, 2014, 9:30 p.m. UTC | #1
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
  
Pedro Alves Sept. 21, 2014, 7:12 p.m. UTC | #2
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
  
Jan Kratochvil Sept. 23, 2014, 10:58 a.m. UTC | #3
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
  
Pedro Alves Sept. 23, 2014, 12:32 p.m. UTC | #4
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
  
Jan Kratochvil Sept. 23, 2014, 12:45 p.m. UTC | #5
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
  
Pedro Alves Sept. 23, 2014, 1:30 p.m. UTC | #6
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
  
Jan Kratochvil Sept. 23, 2014, 1:57 p.m. UTC | #7
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
  
Doug Evans Sept. 23, 2014, 2:48 p.m. UTC | #8
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).
  
Pedro Alves Sept. 23, 2014, 2:48 p.m. UTC | #9
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
  
Doug Evans Sept. 23, 2014, 2:54 p.m. UTC | #10
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.
  
Pedro Alves Sept. 23, 2014, 2:59 p.m. UTC | #11
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
  
Simon Marchi Sept. 23, 2014, 3:16 p.m. UTC | #12
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
  
Jan Kratochvil Sept. 23, 2014, 3:53 p.m. UTC | #13
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
  
Pedro Alves Sept. 23, 2014, 3:56 p.m. UTC | #14
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
  
Andreas Arnez Sept. 24, 2014, 1:22 p.m. UTC | #15
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 ;-)
  

Patch

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;
+}
+
 
 /* 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
+    }
+}