This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 08/19/2016 10:59 AM, Yao Qi wrote:
> 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 was working under the assumption you wanted it in 7.12, given
you set the milestone.  :-)

> 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?

Yeah, the patch ended up larger than I originally thought.  Keeping
it master-only is certainly fine with me.

However, if you want this on 7.12, then there's another option.  We
can start with the hacky version that moves the add_vsyscall_page
call to gdbarch_vsyscall_range, as mentioned in:

> 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.

and then clean this up with the new linux_so_ops, on master, only.

So below's the "ugly" patch.  It's functionally exactly the same as
the larger one.  The switch to linux_so_ops was mainly "cosmetic".

Let me know what you prefer.

>From 1036e36a71623fa84f93a43cad2636ef3dd51da6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 18 Aug 2016 20:16:44 +0100
Subject: [PATCH] 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 and extract the vDSO
  size out of the size of the bfd read in, before we decide to filter
  the vDSO out of the svr4 DSO list.  In order to do that without much
  churn, we hack it in gdbarch_vsyscall_range.
---
 gdb/linux-tdep.c                        | 41 +++++++++++++-----
 gdb/symfile-mem.c                       | 58 ++++++++++++++-----------
 gdb/symfile-mem.h                       | 30 +++++++++++++
 gdb/testsuite/gdb.base/vdso-warning.exp | 76 ++++++++++++++++++++++-----------
 4 files changed, 145 insertions(+), 60 deletions(-)
 create mode 100644 gdb/symfile-mem.h

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ab110b0..2094ee9 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -38,6 +38,7 @@
 #include "gdbcmd.h"
 #include "gdb_regex.h"
 #include "common/enum-flags.h"
+#include "symfile-mem.h"
 
 #include <ctype.h>
 
@@ -185,7 +186,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
@@ -196,6 +197,9 @@ struct linux_info
      yet.  Positive if we tried looking it up, and found it.  Negative
      if we tried looking it up but failed.  */
   int vsyscall_range_p;
+
+  /* True if we've already called add_vsyscall_page.  */
+  int vdso_syms_added_p;
 };
 
 /* Frees whatever allocated space there is to be freed and sets INF's
@@ -2278,7 +2282,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)
@@ -2287,16 +2292,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;
 
@@ -2330,8 +2340,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;
 	    }
 	}
 
@@ -2340,7 +2349,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;
 }
 
 /* Implementation of the "vsyscall_range" gdbarch hook.  Handles
@@ -2362,6 +2371,16 @@ linux_vsyscall_range (struct gdbarch *gdbarch, struct mem_range *range)
   if (info->vsyscall_range_p < 0)
     return 0;
 
+  /* If we couldn't figure out the vDSO size from the mappings, then
+     open it using BFD in order to figure out the size from the ELF
+     structures.  Need to do this before svr4_current_sos need the
+     range info in order to filter out the vDSO.  */
+  if (!info->vdso_syms_added_p)
+    {
+      info->vdso_syms_added_p = 1;
+      add_vsyscall_page (&info->vsyscall_range, SYMFILE_DEFER_BP_RESET);
+    }
+
   *range = info->vsyscall_range;
   return 1;
 }
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 79739a6..3a5ce3a 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -82,7 +82,7 @@ target_read_memory_bfd (bfd_vma memaddr, bfd_byte *myaddr, bfd_size_type len)
    which will be attached to the BFD.  */
 static struct objfile *
 symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
-			     size_t size, char *name, int from_tty)
+			     size_t size, char *name, int add_flags)
 {
   struct objfile *objf;
   struct bfd *nbfd;
@@ -127,8 +127,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
   sai->num_sections = i;
 
   objf = symbol_file_add_from_bfd (nbfd, bfd_get_filename (nbfd),
-				   from_tty ? SYMFILE_VERBOSE : 0,
-                                   sai, OBJF_SHARED, NULL);
+				   add_flags, sai, OBJF_SHARED, NULL);
 
   add_target_sections_of_objfile (objf);
 
@@ -145,6 +144,7 @@ add_symbol_file_from_memory_command (char *args, int from_tty)
 {
   CORE_ADDR addr;
   struct bfd *templ;
+  int add_flags = from_tty ? SYMFILE_VERBOSE : 0;
 
   if (args == NULL)
     error (_("add-symbol-file-from-memory requires an expression argument"));
@@ -160,18 +160,22 @@ add_symbol_file_from_memory_command (char *args, int from_tty)
     error (_("Must use symbol-file or exec-file "
 	     "before add-symbol-file-from-memory."));
 
-  symbol_file_add_from_memory (templ, addr, 0, NULL, from_tty);
+  symbol_file_add_from_memory (templ, addr, 0, NULL, add_flags);
 }
 
 /* Arguments for symbol_file_add_from_memory_wrapper.  */
 
 struct symbol_file_add_from_memory_args
 {
+  /* Inputs.  */
   struct bfd *bfd;
   CORE_ADDR sysinfo_ehdr;
   size_t size;
   char *name;
-  int from_tty;
+  int add_flags;
+
+  /* Outputs.  */
+  struct objfile *objf_added;
 };
 
 /* Wrapper function for symbol_file_add_from_memory, for
@@ -183,23 +187,22 @@ 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->from_tty);
+  args->objf_added = symbol_file_add_from_memory (args->bfd,
+						  args->sysinfo_ehdr,
+						  args->size, args->name,
+						  args->add_flags);
   return 0;
 }
 
-/* Try to add the symbols for the vsyscall page, if there is one.
-   This function is called via the inferior_created observer.  */
+/* See symfile-mem.h.  */
 
-static void
-add_vsyscall_page (struct target_ops *target, int from_tty)
+void
+add_vsyscall_page (struct mem_range *vsyscall_range, int add_flags)
 {
-  struct mem_range vsyscall_range;
-
-  if (gdbarch_vsyscall_range (target_gdbarch (), &vsyscall_range))
     {
       struct bfd *bfd;
       struct symbol_file_add_from_memory_args args;
+      struct stat statbuf;
 
       if (core_bfd != NULL)
 	bfd = core_bfd;
@@ -218,17 +221,26 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	  return;
 	}
       args.bfd = bfd;
-      args.sysinfo_ehdr = vsyscall_range.start;
-      args.size = vsyscall_range.length;
+      args.sysinfo_ehdr = vsyscall_range->start;
+      args.size = vsyscall_range->length;
 
       args.name = xstrprintf ("system-supplied DSO at %s",
-			      paddress (target_gdbarch (), vsyscall_range.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.  */
-      args.from_tty = 0;
+			      paddress (target_gdbarch (), vsyscall_range->start));
+      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 end address of the vDSO pages, 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;
+	}
     }
 }
 
@@ -247,8 +259,4 @@ _initialize_symfile_mem (void)
 	     "Give an expression for the address "
 	     "of the file's shared object file header."),
            &cmdlist);
-
-  /* Want to know of each new inferior so that its vsyscall info can
-     be extracted.  */
-  observer_attach_inferior_created (add_vsyscall_page);
 }
diff --git a/gdb/symfile-mem.h b/gdb/symfile-mem.h
new file mode 100644
index 0000000..df0e879
--- /dev/null
+++ b/gdb/symfile-mem.h
@@ -0,0 +1,30 @@
+/* Definitions for reading symbol files from memory into GDB.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#if !defined (SYMFILE_MEM_H)
+#define SYMFILE_MEM_H
+
+struct mem_range;
+
+/* Try to add the symbols for the vsyscall page, if there is one.  */
+
+extern void add_vsyscall_page (struct mem_range *vsyscall_range,
+			       int add_flags);
+
+#endif /* !defined(SYMFILE_MEM_H) */
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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]