warning: Could not load shared library symbols for linux-vdso.so.1.

Message ID 96141d5a-3384-8314-1274-d1348dafe0c5@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Aug. 19, 2016, 1:04 a.m. UTC
  On 08/12/2016 11:28 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Is there an easy way to check whether we're in the vdso prelinked
>> situation just from doing some address comparisions?  I feel like that
>> should be possible, but I didn't think it through.  It is is indeed
>> possible, we could alwayy skip the /proc/pid/maps parsing entirely
>> even against live processes, on modern kernels.
> 
> I don't know either.  However, looks vDSO prelink is done only on x86
> and was already removed from kernel in 2014, as far as I can see.  The
> base address of vDSO in kernel is VDSO_PRELINK, which is hard-coded.  I
> am not sure we can rely on this hard-coded constant.

I tried to come up with some clean way to detect this, and failed.

This made me have second thoughts on the approach, since there's a remote
chance that we'll hide the wrong DSO, on older kernels.

I think I found an alternative fix.  

The idea is simply to read the vDSO bfd out of memory anyway even
when we weren't able to find its size from the mappings.  When 
elf_bfd_from_remote_memory is passed '0' for size, it figures
out the size of the elf in memory from the elf structure.  If we use
that as another source for the vdso address range, then
svr4_current_sos doesn't have to change at all.

Now, in order to do this, we need to move the add_vsyscall_page
call earlier, before svr4_current_dsos is ever called, in order
to read the vdso bfd out of memory before we ever first need to
filter out the vdso.  The cleanest I could do with the current
gdbarch_vsyscall_range-based design was to do the add_vsyscall_page
call from within gdbarch_vsyscall_range.  But that looks very ugly
to me, for reading symbols from a quite innocent looking gdbarch hook.

So I bit the bullet and made a custom Linux-specific
struct solib_ops that inherits svr4_so_ops, and overrides a couple
methods for vDSO awareness.  I think the end result is clearer
and probably more extensible if/when we decide to do the work necessary
to be able to show the vdso in "info sharedlibrary", without causing
could-not-find-file warnings.

I'm attaching two patches; the first implements the new solib_ops
instance, and then the second uses the vDSO's bfd size, fixing the
bug.

Passes testing on x86_64 Fedora 23 here, as well as the new test.

Let me know what you think.

Pedro Alves
  

Comments

Yao Qi Aug. 19, 2016, 9:59 a.m. UTC | #1
Pedro Alves <palves@redhat.com> writes:

> So I bit the bullet and made a custom Linux-specific
> struct solib_ops that inherits svr4_so_ops, and overrides a couple
> methods for vDSO awareness.  I think the end result is clearer
> and probably more extensible if/when we decide to do the work necessary
> to be able to show the vdso in "info sharedlibrary", without causing
> could-not-find-file warnings.
>
> I'm attaching two patches; the first implements the new solib_ops
> instance, and then the second uses the vDSO's bfd size, fixing the
> bug.
>
> Passes testing on x86_64 Fedora 23 here, as well as the new test.
>
> Let me know what you think.

They are good to me.  Do you plan to push them to 7.12 branch?  I'd like
to keep these patches in master only, because the problem they fix is
minor while the patches are not small changes.  What do you think?
  

Patch

From 17811ce925279d3a64eb04541591cb5c567ef3cc Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 18 Aug 2016 23:00:52 +0100
Subject: [PATCH 2/2] Make vDSO detection work with core files

- Make gdb.base/vdso-warning.exp test loading a core.  With
  LD_DEBUG=unused, we see the warning on systems with local glibc
  patches as well (Fedora/RHEL).

- When debugging a core, we can't find the size of the vDSO from the
  mappings, because they're nowhere to be found.  However, we can
  still read the vDSO out of memory when we don't know its size.  BFD
  will figure it out from the ELF structure.  In fact, this is what we
  always used to do before 5979d6b69b20 ("Handle VDSO section headers
  past end of page").

  So the fix is to read the vDSO out of memory before we ever fetch
  the current list of DSOs, and extract the vDSO size out of the size
  of the bfd read in.
---
 gdb/linux-tdep.c                        | 27 +++++++-----
 gdb/symfile-mem.c                       | 24 ++++++++++-
 gdb/testsuite/gdb.base/vdso-warning.exp | 76 ++++++++++++++++++++++-----------
 3 files changed, 90 insertions(+), 37 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 91931eb..7d1d547 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -193,7 +193,7 @@  static const struct inferior_data *linux_inferior_data;
    transfering data from a remote target to the local host.  */
 struct linux_info
 {
-  /* Cache of the inferior's vsyscall/vDSO mapping range.  Only valid
+  /* Cache of the inferior's vsyscall/vDSO address range.  Only valid
      if VSYSCALL_RANGE_P is positive.  This is cached because getting
      at this info requires an auxv lookup (which is itself cached),
      and looking through the inferior's mappings (which change
@@ -2286,7 +2286,8 @@  linux_gdb_signal_to_target (struct gdbarch *gdbarch,
 }
 
 /* Helper for linux_vsyscall_range that does the real work of finding
-   the vsyscall's address range.  */
+   the vsyscall's address range based on auxv info and page
+   mappings.  */
 
 static int
 linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
@@ -2295,16 +2296,21 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
   long pid;
   char *data;
 
-  /* Can't access /proc if debugging a core file.  */
-  if (!target_has_execution)
+  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &range->start) <= 0)
     return 0;
 
+  /* Alright, we know the starting address.  Let's see if we can find
+     the end address.  */
+  range->length = 0;
+
+  /* Can't access /proc if debugging a core file, and NT_FILE notes
+     don't include the vDSO mapping.  */
+  if (!target_has_execution)
+    return 1;
+
   /* We need to know the real target PID to access /proc.  */
   if (current_inferior ()->fake_pid_p)
-    return 0;
-
-  if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &range->start) <= 0)
-    return 0;
+    return 1;
 
   pid = current_inferior ()->pid;
 
@@ -2338,8 +2344,7 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 		p++;
 	      endaddr = strtoulst (p, &p, 16);
 	      range->length = endaddr - addr;
-	      do_cleanups (cleanup);
-	      return 1;
+	      break;
 	    }
 	}
 
@@ -2348,7 +2353,7 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
   else
     warning (_("unable to open /proc file '%s'"), filename);
 
-  return 0;
+  return 1;
 }
 
 /* Find the address range of the current inferior's vDSO.  If the
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 00d20a2..e172404 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -167,11 +167,15 @@  add_symbol_file_from_memory_command (char *args, int from_tty)
 
 struct symbol_file_add_from_memory_args
 {
+  /* Inputs.  */
   struct bfd *bfd;
   CORE_ADDR sysinfo_ehdr;
   size_t size;
   char *name;
   int add_flags;
+
+  /* Outputs.  */
+  struct objfile *objf_added;
 };
 
 /* Wrapper function for symbol_file_add_from_memory, for
@@ -183,8 +187,10 @@  symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
   struct symbol_file_add_from_memory_args *args
     = (struct symbol_file_add_from_memory_args *) data;
 
-  symbol_file_add_from_memory (args->bfd, args->sysinfo_ehdr, args->size,
-			       args->name, args->add_flags);
+  args->objf_added = symbol_file_add_from_memory (args->bfd,
+						  args->sysinfo_ehdr,
+						  args->size, args->name,
+						  args->add_flags);
   return 0;
 }
 
@@ -196,6 +202,7 @@  add_vsyscall_page (struct mem_range *vsyscall_range, int add_flags)
     {
       struct bfd *bfd;
       struct symbol_file_add_from_memory_args args;
+      struct stat statbuf;
 
       if (core_bfd != NULL)
 	bfd = core_bfd;
@@ -223,6 +230,19 @@  add_vsyscall_page (struct mem_range *vsyscall_range, int add_flags)
       args.add_flags = add_flags;
       catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
 			&args, RETURN_MASK_ALL);
+
+      /* If we couldn't find the size of the vDSO from the mappings,
+	 then set it from the size of the bfd that we just read
+	 in.  */
+      if (vsyscall_range->length == 0)
+	{
+	  if (bfd_stat (args.objf_added->obfd, &statbuf) < 0)
+	    {
+	      internal_error (__FILE__, __LINE__,
+			      _("bfd_stat on memfile bfd failed"));
+	    }
+	  vsyscall_range->length = statbuf.st_size;
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/vdso-warning.exp b/gdb/testsuite/gdb.base/vdso-warning.exp
index af2b2b0..aeb85a2 100644
--- a/gdb/testsuite/gdb.base/vdso-warning.exp
+++ b/gdb/testsuite/gdb.base/vdso-warning.exp
@@ -13,42 +13,70 @@ 
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# Test that on Linux, we don't warn about not finding the vDSO.  E.g.:
+#
+#   warning: Could not load shared library symbols for linux-vdso.so.1.
+
 standard_testfile
 
 if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile] } {
     return -1
 }
 
-gdb_breakpoint "main"
+with_test_prefix "setup" {
+    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"
+    # 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
+proc test_no_vdso {command} {
+    global srcfile
+    global gdb_prompt
 
-set test "stop without warning"
-gdb_test_multiple "" $test {
-    -re "Could not load shared library symbols .*\r\n$gdb_prompt $" {
-	fail $test
+    set message "startup"
+    gdb_test_multiple "$command" $message {
+	-re "Could not load shared library symbols .*\r\n$gdb_prompt $" {
+	    fail $message
+	}
+	-re "main \\(\\) at .*$srcfile.*\r\n$gdb_prompt $" {
+	    pass $message
+	}
     }
-    -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
+	}
     }
 }
 
-# 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
+# First, try a live process.
+with_test_prefix "run" {
+    gdb_run_cmd
+    test_no_vdso ""
+}
+
+# Now, dump a core, and reload it.
+with_test_prefix "core" {
+    set corefile [standard_output_file $testfile.core]
+    set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"]
+    if {!$core_supported} {
+	return -1
     }
+
+    clean_restart ${testfile}
+
+    test_no_vdso "core-file $corefile"
 }
-- 
2.5.5