Make the dcache (code/stack cache) handle line reading errors better
authorPedro Alves <palves@redhat.com>
Wed, 21 May 2014 12:58:16 +0000 (13:58 +0100)
committerPedro Alves <palves@redhat.com>
Wed, 21 May 2014 12:58:16 +0000 (13:58 +0100)
The dcache (code/stack cache) is supposed to be transparent, but it's
actually not in one case.  dcache tries to read chunks (cache lines)
at a time off of the target.  This may end up trying to read
unaccessible or unavailable memory.  Currently the caller gets an xfer
error in this case.  But if the specific bits of memory the caller
actually wanted are available and accessible, then the caller should
get the memory it wanted, not an error.

gdb/
2014-05-21  Pedro Alves  <palves@redhat.com>

* dcache.c (dcache_read_memory_partial): If reading the cache line
fails, fallback to reading just the memory the caller wanted.

gdb/testsuite/
2014-05-21  Pedro Alves  <palves@redhat.com>

* gdb.base/dcache-line-read-error.c: New.
* gdb.base/dcache-line-read-error.exp: New.

gdb/ChangeLog
gdb/dcache.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/dcache-line-read-error.c [new file with mode: 0644]
gdb/testsuite/gdb.base/dcache-line-read-error.exp [new file with mode: 0644]

index 0c87bfff3c5981284d845354ee88cae8fb1866e1..6fe325de7eb98309daa8f1b92ccd7ee53b4d01f7 100644 (file)
@@ -1,3 +1,8 @@
+2014-05-21  Pedro Alves  <palves@redhat.com>
+
+       * dcache.c (dcache_read_memory_partial): If reading the cache line
+       fails, fallback to reading just the memory the caller wanted.
+
 2014-05-20  Doug Evans  <dje@google.com>
 
        * python/py-progspace.c (py_free_pspace): Call target_gdbarch
index d3b546b5bb571720419269930cfa40b761ac06a0..9780f4dd81b73092ff4196749da42d0ed6c4f98f 100644 (file)
@@ -497,8 +497,11 @@ dcache_read_memory_partial (struct target_ops *ops, DCACHE *dcache,
 
   if (i == 0)
     {
-      /* FIXME: We lose the real error status.  */
-      return TARGET_XFER_E_IO;
+      /* Even though reading the whole line failed, we may be able to
+        read a piece starting where the caller wanted.  */
+      return ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+                                  myaddr, NULL, memaddr, len,
+                                  xfered_len);
     }
   else
     {
index be0bdf3c3102f373afef07d839634e55f20037af..1e39142e688836cd40f1ec262aef750aa6863e3b 100644 (file)
@@ -1,3 +1,8 @@
+2014-05-21  Pedro Alves  <palves@redhat.com>
+
+       * gdb.base/dcache-line-read-error.c: New.
+       * gdb.base/dcache-line-read-error.exp: New.
+
 2014-05-20  Pedro Alves  <palves@redhat.com>
 
        * gdb.base/compare-sections.c: New file.
diff --git a/gdb/testsuite/gdb.base/dcache-line-read-error.c b/gdb/testsuite/gdb.base/dcache-line-read-error.c
new file mode 100644 (file)
index 0000000..4120593
--- /dev/null
@@ -0,0 +1,107 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <string.h>
+
+size_t pg_size;
+void *first_mapped_page;
+void *last_mapped_page;
+
+void
+breakpt (void)
+{
+  /* Nothing. */
+}
+
+int
+main (void)
+{
+  void *p;
+  int pg_count;
+  size_t i;
+
+  /* Map 6 contiguous pages, and then unmap all second first, and
+     second last.
+
+     From GDB we will disassemble each of the _mapped_ pages, with a
+     code-cache (dcache) line size bigger than the page size (twice
+     bigger).  This makes GDB try to read one page before the mapped
+     page once, and the page after another time.  GDB should give no
+     error in either case.
+
+     That is, depending on where the kernel aligns the pages, we get
+     either:
+
+      .---.---.---.---.---.---.
+      | U | M | U | U | M | U |
+      '---'---'---'---'---'---.
+      |       |       |       |  <- line alignment
+       ^^^^^^^         ^^^^^^^
+          |               |
+          + line1         + line2
+
+     Or:
+
+      .---.---.---.---.---.---.
+      | U | M | U | U | M | U |
+      '---'---'---'---'---'---.
+          |       |       |      <- line alignment
+           ^^^^^^^ ^^^^^^^
+              |       |
+        line1 +       + line2
+
+    Note we really want to test that dcache behaves correctly when
+    reading a cache line fails.  We're just using unmapped memory as
+    proxy for any kind of error.  */
+
+  pg_size = getpagesize ();
+  pg_count = 6;
+
+  p = mmap (0, pg_count * pg_size, PROT_READ|PROT_WRITE,
+           MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+  if (p == MAP_FAILED)
+    {
+      perror ("mmap");
+      return EXIT_FAILURE;
+    }
+
+  /* Leave memory zero-initialized.  Disassembling 0s should behave on
+     all targets.  */
+
+  for (i = 0; i < pg_count; i++)
+    {
+      if (i == 1 || i == 4)
+       continue;
+
+      if (munmap (p + (i * pg_size), pg_size) == -1)
+       {
+         perror ("munmap");
+         return EXIT_FAILURE;
+       }
+    }
+
+  first_mapped_page = p + 1 * pg_size;;
+  last_mapped_page = p + 4 * pg_size;
+
+  breakpt ();
+
+  return EXIT_SUCCESS;
+}
diff --git a/gdb/testsuite/gdb.base/dcache-line-read-error.exp b/gdb/testsuite/gdb.base/dcache-line-read-error.exp
new file mode 100644 (file)
index 0000000..7e75460
--- /dev/null
@@ -0,0 +1,68 @@
+# 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/>.
+
+# Test that dcache behaves correctly when reading a cache line fails.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile}] } {
+    return -1
+}
+
+if ![runto breakpt] {
+    return -1
+}
+
+# Issue the "delete mem" command.  This makes GDB ignore the
+# target-provided list, if any.
+
+proc delete_mem {} {
+    global gdb_prompt
+
+    set test "delete mem"
+    gdb_test_multiple $test $test {
+       -re "Delete all memory regions.*y or n.*$" {
+           send_gdb "y\n"
+           exp_continue
+       }
+       -re "$gdb_prompt $" {
+           pass $test
+       }
+    }
+}
+
+# Make the dcache line size bigger than the pagesize.
+set pagesize [get_integer_valueof "pg_size" -1]
+set linesize [expr $pagesize * 2]
+
+gdb_test_no_output "set dcache line-size $linesize" \
+    "set dcache line size to twice the pagesize"
+
+gdb_test "info dcache" \
+    "Dcache 4096 lines of $linesize bytes each.\r\nNo data cache available."
+
+# Make sure dcache doesn't automatically skip unmapped regions.
+delete_mem
+
+gdb_test "info mem" \
+    "Using user-defined memory regions.\r\nThere are no memory regions defined\."
+
+# Given the line size is bigger than the page size, we have
+# alternating mapped and unmapped pages, these make dcache fail to
+# fill in the cache line.  GDB used to have a bug where that failure
+# would end up as user-visible error.  The range being disassembled is
+# wholly available, so GDB should succeed.
+gdb_test "disassemble first_mapped_page, +10" "End of assembler dump\."
+gdb_test "disassemble last_mapped_page, +10" "End of assembler dump\."