Work around incorrect/broken pathnames in NT_FILE note

Luis Machado reported some regressions after I pushed recent core file
related patches fixing BZ 25631:

    FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
    FAIL: gdb.base/corefile.exp: core-file warning-free
    FAIL: gdb.base/corefile.exp: print func2::coremaker_local
    FAIL: gdb.base/corefile.exp: up in corefile.exp
    FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)

This commit fixes these regressions.  Thanks to Luis for testing
an earlier version of the patch.  (I was unable to reproduce these
regressions in various test environments that I created.)

Luis is testing in a docker container which is using the AUFS storage
driver.  It turns out that the kernel is placing docker host paths in
the NT_FILE note instead of paths within the container.

I've made a similar docker environment (though apparently not similar
enough to reproduce the regressions).  This is one of the paths that
I see mentioned in the warning messages printed while loading the
core file during NT_FILE note processing - note that I've shortened
the path component starting with "d07c4":

/var/lib/docker/aufs/diff/d07c4...21/lib/x86_64-linux-gnu/ld-2.27.so

This is a path on the docker host; it does not exist in the
container.  In the docker container, this is the path:

/lib/x86_64-linux-gnu/ld-2.27.so

My first thought was to disable all NT_FILE mappings when any path was
found to be bad.  This would have caused GDB to fall back to accessing
memory using the file stratum as it did before I added the NT_FILE
note loading code.  After further consideration, I realized that we
could do better than this.  For file-backed memory access, we can
still use the NT_FILE mappings when available, and then attempt to
access memory using the file stratum constrained to those address
ranges corresponding to the "broken" mappings.

In order to test it, I made some additions to corefile2.exp in which
the test case's executable is renamed.  The core file is then loaded;
due to the fact that the executable has been renamed, those mappings
will be unavailable.  After loading the core file, the executable is
renamed back to its original name at which point it is loaded using
GDB's "file" command.  The "interesting" tests are then run.  These
tests will print out values in file-backed memory regions along with
mmap'd regions placed within/over the file-backed regions.  Despite
the fact that the executable could not be found during the NT_FILE
note processing, these tests still work correctly due to the fact that
memory is available from the file stratum combined with the fact that
the broken NT_FILE mappings are used to prevent file-backed access
outside of the "broken" mappings.

gdb/ChangeLog:

	* corelow.c (unordered_set): Include.
	(class core_target): Add field 'm_core_unavailable_mappings'.
	(core_target::build_file_mappings): Print only one warning
	per inaccessible file.  Add unavailable/broken mappings
	to m_core_unavailable_mappings.
	(core_target::xfer_partial): Call...
	(core_target::xfer_memory_via_mappings): New method.

gdb/testsuite/ChangeLog:

	* gdb.base/corefile2.exp (renamed binfile): New tests.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7aadd870423ab6cf648092d88034500fa97c0d74..4840de8591a4299ef8f3c5563dec7b9fd5850b43 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2020-08-31  Kevin Buettner  <kevinb@redhat.com>
+
+       * corelow.c (unordered_set): Include.
+       (class core_target): Add field 'm_core_unavailable_mappings'.
+       (core_target::build_file_mappings): Print only one warning
+       per inaccessible file.  Add unavailable/broken mappings
+       to m_core_unavailable_mappings.
+       (core_target::xfer_partial): Call...
+       (core_target::xfer_memory_via_mappings): New method.
+
 2020-08-31  Simon Marchi  <simon.marchi@polymtl.ca>
 
        * dwarf2/read.c (struct field_info) <non_public_fields>: Change

diff --git a/gdb/corelow.c b/gdb/corelow.c
index b6ee219f57c293eed300c790d76fd8043e209bd5..96ec739c62fb4fc33e91bdd66274250b0ae614e5 100644 (file)
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -47,6 +47,7 @@
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
 #include <unordered_map>
+#include <unordered_set>
 #include "gdbcmd.h"
 
 #ifndef O_LARGEFILE
@@ -131,9 +132,21 @@ private: /* per-core data */
      information about memory mapped files.  */
   target_section_table m_core_file_mappings {};
 
+  /* Unavailable mappings.  These correspond to pathnames which either
+     weren't found or could not be opened.  Knowing these addresses can
+     still be useful.  */
+  std::vector<mem_range> m_core_unavailable_mappings;
+
   /* Build m_core_file_mappings.  Called from the constructor.  */
   void build_file_mappings ();
 
+  /* Helper method for xfer_partial.  */
+  enum target_xfer_status xfer_memory_via_mappings (gdb_byte *readbuf,
+                                                   const gdb_byte *writebuf,
+                                                   ULONGEST offset,
+                                                   ULONGEST len,
+                                                   ULONGEST *xfered_len);
+
   /* FIXME: kettenis/20031023: Eventually this field should
      disappear.  */
   struct gdbarch *m_core_gdbarch = NULL;
@@ -182,6 +195,7 @@ void
 core_target::build_file_mappings ()
 {
   std::unordered_map<std::string, struct bfd *> bfd_map;
+  std::unordered_set<std::string> unavailable_paths;
 
   /* See linux_read_core_file_mappings() in linux-tdep.c for an example
      read_core_file_mappings method.  */
@@ -216,9 +230,12 @@ core_target::build_file_mappings ()
              = exec_file_find (filename, NULL);
            if (expanded_fname == nullptr)
              {
-               warning (_("Can't open file %s during file-backed mapping "
-                          "note processing"),
-                        filename);
+               m_core_unavailable_mappings.emplace_back (start, end - start);
+               /* Print just one warning per path.  */
+               if (unavailable_paths.insert (filename).second)
+                 warning (_("Can't open file %s during file-backed mapping "
+                            "note processing"),
+                          filename);
                return;
              }
 
@@ -227,6 +244,7 @@ core_target::build_file_mappings ()
 
            if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
              {
+               m_core_unavailable_mappings.emplace_back (start, end - start);
                /* If we get here, there's a good chance that it's due to
                   an internal error.  We issue a warning instead of an
                   internal error because of the possibility that the
@@ -268,6 +286,8 @@ core_target::build_file_mappings ()
        ts->owner = nullptr;
        ts->the_bfd_section = sec;
       });
+
+  normalize_mem_ranges (&m_core_unavailable_mappings);
 }
 
 static void add_to_thread_list (bfd *, asection *, void *);
@@ -728,6 +748,57 @@ core_target::files_info ()
   print_section_info (&m_core_section_table, core_bfd);
 }
 \f
+/* Helper method for core_target::xfer_partial.  */
+
+enum target_xfer_status
+core_target::xfer_memory_via_mappings (gdb_byte *readbuf,
+                                      const gdb_byte *writebuf,
+                                      ULONGEST offset, ULONGEST len,
+                                      ULONGEST *xfered_len)
+{
+  enum target_xfer_status xfer_status;
+
+  xfer_status = (section_table_xfer_memory_partial
+                  (readbuf, writebuf,
+                   offset, len, xfered_len,
+                   m_core_file_mappings.sections,
+                   m_core_file_mappings.sections_end));
+
+  if (xfer_status == TARGET_XFER_OK || m_core_unavailable_mappings.empty ())
+    return xfer_status;
+
+  /* There are instances - e.g. when debugging within a docker
+     container using the AUFS storage driver - where the pathnames
+     obtained from the note section are incorrect.  Despite the path
+     being wrong, just knowing the start and end addresses of the
+     mappings is still useful; we can attempt an access of the file
+     stratum constrained to the address ranges corresponding to the
+     unavailable mappings.  */
+
+  ULONGEST memaddr = offset;
+  ULONGEST memend = offset + len;
+
+  for (const auto &mr : m_core_unavailable_mappings)
+    {
+      if (address_in_mem_range (memaddr, &mr))
+        {
+         if (!address_in_mem_range (memend, &mr))
+           len = mr.start + mr.length - memaddr;
+
+         xfer_status = this->beneath ()->xfer_partial (TARGET_OBJECT_MEMORY,
+                                                       NULL,
+                                                       readbuf,
+                                                       writebuf,
+                                                       offset,
+                                                       len,
+                                                       xfered_len);
+         break;
+       }
+    }
+
+  return xfer_status;
+}
+
 enum target_xfer_status
 core_target::xfer_partial (enum target_object object, const char *annex,
                           gdb_byte *readbuf, const gdb_byte *writebuf,
@@ -761,11 +832,8 @@ core_target::xfer_partial (enum target_object object, const char *annex,
           result.  If not, check the stratum beneath us, which should
           be the file stratum.  */
        if (m_core_file_mappings.sections != nullptr)
-         xfer_status = section_table_xfer_memory_partial
-                         (readbuf, writebuf,
-                          offset, len, xfered_len,
-                          m_core_file_mappings.sections,
-                          m_core_file_mappings.sections_end);
+         xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
+                                                 len, xfered_len);
        else
          xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
                                                        writebuf, offset, len,

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 148f31d26dd6558f9f9432107de4be7d8f1be1a7..e50b2352d9ff2413c6f1447261dd410c14cb38f3 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-08-31  Kevin Buettner  <kevinb@redhat.com>
+
+       * gdb.base/corefile2.exp (renamed binfile): New tests.
+
 2020-08-31  Simon Marchi  <simon.marchi@efficios.com>
 
        * gdb.dwarf2/dw2-reg-undefined.exp: Remove spurious #.

diff --git a/gdb/testsuite/gdb.base/corefile2.exp b/gdb/testsuite/gdb.base/corefile2.exp
index 5de7ead4d44c7d88d1d3cd81edfa46279259b7b3..38b8eb770bdf83c809a94da8da09b48c06f547e1 100644 (file)
--- a/gdb/testsuite/gdb.base/corefile2.exp
+++ b/gdb/testsuite/gdb.base/corefile2.exp
@@ -143,6 +143,33 @@ gdb_test_multiple $test "" {
     }
 }
 
+# Test again with executable renamed during loading of core file.
+
+with_test_prefix "renamed binfile" {
+    # Don't load $binfile in this call to clean_restart.  (BFD will
+    # complain that $binfile has disappeared after the rename if it's
+    # loaded first.)
+    clean_restart
+
+    # Rename $binfile so that it won't be found during loading of
+    # the core file.
+    set hide_binfile [standard_output_file "${testfile}.hide"]
+    remote_exec host "mv -f $binfile $hide_binfile"
+
+    # Load core file - check that a warning is printed.
+    global xfail
+    if { $xfail } { setup_xfail "*-*-*" }
+    gdb_test "core-file $corefile" \
+       "warning: Can't open file.*during.* note processing.*Core was generated by .*\#0  .*\(\)" \
+       "load core file without having first loaded binfile"
+
+    # Restore $binfile and then load it.
+    remote_exec host "mv -f $hide_binfile $binfile"
+    gdb_load ${binfile}
+
+    do_tests
+}
+
 # Restart and run to the abort call.
 
 clean_restart $binfile