[PATCH v2] gdb/corelow: mark bytes unavailable when reading from unavailable mapping

[simon.marchi@polymtl.ca]

From: Simon Marchi <simon.marchi@efficios.com>

New in v2: adjust some more tests, thanks to the Linaro CI for pointing
this out.

The main motivation for this change is to nicely support "lightweight"
core files on ROCm (more on this below), but I think that the change
also makes sense for regular core files.

When handling a file mappings from a core file, the core target
attempts to open the referenced file.  If successful, the mappings from
this file end up in the m_core_file_mappings vector.  Otherwise, they
end up in the m_core_unavailable_mappings vector.

When trying to read from an address within an unavailable mapping,
unless the executable target beneath is able to fulfill the request, the
core target returns an error (TARGET_XFER_E_IO).  This is from
gdb.base/corefile.exp before the patch:

    (gdb) PASS: gdb.base/corefile.exp: accessing mmapped data in core file with coremmap.data removed
    x/8bd buf2ro
    0x7f095a517000: Cannot access memory at address 0x7f095a517000

I think that this would be a good use case for the "unavailable" status.
We know the memory was there at runtime, it's just not available during
post-mortem debugging.  That is the definition of "unavailable".  After
changing core_target::xfer_partial to report the bytes as unavailable,
which this patch does, the same test now shows:

    (gdb) PASS: gdb.base/corefile.exp: accessing mmapped data in core file with coremmap.data removed
    x/8bd buf2ro
    0x7f0250f52000: <unavailable>   <unavailable>   <unavailable>   <unavailable>   <unavailable>   <unavailable>   <unavailable>   <unavailable>

I would say that the output of the x command isn't great, but that is
just a presentation issue.

The original motivation for me to do this change is that we are working
on lightweight GPU core dump support in ROCm.  By default, the ROC
runtime will dump all the memory allocated in the context of the
crashing wave.  This can result in absurdly big core dumps.  With
lightweight core dumps, the runtime only dumps a certain subset of the
information that is considered essential.  When trying to read a value
from a segment of memory that was not dumped, I believe that it is
natural to use the "unavailable" status.  That is handled by this patch.

In the following example, `d` is a kernel parameter of type `int *`.
Its value was collected in the core dump, but the memory it points to,
allocated with hipMalloc, was not.  Before:

    (gdb) p data
    $1 = (int *) 0x78bf26e00000
    (gdb) p data[5]
    ❌️ Cannot access memory at address 0x78bf26e00014

After:

    (gdb) p data
    $1 = (int *) 0x78bf26e00000
    (gdb) p data[5]
    $2 = <unavailable>

Note that the same concept exists on Linux with the minicoredumper
project [1].  We could adjust the core target to act the same way when
dealing with minicoredumps.

[1] https://www.linutronix.de/minicoredumper/

Change-Id: I4df82ba4116e87545691facec0cb662c4b2b7797
---
 gdb/corelow.c                                         | 3 ++-
 gdb/testsuite/gdb.base/coredump-filter.exp            | 2 +-
 gdb/testsuite/gdb.base/corefile.exp                   | 2 +-
 gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index a28a707c293a..57d05504fa3c 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -1470,7 +1470,8 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 		if (xfer_status == TARGET_XFER_OK)
 		  return TARGET_XFER_OK;
 
-		return TARGET_XFER_E_IO;
+		*xfered_len = len;
+		return TARGET_XFER_UNAVAILABLE;
 	      }
 	  }
 
diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index c35a0334ff33..e2ce86d48e8a 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -62,7 +62,7 @@ proc do_load_and_test_core { core var working_var working_value dump_excluded }
 
     # Access the memory the addresses point to.
     if { $dump_excluded == 0 } {
-	gdb_test "print/x *(char *) $coredump_var_addr($var)" "\(${::valnum_re} = <error: \)?Cannot access memory at address $hex\(>\)?" \
+	gdb_test "print/x *(char *) $coredump_var_addr($var)" "(Cannot access memory at address $hex|${::valnum_re} = <unavailable>)" \
 	    "printing $var when core is loaded (should not work)"
 	gdb_test "print/x *(char *) $coredump_var_addr($working_var)" " = $working_value.*" \
 	    "print/x *$working_var ( = $working_value)"
diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index 957bccf43a40..f6a0a7a1ecad 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -229,7 +229,7 @@ gdb_test "x/8bd buf2" \
     "accessing mmapped data in core file with coremmap.data removed"
 
 gdb_test "x/8bd buf2ro" \
-    "$hex\[^:\]*:\\s+Cannot access memory at address $hex" \
+    "$hex\[^:\]*:(\\s+<unavailable>){8}" \
     "accessing read-only mmapped data in core file with coremmap.data removed"
 
 # Restore the coremmap.data file so later tests don't give warnings
diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
index 784762c2057d..b1934c6b7e4d 100644
--- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
+++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
@@ -144,7 +144,7 @@ proc read_ptr_value { } {
 	-re -wrap "^${::hex}(?:\\s+<\[^>\]+>)?:\\s+($::hex)" {
 	    set value $expect_out(1,string)
 	}
-	-re -wrap "^${::hex}(?:\\s+<\[^>\]+>)?:\\s+Cannot access memory at address ${::hex}" {
+	-re -wrap "^${::hex}(?:\\s+<\[^>\]+>)?:\\s+<unavailable>" {
 	    set value "unavailable"
 	}
     }

base-commit: dd9a411627a249b7b3dfb4ad879c62d862dc93e3
-- 
2.53.0