[binutils-gdb] gdb/corefile: don't pretend unavailable sections are readable
Andrew Burgess
aburgess@sourceware.org
Sat Sep 7 19:54:13 GMT 2024
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=522875f679bd0a5c31cc02a57a7a2149193dea1d
commit 522875f679bd0a5c31cc02a57a7a2149193dea1d
Author: Andrew Burgess <aburgess@redhat.com>
Date: Wed May 15 15:28:45 2024 +0100
gdb/corefile: don't pretend unavailable sections are readable
When GDB opens a core file the bfd library processes the core file and
creates sections within the bfd object to represent each of the
segments within the core file.
GDB then creates two target_section lists, m_core_section_table and
m_core_file_mappings, these, along with m_core_unavailable_mappings,
are used by GDB to implement core_target::xfer_partial; this is the
function used when GDB tries to read memory from a core file inferior.
The m_core_section_table list represents sections within the core file
itself. The sections in this list can be split into two groups based
on whether the section has the SEC_HAS_CONTENTS flag set or not.
Sections (from the core file) that have the SEC_HAS_CONTENTS flag had
their contents copied into the core file when the core file was
created. These correspond to writable sections within the original
inferior (the inferior for which the core file was created).
Sections (from the core file) that do not have the SEC_HAS_CONTENTS
flag will not have had their contents copied into the core file when
it was created. These sections correspond to read-only sections
mapped from a file (possibly the initial executable, or possibly some
other file) in the original inferior. The expectation is that the
contents of these sections can still be found by looking in the file
that was originally mapped.
The m_core_file_mappings list is created when GDB parses the mapped
file list in the core file. Every mapped region will be covered by
entries in the m_core_section_table list (see above), but for
read-only mappings the entry in m_core_section_table will not have the
SEC_HAS_CONTENTS flag set. As GDB parses the mapped file list, if the
file that was originally mapped can be found, then GDB creates an
entry in the m_core_file_mappings list which represents the region
of the file that was mapped into the original inferior.
However, GDB only creates entries in m_core_file_mappings if it is
able to find the correct on-disk file to open. If the file can't be
found then an entry is added to m_core_unavailable_mappings instead.
If is the handling m_core_unavailable_mappings which I think is
currently not completely correct.
When a read lands within an m_core_unavailable_mappings region we
currently forward the read to the exec file stratum. The reason for
this is this: when GDB read the mapped file list, if the executable
file could not be found at the expected path then mappings within the
executable will end up in the m_core_unavailable_mappings list.
However, the user might provide the executable to GDB from a different
location. If this happens then forwarding the read to the exec file
stratum might give a result.
But, if the exec file stratum does not resolve the access then
currently we continue through ::xfer_partial, the next step of which
is to handle m_core_section_table entries that don't have the
SEC_HAS_CONTENTS flag set. Every m_core_unavailable_mappings entry
will naturally have an m_core_section_table without the
SEC_HAS_CONTENTS flag set, and so we treat the unavailable mapping as
zero initialised memory and return all zeros.
It is this fall through behaviour that I think is wrong. If a read
falls in an unavailable region, and the exec file stratum cannot help,
then I think the access should fail.
To achieve this goal I have removed the xfer_memory_via_mappings
helper function and moved its content inline into ::xfer_partial.
Now, if an access is within an m_core_unavailable_mappings region, and
the exec file stratum doesn't help, we immediately return with an
error.
The reset of ::xfer_partial is unchanged, I've extended some comments
in the area that I have changed to (I hope) explain better what's
going on.
There's a new test that covers the new functionality, an inferior maps
a file and generates a core file. We then remove the mapped file,
load the core file and try to read from the mapped region. The
expectation is that GDB should give an error rather than claiming that
the region is full of zeros.
Diff:
---
gdb/corelow.c | 134 +++++++++++++++++-------------------
gdb/testsuite/gdb.base/corefile.exp | 39 +++++++++++
2 files changed, 101 insertions(+), 72 deletions(-)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 2b7a3550fc1..178f9e27898 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -160,13 +160,6 @@ class core_target final : public process_stratum_target
/* 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;
@@ -964,55 +957,6 @@ core_target::files_info ()
print_section_info (&m_core_section_table, current_program_space->core_bfd ());
}
-/* 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));
-
- 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 (mr.contains (memaddr))
- {
- if (!mr.contains (memend))
- 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,
@@ -1040,26 +984,72 @@ core_target::xfer_partial (enum target_object object, const char *annex,
if (xfer_status == TARGET_XFER_OK)
return TARGET_XFER_OK;
- /* Check file backed mappings. If they're available, use
- core file provided mappings (e.g. from .note.linuxcore.file
- or the like) as this should provide a more accurate
- result. If not, check the stratum beneath us, which should
- be the file stratum.
+ /* Check file backed mappings. If they're available, use core file
+ provided mappings (e.g. from .note.linuxcore.file or the like)
+ as this should provide a more accurate result. */
+ if (!m_core_file_mappings.empty ())
+ {
+ xfer_status = section_table_xfer_memory_partial
+ (readbuf, writebuf, offset, len, xfered_len,
+ m_core_file_mappings);
+ if (xfer_status == TARGET_XFER_OK)
+ return xfer_status;
+ }
- We also check unavailable mappings due to Docker/AUFS driver
- issues. */
- if (!m_core_file_mappings.empty ()
- || !m_core_unavailable_mappings.empty ())
+ /* If the access is within an unavailable file mapping then we try
+ to check in the stratum below (the executable stratum). The
+ thinking here is that if the mapping was read/write then the
+ contents would have been written into the core file and the
+ access would have been satisfied by m_core_section_table.
+
+ But if the access has not yet been resolved then we can assume
+ the access is read-only. If the executable was not found
+ during the mapped file check then we'll have an unavailable
+ mapping entry, however, if the user has provided the executable
+ (maybe in a different location) then we might be able to
+ resolve the access from there.
+
+ If that fails, but the access is within an unavailable region,
+ then the access itself should fail. */
+ for (const auto &mr : m_core_unavailable_mappings)
{
- xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
+ if (mr.contains (offset))
+ {
+ if (!mr.contains (offset + len))
+ len = mr.start + mr.length - offset;
+
+ xfer_status
+ = this->beneath ()->xfer_partial (TARGET_OBJECT_MEMORY,
+ nullptr, readbuf,
+ writebuf, offset,
len, xfered_len);
+ if (xfer_status == TARGET_XFER_OK)
+ return TARGET_XFER_OK;
+
+ return TARGET_XFER_E_IO;
+ }
+ }
+
+ /* The following is acting as a fallback in case we encounter a
+ situation where the core file is lacking and mapped file
+ information. Here we query the exec file stratum to see if it
+ can resolve the access. Doing this when we are missing mapped
+ file information might be the best we can do, but there are
+ certainly cases this will get wrong, e.g. if an inferior created
+ a zero initialised mapping over the top of some data that exists
+ within the executable then this will return the executable data
+ rather than the zero data. Maybe we should just drop this
+ block? */
+ if (m_core_file_mappings.empty ()
+ && m_core_unavailable_mappings.empty ())
+ {
+ xfer_status
+ = this->beneath ()->xfer_partial (object, annex, readbuf,
+ writebuf, offset, len,
+ xfered_len);
+ if (xfer_status == TARGET_XFER_OK)
+ return TARGET_XFER_OK;
}
- else
- xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
- writebuf, offset, len,
- xfered_len);
- if (xfer_status == TARGET_XFER_OK)
- return TARGET_XFER_OK;
/* Finally, attempt to access data in core file sections with
no contents. These will typically read as all zero. */
diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index 79363c5db53..f4f102a156b 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -199,6 +199,45 @@ gdb_test "up" "#\[0-9\]* *(\[0-9xa-fH'\]* in)? .* \\(.*\\).*" "up, reinit"
gdb_test "core" "No core file now."
+# Temporarily move coremmap.data out of the way and reload the core
+# file. We should still be able to read buf2 as the contents of this
+# are written into the core file. In contrast buf2ro should no longer
+# be readable as the contents of this region are not within the core
+# file, GDB relies on reading this from the coremmap.data file, which
+# can no longer be found.
+set coremmap_data_filename \
+ [standard_output_file coredir.[getpid]/coremmap.data]
+set coremmap_data_backup_filename \
+ [standard_output_file coredir.[getpid]/coremmap.data.backup]
+remote_exec host "mv ${coremmap_data_filename} \
+ ${coremmap_data_backup_filename}"
+
+clean_restart $binfile
+
+# Load the core file and check we get a warning about the
+# coremmap.data file being missing.
+gdb_test_multiple "core-file $corefile" "warn about coremmap.data missing" {
+ -re -wrap "warning: Can't open file \[^\r\n\]+/coremmap.data during file-backed mapping note processing\r\n.*" {
+ pass $gdb_test_name
+ }
+}
+
+# This xfail was just copied from earlier in the script where we also
+# read from buf2.
+setup_xfail "*-*-sunos*" "*-*-aix*"
+gdb_test "x/8bd buf2" \
+ ".*:.*0.*1.*2.*3.*4.*5.*6.*7.*" \
+ "accessing mmapped data in core file with coremmap.data removed"
+
+gdb_test "x/8bd buf2ro" \
+ "$hex\[^:\]*:\\s+Cannot access memory at address $hex" \
+ "accessing read-only mmapped data in core file with coremmap.data removed"
+
+# Restore the coremmap.data file so later tests don't give warnings
+# when the core file is reloaded.
+remote_exec host "mv ${coremmap_data_backup_filename} \
+ ${coremmap_data_filename}"
+
# Test that we can unload the core with the "detach" command.
proc_with_prefix corefile_detach {} {
More information about the Gdb-cvs
mailing list