[1/4] gdb/corefile: remove unavailable target sections
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
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, as well as m_core_unavailable_mappings,
are used by GDB to implement core_target::xfer_partial, that is, to
allow reading from inferior memory.
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 that have SEC_HAS_CONTENTS have their contents copied into
the core file while sections that do not have the SEC_HAS_CONTENTS
flag will not have their contents copied into the core file when it is
created.
The m_core_file_mappings list is created when GDB parses the mapped
file list in the core file. Every mapped file will also be covered by
entries in the m_core_section_table list, however, when parts of a
file are mapped read-only then the corresponding entry in
m_core_section_table will not have the SEC_HAS_CONTENTS flag set. If
parts of a file are mapped read/write then the corresponding
m_core_section_table entry will have the SEC_HAS_CONTENTS flag, and
the corresponding (possibly modified contents) will be copied into the
core file.
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 corresponding
file can't be found then an entry is added to
m_core_unavailable_mappings. An attempt to read from a region covered
by this list will be redirected to the lower file target in the hope
that a read-only section from the executable (supplied by the user)
might satisfy the read.
And so, in core_target::xfer_partial, the order in which the various
lists are checked to satisfy a read are:
1. Check m_core_section_table for sections with SEC_HAS_CONTENTS
flag, this reads content from the core file bfd directly,
2. Check m_core_file_mappings, this will read content from mapped
files assuming that GDB was able to find the file on-disk,
3. Check m_core_unavailable_mappings and redirect to the file
target, this allows the user supplied executable to satisfy some
reads,
4. Check m_core_section_table for sections without SEC_HAS_CONTENTS
flag, this returns zero for these regions.
If a file is recorded as mapped in the core file, but the file was
mapped read only, then the matching entry in m_core_section_table will
not have the SEC_HAS_CONTENTS flag set.
If GDB can find the file on disk then an entry is added to
m_core_file_mappings, and reads from the region covered by the file
will be satisfied by #2 in the list above.
However, if GDB can't find the file then an entry is added to the
m_core_unavailable_mappings list. An attempt will be made to satisfy
the read at stage #3 in the list above, however, if this file is not
the executable, then stage #3 will not satisfy the read.
We then fall through to stage #4, the section does not have the
SEC_HAS_CONTENTS flag, and so we end up returning zero.
In this case, I think this is the wrong thing to do. If the core file
tells us that a file is mapped in, but GDB can't find the file on
disk, then we should instead return a failure when trying to read from
the region covered by the file, anything else is just misleading.
And so, I propose that in core_target::core_target() when the core
file is initially opened and m_core_section_table is setup, we should
remove entries that correspond to m_core_unavailable_mappings entries.
This means that an attempt to read from a mapped file, when GDB is not
able to find the on-disk file, and the contents of the mapped file
were not copied into the core file, then the read will fail. This
feels like the only sensible option.
I've extended an existing test to exercise this case. The test maps a
single file twice, once in read-only mode, and once in read-write
mode. We then remove the on-disk file and load the core file in GDB.
The mapping which was read-write is still accessible (the content of
this region is found in the core file bfd object), while the read-only
mapping is no longer accessible.
---
gdb/corelow.c | 58 +++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/corefile.exp | 39 +++++++++++++++++++
2 files changed, 97 insertions(+)
@@ -166,6 +166,11 @@ class core_target final : public process_stratum_target
ULONGEST len,
ULONGEST *xfered_len);
+ /* Remove from m_core_section_table any entries that are covered by an
+ unavailable mapping. This will prevent xfer_partial from returning
+ zero for unavailable mappings. */
+ void filter_unavailable_mappings_from_section_table ();
+
/* FIXME: kettenis/20031023: Eventually this field should
disappear. */
struct gdbarch *m_core_gdbarch = NULL;
@@ -198,6 +203,59 @@ core_target::core_target ()
m_core_section_table = build_section_table (current_program_space->core_bfd ());
build_file_mappings ();
+
+ if (!m_core_unavailable_mappings.empty ())
+ filter_unavailable_mappings_from_section_table ();
+}
+
+/* See comment in class declaration. */
+
+void
+core_target::filter_unavailable_mappings_from_section_table ()
+{
+ auto filter_cb = [&] (const target_section &tsect) -> bool
+ {
+ /* If TSECT has contents then this means that there is content to be
+ read within the core file bfd itself. If TSECT does not have
+ content then we are relying on reading the content from some other
+ file that was mapped into the inferior at the time the core file was
+ created, but if GDB couldn't find that file then we are not able to
+ read the content and TSECT should be discarded. */
+ if ((tsect.the_bfd_section->flags & SEC_HAS_CONTENTS) != 0)
+ return false;
+
+ /* Find the first entry in m_core_unavailable_mappings whose start
+ address is not less than or equal to the start of this target
+ section. The previous unavailable mapping might cover TSECT. */
+ auto it = std::lower_bound (m_core_unavailable_mappings.begin (),
+ m_core_unavailable_mappings.end (),
+ tsect.addr,
+ [] (const mem_range &mr,
+ const CORE_ADDR &addr)
+ {
+ return mr.start <= addr;
+ });
+
+ /* If there is a previous unavailable mapping, then that mapping's
+ start address is less than, or equal to the start of TSECT. The
+ unavailable mapping might cover TSECT. If it does then we will
+ delete TSECT. */
+ if (it != m_core_unavailable_mappings.begin ())
+ {
+ --it;
+
+ if (it->start <= tsect.addr
+ && (it->start + it->length) >= tsect.endaddr)
+ return true;
+ }
+
+ return false;
+ };
+
+ auto end = std::remove_if (m_core_section_table.begin (),
+ m_core_section_table.end (),
+ filter_cb);
+ m_core_section_table.erase (end, m_core_section_table.end ());
}
/* Construct the table for file-backed mappings if they exist.
@@ -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 {} {