[PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Kevin Buettner]

This series fixes several core file related bugs.  The bug which
started this work can be viewed here:

    https://sourceware.org/bugzilla/show_bug.cgi?id=25631

Other problems were found either during review or during development. 
I discuss these in my commit log remarks.

This v5 series has only minor changes from v4.  Unless noted
otherwise, these changes all address Pedro's concerns:

o Patch #5, Test ability to access unwritten-to mmap data in core file:

  - Add setup_xfail for non-Linux OSes.  (I spotted this while testing
    on FreeBSD.)

o Patch #8, Use NT_FILE note section for reading core target memory:

  - Formatting fixes.
  - Close bfd after failed bfd_check_format_check.
  - Fix SECNAME leak.

o Patch #11, Adjust coredump-filter.exp to account for NT_FILE note handling:

  - Instead of XFAILing test, use Mihails Strasuns's suggestion
    instead.

o Patch #12, Add new command "maint print core-file-backed-mappings":

  - Formatting fixes.
  - Don't crash GDB with gdb_assert().
  - Add comments (in both code and commit log) regarding utility
    of this new command.

o Patch #14, New core file tests with mappings over existing program memory:

  - Formatting fixes.
  - Add another "maint print core-file-backed-mappings" test requested
    by Pedro in patch #12 review.  This one makes sure that we don't
    crash GDB when NOT debugging a core file.

I believe that all patches not listed above have been approved. 
Actually, I think that #5 is still approved too; I doubt that adding
an XFAIL changes that.

Kevin Buettner (14):
  Remove hack for GDB which sets the section size to 0
  Adjust corefile.exp test to show regression after bfd hack removal
  section_table_xfer_memory: Replace section name with callback
    predicate
  Provide access to non SEC_HAS_CONTENTS core file sections
  Test ability to access unwritten-to mmap data in core file
  Update binary_get_section_contents to seek using section's file
    position
  Add new gdbarch method, read_core_file_mappings
  Use NT_FILE note section for reading core target memory
  Add test for accessing read-only mmapped data in a core file
  gcore command: Place all file-backed mappings in NT_FILE note
  Adjust coredump-filter.exp to account for NT_FILE note handling
  Add new command "maint print core-file-backed-mappings"
  Add documentation for "maint print core-file-backed-mappings"
  New core file tests with mappings over existing program memory

 bfd/binary.c                               |  12 +-
 bfd/elf.c                                  |   8 -
 gdb/NEWS                                   |   4 +
 gdb/arch-utils.c                           |  16 ++
 gdb/arch-utils.h                           |  12 +
 gdb/bfd-target.c                           |   3 +-
 gdb/corelow.c                              | 264 ++++++++++++++++++++-
 gdb/doc/gdb.texinfo                        |   8 +
 gdb/exec.c                                 |   8 +-
 gdb/exec.h                                 |  13 +-
 gdb/gdbarch.c                              |  23 ++
 gdb/gdbarch.h                              |   6 +
 gdb/gdbarch.sh                             |   3 +
 gdb/linux-tdep.c                           | 244 +++++++++++++------
 gdb/target.c                               |  18 +-
 gdb/testsuite/gdb.base/coredump-filter.exp |  18 +-
 gdb/testsuite/gdb.base/corefile.exp        |  27 ++-
 gdb/testsuite/gdb.base/corefile2.exp       | 185 +++++++++++++++
 gdb/testsuite/gdb.base/coremaker.c         |  30 ++-
 gdb/testsuite/gdb.base/coremaker2.c        | 150 ++++++++++++
 20 files changed, 942 insertions(+), 110 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/corefile2.exp
 create mode 100644 gdb/testsuite/gdb.base/coremaker2.c

-- 
2.26.2

[PATCH v5 01/14] Remove hack for GDB which sets the section size to 0

[Kevin Buettner]

This commit removes a hack for GDB which was introduced in 2007.
See:

    https://sourceware.org/ml/binutils/2007-08/msg00044.html

That hack mostly allowed GDB's handling of core files to continue to
work without any changes to GDB.

The problem with setting the section size to zero is that GDB won't
know how big that section is/was.  Often, this doesn't matter because
the data in question are found in the exec file.  But it can happen
that the section describes memory that had been allocated, but never
written to.  In this instance, the contents of that memory region are
not written to the core file.  Also, since the region in question was
dynamically allocated, it won't appear in the exec file.  We don't
want these regions to appear as inaccessible to GDB (since they *were*
accessible when the process was live), so it's important that GDB know
the size of the region.

I've made changes to GDB which correctly handles this case.  When
attempting to access memory, GDB will first consider core file data
for which both SEC_ALLOC and SEC_HAS_CONTENTS is set.  Next, if that
fails, GDB will attempt to find the data in the exec file.  Finally,
if that also fails, GDB will attempt to access memory in the sections
which are flagged as SEC_ALLOC, but not SEC_HAS_CONTENTS.

bfd/ChangeLog:

	* elf.c (_bfd_elf_make_section_from_phdr): Remove hack for GDB.
---
 bfd/elf.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 9ca42e10d8..991a71ca32 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3026,14 +3026,6 @@ _bfd_elf_make_section_from_phdr (bfd *abfd,
       newsect->alignment_power = bfd_log2 (align);
       if (hdr->p_type == PT_LOAD)
 	{
-	  /* Hack for gdb.  Segments that have not been modified do
-	     not have their contents written to a core file, on the
-	     assumption that a debugger can find the contents in the
-	     executable.  We flag this case by setting the fake
-	     section size to zero.  Note that "real" bss sections will
-	     always have their contents dumped to the core file.  */
-	  if (bfd_get_format (abfd) == bfd_core)
-	    newsect->size = 0;
 	  newsect->flags |= SEC_ALLOC;
 	  if (hdr->p_flags & PF_X)
 	    newsect->flags |= SEC_CODE;
-- 
2.26.2

[PATCH v5 02/14] Adjust corefile.exp test to show regression after bfd hack removal

[Kevin Buettner]

In his review of my BZ 25631 patch series, Pedro was unable to
reproduce the regression which should occur after patch #1, "Remove
hack for GDB which sets the section size to 0", is applied.

Pedro was using an ld version older than 2.30.  Version 2.30
introduced the linker option -z separate-code.  Here's what the man
page has to say about it:

    Create separate code "PT_LOAD" segment header in the object.  This
    specifies a memory segment that should contain only instructions
    and must be in wholly disjoint pages from any other data.

In ld version 2.31, use of separate-code became the default for
Linux/x86.  So, really, 2.31 or later is required in order to see the
regression that occurs in recent Linux distributions when only the
bfd hack removal patch is applied.

For the test case in question, use of the separate-code linker option
means that the global variable "coremaker_ro" ends up in a separate
load segment (though potentially with other read-only data).  The
upshot of this is that when only patch #1 is applied, GDB won't be
able to correctly access coremaker_ro.  The reason for this is due
to the fact that this section will now have a non-zero size, but
will not have contents from the core file to find this data.
So GDB will ask BFD for the contents and BFD will respond with
zeroes for anything from those sections.  GDB should instead be
looking in the executable for this data.  Failing that, it can
then ask BFD for a reasonable value.  This is what a later patch
in this series does.

When using ld versions earlier than 2.31 (or 2.30 w/ the
-z separate-code option explicitly provided to the linker), there is
the possibility that coremaker_ro ends up being placed near other data
which is recorded in the core file.  That means that the correct value
will end up in the core file, simply because it resides on a page that
the kernel chooses to put in the core file.  This is why Pedro wasn't
able to reproduce the regression that should occur after fixing the
BFD hack.

This patch places a big chunk of memory, two pages worth on x86, in
front of "coremaker_ro" to attempt to force it onto another page
without requiring use of that new-fangled linker switch.

Speaking of which, I considered changing the test to use
-z separate-code, but this won't work because it didn't
exist prior to version 2.30.  The linker would probably complain
of an unrecognized switch.  Also, it likely won't be available in
other linkers not based on current binutils.  I.e. it probably won't
work in FreeBSD, NetBSD, etc.

To make this more concrete, this is what *should* happen when
attempting to access coremaker_ro when only patch #1 is applied:

    Core was generated by `/mesquite2/sourceware-git/f28-coresegs/bld/gdb/testsuite/outputs/gdb.base/coref'.
    Program terminated with signal SIGABRT, Aborted.
    #0  0x00007f68205deefb in raise () from /lib64/libc.so.6
    (gdb) p coremaker_ro
    $1 = 0

Note that this result is wrong; 201 should have been printed instead.
But that's the point of the rest of the patch series.

However, without this commit, or when using an old Linux distro with
a pre-2.31 ld, this is what you might see instead:

    Core was generated by `/mesquite2/sourceware-git/f28-coresegs/bld/gdb/testsuite/outputs/gdb.base/coref'.
    Program terminated with signal SIGABRT, Aborted.
    #0  0x00007f63dd658efb in raise () from /lib64/libc.so.6
    (gdb) p coremaker_ro
    $1 = 201

I.e. it prints the right answer, which sort of makes it seem like the
rest of the series isn't required.

Now, back to the patch itself... what should be the size of the memory
chunk placed before coremaker_ro?

It needs to be at least as big as the page size (PAGE_SIZE) from
the kernel.  For x86 and several other architectures this value is
4096.  I used MAPSIZE which is defined to be 8192 in coremaker.c.
So it's twice as big as what's currently needed for most Linux
architectures.  The constant PAGE_SIZE is available from <sys/user.h>,
but this isn't portable either.  In the end, it seemed simpler to
just pick a value and hope that it's big enough.  (Running a separate
program which finds the page size via sysconf(_SC_PAGESIZE) and then
passes it to the compilation via a -D switch seemed like overkill
for a case which is rendered moot by recent linker versions.)

Further information can be found here:

   https://sourceware.org/pipermail/gdb-patches/2020-May/168168.html
   https://sourceware.org/pipermail/gdb-patches/2020-May/168170.html

Thanks to H.J. Lu for telling me about the '-z separate-code' linker
switch.

gdb/testsuite/ChangeLog:

	* gdb.base/coremaker.c (filler_ro): New global constant.
---
 gdb/testsuite/gdb.base/coremaker.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/testsuite/gdb.base/coremaker.c b/gdb/testsuite/gdb.base/coremaker.c
index 3cc97e1e8e..a39b3ba8a4 100644
--- a/gdb/testsuite/gdb.base/coremaker.c
+++ b/gdb/testsuite/gdb.base/coremaker.c
@@ -42,6 +42,12 @@ char *buf2;
 int coremaker_data = 1;	/* In Data section */
 int coremaker_bss;	/* In BSS section */
 
+/* Place a chunk of memory before coremaker_ro to improve the chances
+   that coremaker_ro will end up on it's own page.  See:
+
+   https://sourceware.org/pipermail/gdb-patches/2020-May/168168.html
+   https://sourceware.org/pipermail/gdb-patches/2020-May/168170.html  */
+const unsigned char filler_ro[MAPSIZE] = {1, 2, 3, 4, 5, 6, 7, 8};
 const int coremaker_ro = 201;	/* In Read-Only Data section */
 
 /* Note that if the mapping fails for any reason, we set buf2
-- 
2.26.2

[PATCH v5 03/14] section_table_xfer_memory: Replace section name with callback predicate

[Kevin Buettner]

This patch is motivated by the need to be able to select sections
that section_table_xfer_memory_partial should consider for memory
transfers.  I'll use this facility in the next patch in this series.

section_table_xfer_memory_partial() can currently be passed a section
name which may be used to make name-based selections.  This is similar
to what I want to do, except that I want to be able to consider
section flags instead of the name.

I'm replacing the section name parameter with a predicate that,
when passed a pointer to a target_section struct, will return
true if that section should be further considered, or false which
indicates that it shouldn't.

I've converted the one existing use where a non-NULL section
name is passed to section_table_xfer_memory_partial().   Instead
of passing the section name, it now looks like this:

	  auto match_cb = [=] (const struct target_section *s)
	    {
	      return (strcmp (section_name, s->the_bfd_section->name) == 0);
	    };

	  return section_table_xfer_memory_partial (readbuf, writebuf,
						    memaddr, len, xfered_len,
						    table->sections,
						    table->sections_end,
						    match_cb);

The other callers all passed NULL; they've been simplified somewhat
in that they no longer need to pass NULL.

gdb/ChangeLog:

	* exec.h (section_table_xfer_memory): Revise declaration,
	replacing section name parameter with an optional callback
	predicate.
	* exec.c (section_table_xfer_memory): Likewise.
	* bfd-target.c, exec.c, target.c, corelow.c: Adjust all callers
	of section_table_xfer_memory.
---
 gdb/bfd-target.c |  3 +--
 gdb/corelow.c    |  3 +--
 gdb/exec.c       |  8 ++++----
 gdb/exec.h       | 13 ++++++++++---
 gdb/target.c     | 11 ++++++++---
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index b75abd7fb0..3d266951c5 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -77,8 +77,7 @@ target_bfd::xfer_partial (target_object object,
 	return section_table_xfer_memory_partial (readbuf, writebuf,
 						  offset, len, xfered_len,
 						  m_table.sections,
-						  m_table.sections_end,
-						  NULL);
+						  m_table.sections_end);
       }
     default:
       return TARGET_XFER_E_IO;
diff --git a/gdb/corelow.c b/gdb/corelow.c
index b6a12c0818..f4a5fdee12 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -615,8 +615,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 	      (readbuf, writebuf,
 	       offset, len, xfered_len,
 	       m_core_section_table.sections,
-	       m_core_section_table.sections_end,
-	       NULL));
+	       m_core_section_table.sections_end));
 
     case TARGET_OBJECT_AUXV:
       if (readbuf)
diff --git a/gdb/exec.c b/gdb/exec.c
index 2ff5846c0e..e50f38899d 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -956,7 +956,8 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf,
 				   ULONGEST *xfered_len,
 				   struct target_section *sections,
 				   struct target_section *sections_end,
-				   const char *section_name)
+				   gdb::function_view<bool
+				     (const struct target_section *)> match_cb)
 {
   int res;
   struct target_section *p;
@@ -970,7 +971,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf,
       struct bfd_section *asect = p->the_bfd_section;
       bfd *abfd = asect->owner;
 
-      if (section_name && strcmp (section_name, asect->name) != 0)
+      if (match_cb != nullptr && !match_cb (p))
 	continue;		/* not the section we need.  */
       if (memaddr >= p->addr)
         {
@@ -1043,8 +1044,7 @@ exec_target::xfer_partial (enum target_object object,
     return section_table_xfer_memory_partial (readbuf, writebuf,
 					      offset, len, xfered_len,
 					      table->sections,
-					      table->sections_end,
-					      NULL);
+					      table->sections_end);
   else
     return TARGET_XFER_E_IO;
 }
diff --git a/gdb/exec.h b/gdb/exec.h
index 54e6ff4d9b..82eb39c55d 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -65,8 +65,13 @@ extern enum target_xfer_status
    Request to transfer up to LEN 8-bit bytes of the target sections
    defined by SECTIONS and SECTIONS_END.  The OFFSET specifies the
    starting address.
-   If SECTION_NAME is not NULL, only access sections with that same
-   name.
+
+   The MATCH_CB predicate is optional; when provided it will be called
+   for each section under consideration.  When MATCH_CB evaluates as
+   true, the section remains under consideration; a false result
+   removes it from consideration for performing the memory transfers
+   noted above.  See memory_xfer_partial_1() in target.c for an
+   example.
 
    Return the number of bytes actually transfered, or zero when no
    data is available for the requested range.
@@ -83,7 +88,9 @@ extern enum target_xfer_status
 				     ULONGEST, ULONGEST, ULONGEST *,
 				     struct target_section *,
 				     struct target_section *,
-				     const char *);
+				     gdb::function_view<bool
+				       (const struct target_section *)> match_cb
+				         = nullptr);
 
 /* Read from mappable read-only sections of BFD executable files.
    Similar to exec_read_partial_read_only, but return
diff --git a/gdb/target.c b/gdb/target.c
index cd66675e8a..d03f0d5f38 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -980,11 +980,17 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	  const char *section_name = section->the_bfd_section->name;
 
 	  memaddr = overlay_mapped_address (memaddr, section);
+
+	  auto match_cb = [=] (const struct target_section *s)
+	    {
+	      return (strcmp (section_name, s->the_bfd_section->name) == 0);
+	    };
+
 	  return section_table_xfer_memory_partial (readbuf, writebuf,
 						    memaddr, len, xfered_len,
 						    table->sections,
 						    table->sections_end,
-						    section_name);
+						    match_cb);
 	}
     }
 
@@ -1002,8 +1008,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	  return section_table_xfer_memory_partial (readbuf, writebuf,
 						    memaddr, len, xfered_len,
 						    table->sections,
-						    table->sections_end,
-						    NULL);
+						    table->sections_end);
 	}
     }
 
-- 
2.26.2

[PATCH v5 04/14] Provide access to non SEC_HAS_CONTENTS core file sections

[Kevin Buettner]

Consider the following program:

- - - mkmmapcore.c - - -

static char *buf;

int
main (int argc, char **argv)
{
  buf = mmap (NULL, 8192, PROT_READ | PROT_WRITE,
              MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
  abort ();
}
- - - end mkmmapcore.c - - -

Compile it like this:

gcc -g -o mkmmapcore mkmmapcore.c

Now let's run it from GDB.  I've already placed a breakpoint on the
line with the abort() call and have run to that breakpoint.

Breakpoint 1, main (argc=1, argv=0x7fffffffd678) at mkmmapcore.c:11
11	  abort ();
(gdb) x/x buf
0x7ffff7fcb000:	0x00000000

Note that we can examine the memory allocated via the call to mmap().

Now let's try debugging a core file created by running this program.
Depending on your system, in order to make a core file, you may have to
run the following as root (or using sudo):

    echo core > /proc/sys/kernel/core_pattern

It may also be necessary to do:

    ulimit -c unlimited

I'm using Fedora 31. YMMV if you're using one of the BSDs or some other
(non-Linux) system.

This is what things look like when we debug the core file:

    [kev@f31-1 tmp]$ gdb -q ./mkmmapcore core.304767
    Reading symbols from ./mkmmapcore...
    [New LWP 304767]
    Core was generated by `/tmp/mkmmapcore'.
    Program terminated with signal SIGABRT, Aborted.
    #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
    50	  return ret;
    (gdb) x/x buf
    0x7ffff7fcb000:	Cannot access memory at address 0x7ffff7fcb000

Note that we can no longer access the memory region allocated by mmap().

Back in 2007, a hack for GDB was added to _bfd_elf_make_section_from_phdr()
in bfd/elf.c:

	  /* Hack for gdb.  Segments that have not been modified do
	     not have their contents written to a core file, on the
	     assumption that a debugger can find the contents in the
	     executable.  We flag this case by setting the fake
	     section size to zero.  Note that "real" bss sections will
	     always have their contents dumped to the core file.  */
	  if (bfd_get_format (abfd) == bfd_core)
	    newsect->size = 0;

You can find the entire patch plus links to other discussion starting
here:

    https://sourceware.org/ml/binutils/2007-08/msg00047.html

This hack sets the size of certain BFD sections to 0, which
effectively causes GDB to ignore them.  I think it's likely that the
bug described above existed even before this hack was added, but I
have no easy way to test this now.

The output from objdump -h shows the result of this hack:

 25 load13        00000000  00007ffff7fcb000  0000000000000000  00013000  2**12
                  ALLOC

(The first field, after load13, shows the size of 0.)

Once the hack is removed, the output from objdump -h shows the correct
size:

 25 load13        00002000  00007ffff7fcb000  0000000000000000  00013000  2**12
                  ALLOC

(This is a digression, but I think it's good that objdump will now show
the correct size.)

If we remove the hack from bfd/elf.c, but do nothing to GDB, we'll
see the following regression:

FAIL: gdb.base/corefile.exp: print coremaker_ro

The reason for this is that all sections which have the BFD flag
SEC_ALLOC set, but for which SEC_HAS_CONTENTS is not set no longer
have zero size.  Some of these sections have data that can (and should)
be read from the executable.  (Sections for which SEC_HAS_CONTENTS
is set should be read from the core file; sections which do not have
this flag set need to either be read from the executable or, failing
that, from the core file using whatever BFD decides is the best value
to present to the user - it uses zeros.)

At present, due to the way that the target strata are traversed when
attempting to access memory, the non-SEC_HAS_CONTENTS sections will be
read as zeroes from the process_stratum (which in this case is the
core file stratum) without first checking the file stratum, which is
where the data might actually be found.

What we should be doing is this:

- Attempt to access core file data for SEC_HAS_CONTENTS sections.
- Attempt to access executable file data if the above fails.
- Attempt to access core file data for non SEC_HAS_CONTENTS sections, if
  both of the above fail.

This corresponds to the analysis of Daniel Jacobowitz back in 2007
when the hack was added to BFD:

    https://sourceware.org/legacy-ml/binutils/2007-08/msg00045.html

The difference, observed by Pedro in his review of my v1 patches, is
that I'm using "the section flags as proxy for the p_filesz/p_memsz
checks."

gdb/ChangeLog:

	PR corefiles/25631
	* corelow.c (core_target:xfer_partial):  Revise
	TARGET_OBJECT_MEMORY case to consider non-SEC_HAS_CONTENTS
	case after first checking the stratum beneath the core
	target.
	(has_all_memory): Return true.
	* target.c (raw_memory_xfer_partial): Revise comment
	regarding use of has_all_memory.
---
 gdb/corelow.c | 47 +++++++++++++++++++++++++++++++++++++++++------
 gdb/target.c  |  7 +++++--
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index f4a5fdee12..8304d60129 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -87,7 +87,7 @@ class core_target final : public process_stratum_target
 
   const char *thread_name (struct thread_info *) override;
 
-  bool has_all_memory () override { return false; }
+  bool has_all_memory () override { return true; }
   bool has_memory () override;
   bool has_stack () override;
   bool has_registers () override;
@@ -611,12 +611,47 @@ core_target::xfer_partial (enum target_object object, const char *annex,
   switch (object)
     {
     case TARGET_OBJECT_MEMORY:
-      return (section_table_xfer_memory_partial
-	      (readbuf, writebuf,
-	       offset, len, xfered_len,
-	       m_core_section_table.sections,
-	       m_core_section_table.sections_end));
+      {
+	enum target_xfer_status xfer_status;
+
+	/* Try accessing memory contents from core file data,
+	   restricting consideration to those sections for which
+	   the BFD section flag SEC_HAS_CONTENTS is set.  */
+	auto has_contents_cb = [] (const struct target_section *s)
+	  {
+	    return ((s->the_bfd_section->flags & SEC_HAS_CONTENTS) != 0);
+	  };
+	xfer_status = section_table_xfer_memory_partial
+			(readbuf, writebuf,
+			 offset, len, xfered_len,
+			 m_core_section_table.sections,
+			 m_core_section_table.sections_end,
+			 has_contents_cb);
+	if (xfer_status == TARGET_XFER_OK)
+	  return TARGET_XFER_OK;
+
+	/* Now check the stratum beneath us; this should be file_stratum.  */
+	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.  */
+	auto no_contents_cb = [&] (const struct target_section *s)
+	  {
+	    return !has_contents_cb (s);
+	  };
+	xfer_status = section_table_xfer_memory_partial
+			(readbuf, writebuf,
+			 offset, len, xfered_len,
+			 m_core_section_table.sections,
+			 m_core_section_table.sections_end,
+			 no_contents_cb);
+
+	return xfer_status;
+      }
     case TARGET_OBJECT_AUXV:
       if (readbuf)
 	{
diff --git a/gdb/target.c b/gdb/target.c
index d03f0d5f38..58189e6202 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -925,8 +925,11 @@ raw_memory_xfer_partial (struct target_ops *ops, gdb_byte *readbuf,
       if (res == TARGET_XFER_UNAVAILABLE)
 	break;
 
-      /* We want to continue past core files to executables, but not
-	 past a running target's memory.  */
+      /* Don't continue past targets which have all the memory.
+         At one time, this code was necessary to read data from
+	 executables / shared libraries when data for the requested
+	 addresses weren't available in the core file.  But now the
+	 core target handles this case itself.  */
       if (ops->has_all_memory ())
 	break;
 
-- 
2.26.2

[PATCH v5 05/14] Test ability to access unwritten-to mmap data in core file

[Kevin Buettner]

gdb/testsuite/ChangeLog:

	PR corefiles/25631
	* gdb.base/corefile.exp (accessing anonymous, unwritten-to mmap data):
	New test.
	* gdb.base/coremaker.c (buf3): New global.
	(mmapdata): Add mmap call which uses MAP_ANONYMOUS and MAP_PRIVATE
	flags.
---
 gdb/testsuite/gdb.base/corefile.exp |  9 +++++++++
 gdb/testsuite/gdb.base/coremaker.c  | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index 34b903b350..eaabe6c0f8 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -175,6 +175,15 @@ gdb_test_multiple "x/8bd buf2" "$test" {
     }
 }
 
+# Test ability to read anonymous and, more importantly, unwritten-to
+# mmap'd data.
+
+if { ![istarget *-linux*] } {
+    setup_xfail "*-*-*"
+}
+gdb_test "x/wx buf3" "$hex:\[ \t\]+0x00000000" \
+	 "accessing anonymous, unwritten-to mmap data"
+
 # test reinit_frame_cache
 
 gdb_load ${binfile}
diff --git a/gdb/testsuite/gdb.base/coremaker.c b/gdb/testsuite/gdb.base/coremaker.c
index a39b3ba8a4..0981b21738 100644
--- a/gdb/testsuite/gdb.base/coremaker.c
+++ b/gdb/testsuite/gdb.base/coremaker.c
@@ -38,6 +38,7 @@
 
 char *buf1;
 char *buf2;
+char *buf3;
 
 int coremaker_data = 1;	/* In Data section */
 int coremaker_bss;	/* In BSS section */
@@ -104,6 +105,15 @@ mmapdata ()
     }
   /* Touch buf2 so kernel writes it out into 'core'. */
   buf2[0] = buf1[0];
+
+  /* Create yet another region which is allocated, but not written to.  */
+  buf3 = mmap (NULL, MAPSIZE, PROT_READ | PROT_WRITE,
+               MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (buf3 == (char *) -1)
+    {
+      perror ("mmap failed");
+      return;
+    }
 }
 
 void
-- 
2.26.2

[PATCH v5 06/14] Update binary_get_section_contents to seek using section's file position

[Kevin Buettner]

I have a patch for GDB which opens and reads from BFDs using the
"binary" target.  However, for it to work, we need to be able to get a
section's contents based from the file position of that section.

At the moment, reading a section's contents will always read from the
start of the file regardless of where that section is located.  While
this was fine for the original use of the "binary" target, it won't
work for my use case.  This change shouldn't impact any existing
callers due to the fact that the single .data section is initialized
with a filepos of 0.

bfd/ChangeLog:

	* binary.c (binary_get_section_contents): Seek using offset
	from section's file position.
---
 bfd/binary.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bfd/binary.c b/bfd/binary.c
index 999de0d8c4..e872924a2d 100644
--- a/bfd/binary.c
+++ b/bfd/binary.c
@@ -19,10 +19,10 @@
    Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
    MA 02110-1301, USA.  */
 
-/* This is a BFD backend which may be used to write binary objects.
-   It may only be used for output, not input.  The intention is that
-   this may be used as an output format for objcopy in order to
-   generate raw binary data.
+/* This is a BFD backend which may be used to read or write binary
+   objects.  Historically, it was used as an output format for objcopy
+   in order to generate raw binary data, but is now used for other
+   purposes as well.
 
    This is very simple.  The only complication is that the real data
    will start at some address X, and in some cases we will not want to
@@ -97,12 +97,12 @@ binary_object_p (bfd *abfd)
 
 static bfd_boolean
 binary_get_section_contents (bfd *abfd,
-			     asection *section ATTRIBUTE_UNUSED,
+			     asection *section,
 			     void * location,
 			     file_ptr offset,
 			     bfd_size_type count)
 {
-  if (bfd_seek (abfd, offset, SEEK_SET) != 0
+  if (bfd_seek (abfd, section->filepos + offset, SEEK_SET) != 0
       || bfd_bread (location, count, abfd) != count)
     return FALSE;
   return TRUE;
-- 
2.26.2

[PATCH v5 07/14] Add new gdbarch method, read_core_file_mappings

[Kevin Buettner]

The new gdbarch method, read_core_file_mappings, will be used for
reading file-backed mappings from a core file.  It'll be used
for two purposes: 1) to construct a table of file-backed mappings
in corelow.c, and 2) for display of core file mappings.

For Linux, I tried a different approach in which knowledge of the note
format was placed directly in corelow.c.  This seemed okay at first;
it was only one note format and the note format was fairly simple.
After looking at FreeBSD's note/mapping reading code, I concluded
that it's best to leave architecture specific details for decoding
the note in (architecture specific) tdep files.

With regard to display of core file mappings, I experimented with
placing the mappings display code in corelow.c.  It has access to the
file-backed mappings which were read in when the core file was loaded.
And, better, still common code could be used for all architectures.
But, again, the FreeBSD mapping code convinced me that this was not
the best approach since it has even more mapping info than Linux.
Display code which would work well for Linux will leave out mappings
as well as protection info for mappings.

So, for these reasons, I'm introducing a new gdbarch method for
reading core file mappings.

gdb/ChangeLog:

	* arch-utils.c (default_read_core_file_mappings): New function.
	* arch-utils.c (default_read_core_file_mappings): Declare.
	* gdbarch.sh (read_core_file_mappings): New gdbarch method.
	* gdbarch.h, gdbarch.c: Regenerate.
---
 gdb/arch-utils.c | 16 ++++++++++++++++
 gdb/arch-utils.h | 12 ++++++++++++
 gdb/gdbarch.c    | 23 +++++++++++++++++++++++
 gdb/gdbarch.h    |  6 ++++++
 gdb/gdbarch.sh   |  3 +++
 5 files changed, 60 insertions(+)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 13ba50abe6..12e3b8dbbb 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1036,6 +1036,22 @@ default_get_pc_address_flags (frame_info *frame, CORE_ADDR pc)
   return "";
 }
 
+/* See arch-utils.h.  */
+void
+default_read_core_file_mappings (struct gdbarch *gdbarch,
+                                 struct bfd *cbfd,
+				 gdb::function_view<void (ULONGEST count)>
+				   pre_loop_cb,
+				 gdb::function_view<void (int num,
+				                          ULONGEST start,
+							  ULONGEST end,
+							  ULONGEST file_ofs,
+							  const char *filename,
+							  const void *other)>
+				   loop_cb)
+{
+}
+
 void _initialize_gdbarch_utils ();
 void
 _initialize_gdbarch_utils ()
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 43d64b1f4f..8cb0db04c8 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -280,4 +280,16 @@ extern ULONGEST default_type_align (struct gdbarch *gdbarch,
 extern std::string default_get_pc_address_flags (frame_info *frame,
 						 CORE_ADDR pc);
 
+/* Default implementation of gdbarch read_core_file_mappings method.  */
+extern void default_read_core_file_mappings (struct gdbarch *gdbarch,
+					     struct bfd *cbfd,
+					     gdb::function_view<void (ULONGEST count)>
+					       pre_loop_cb,
+					     gdb::function_view<void (int num,
+								      ULONGEST start,
+								      ULONGEST end,
+								      ULONGEST file_ofs,
+								      const char *filename,
+								      const void *other)>
+					       loop_cb);
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 21ee840e88..bdf6977f7c 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -348,6 +348,7 @@ struct gdbarch
   const disasm_options_and_args_t * valid_disassembler_options;
   gdbarch_type_align_ftype *type_align;
   gdbarch_get_pc_address_flags_ftype *get_pc_address_flags;
+  gdbarch_read_core_file_mappings_ftype *read_core_file_mappings;
 };
 
 /* Create a new ``struct gdbarch'' based on information provided by
@@ -464,6 +465,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->addressable_memory_unit_size = default_addressable_memory_unit_size;
   gdbarch->type_align = default_type_align;
   gdbarch->get_pc_address_flags = default_get_pc_address_flags;
+  gdbarch->read_core_file_mappings = default_read_core_file_mappings;
   /* gdbarch_alloc() */
 
   return gdbarch;
@@ -712,6 +714,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of valid_disassembler_options, invalid_p == 0 */
   /* Skip verify of type_align, invalid_p == 0 */
   /* Skip verify of get_pc_address_flags, invalid_p == 0 */
+  /* Skip verify of read_core_file_mappings, invalid_p == 0 */
   if (!log.empty ())
     internal_error (__FILE__, __LINE__,
                     _("verify_gdbarch: the following are invalid ...%s"),
@@ -1281,6 +1284,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: ravenscar_ops = %s\n",
                       host_address_to_string (gdbarch->ravenscar_ops));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: read_core_file_mappings = <%s>\n",
+                      host_address_to_string (gdbarch->read_core_file_mappings));
   fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_read_pc_p() = %d\n",
                       gdbarch_read_pc_p (gdbarch));
@@ -5137,6 +5143,23 @@ set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch,
   gdbarch->get_pc_address_flags = get_pc_address_flags;
 }
 
+void
+gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd,gdb::function_view<void (ULONGEST count)> pre_loop_cb,gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const void *other)> loop_cb)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->read_core_file_mappings != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_read_core_file_mappings called\n");
+  gdbarch->read_core_file_mappings (gdbarch, cbfd, pre_loop_cb, loop_cb);
+}
+
+void
+set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch,
+                                     gdbarch_read_core_file_mappings_ftype read_core_file_mappings)
+{
+  gdbarch->read_core_file_mappings = read_core_file_mappings;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0940156aeb..7143b78f1d 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1628,6 +1628,12 @@ typedef std::string (gdbarch_get_pc_address_flags_ftype) (frame_info *frame, COR
 extern std::string gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, frame_info *frame, CORE_ADDR pc);
 extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_get_pc_address_flags_ftype *get_pc_address_flags);
 
+/* Read core file mappings */
+
+typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd,gdb::function_view<void (ULONGEST count)> pre_loop_cb,gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const void *other)> loop_cb);
+extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd,gdb::function_view<void (ULONGEST count)> pre_loop_cb,gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const void *other)> loop_cb);
+extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings);
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 41e7b8d5cc..95497f32fb 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1177,6 +1177,9 @@ m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0
 # Return a string containing any flags for the given PC in the given FRAME.
 f;std::string;get_pc_address_flags;frame_info *frame, CORE_ADDR pc;frame, pc;;default_get_pc_address_flags;;0
 
+# Read core file mappings
+m;void;read_core_file_mappings;struct bfd *cbfd,gdb::function_view<void (ULONGEST count)> pre_loop_cb,gdb::function_view<void (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, const char *filename, const void *other)> loop_cb;cbfd, pre_loop_cb, loop_cb;;default_read_core_file_mappings;;0
+
 EOF
 }
 
-- 
2.26.2

[PATCH v5 08/14] Use NT_FILE note section for reading core target memory

[Kevin Buettner]

In his reviews of my v1 and v2 corefile related patches, Pedro
identified two cases which weren't handled by those patches.

In https://sourceware.org/pipermail/gdb-patches/2020-May/168826.html,
Pedro showed that debugging a core file in which mmap() is used to
create a new mapping over an existing file-backed mapping will
produce incorrect results.  I.e, for his example, GDB would
show:

(gdb) disassemble main
Dump of assembler code for function main:
   0x00000000004004e6 <+0>:	push   %rbp
   0x00000000004004e7 <+1>:	mov    %rsp,%rbp
=> 0x00000000004004ea <+4>:	callq  0x4003f0 <abort@plt>
End of assembler dump.

This sort of looks like it might be correct, but is not due to the
fact that mmap(...MAP_FIXED...) was used to create a mapping (of all
zeros) on top of the .text section.  So, the correct result should be:

(gdb) disassemble main
Dump of assembler code for function main:
   0x00000000004004e6 <+0>:	add    %al,(%rax)
   0x00000000004004e8 <+2>:	add    %al,(%rax)
=> 0x00000000004004ea <+4>:	add    %al,(%rax)
   0x00000000004004ec <+6>:	add    %al,(%rax)
   0x00000000004004ee <+8>:	add    %al,(%rax)
End of assembler dump.

The other case that Pedro found involved an attempted examination of a
particular section in the test case from gdb.base/corefile.exp.  On
Fedora 27 or 28, the following behavior may be observed:

(gdb) info proc mappings
Mapped address spaces:

          Start Addr           End Addr       Size     Offset objfile
...
      0x7ffff7839000     0x7ffff7a38000   0x1ff000   0x1b5000 /usr/lib64/libc-2.27.so
...
(gdb) x/4x 0x7ffff7839000
0x7ffff7839000:	Cannot access memory at address 0x7ffff7839000

FYI, this section appears to be unrelocated vtable data.  See
https://sourceware.org/pipermail/gdb-patches/2020-May/168331.html for
a detailed analysis.

The important thing here is that GDB should be able to access this
address since it should be backed by the shared library.  I.e. it
should do this:

(gdb) x/4x 0x7ffff7839000
0x7ffff7839000:	0x0007ddf0	0x00000000	0x0007dba0	0x00000000

Both of these cases are fixed with this commit.

In a nutshell, this commit opens a "binary" target BFD for each of the
files that are mentioned in an NT_FILE / .note.linuxcore.file note
section.  It then uses these mappings instead of the file stratum
mappings that GDB has used in the past.

If this note section doesn't exist or is mangled for some reason, then
GDB will use the file stratum as before.  Should this happen, then
we can expect both of the above problems to again be present.

See the code comments in the commit for other details.

gdb/ChangeLog:

	* corelow.c (solist.h, unordered_map): Include.
	(class core_target): Add field m_core_file_mappings and
	method build_file_mappings.
	(core_target::core_target): Call build_file_mappings.
	(core_target::~core_target): Free memory associated with
	m_core_file_mappings.
	(core_target::build_file_mappings): New method.
	(core_target::xfer_partial): Use m_core_file_mappings
	for memory transfers.
	* linux-tdep.c (linux_read_core_file_mappings): New
	function.
	(linux_core_info_proc_mappings): Rewrite to use
	linux_read_core_file_mappings.
	(linux_init_abi): Register linux_read_core_file_mappings.
---
 gdb/corelow.c    | 137 +++++++++++++++++++++++++++++++-
 gdb/linux-tdep.c | 203 +++++++++++++++++++++++++++++++----------------
 2 files changed, 269 insertions(+), 71 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 8304d60129..aeddcccc5d 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -37,6 +37,7 @@
 #include "exec.h"
 #include "readline/tilde.h"
 #include "solib.h"
+#include "solist.h"
 #include "filenames.h"
 #include "progspace.h"
 #include "objfiles.h"
@@ -45,6 +46,7 @@
 #include "gdbsupport/filestuff.h"
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
+#include <unordered_map>
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -121,6 +123,13 @@ class core_target final : public process_stratum_target
      targets.  */
   target_section_table m_core_section_table {};
 
+  /* File-backed address space mappings: some core files include
+     information about memory mapped files.  */
+  target_section_table m_core_file_mappings {};
+
+  /* Build m_core_file_mappings.  Called from the constructor.  */
+  void build_file_mappings ();
+
   /* FIXME: kettenis/20031023: Eventually this field should
      disappear.  */
   struct gdbarch *m_core_gdbarch = NULL;
@@ -141,11 +150,120 @@ core_target::core_target ()
 			   &m_core_section_table.sections_end))
     error (_("\"%s\": Can't find sections: %s"),
 	   bfd_get_filename (core_bfd), bfd_errmsg (bfd_get_error ()));
+
+  build_file_mappings ();
 }
 
 core_target::~core_target ()
 {
   xfree (m_core_section_table.sections);
+  xfree (m_core_file_mappings.sections);
+}
+
+/* Construct the target_section_table for file-backed mappings if
+   they exist.
+
+   For each unique path in the note, we'll open a BFD with a bfd
+   target of "binary".  This is an unstructured bfd target upon which
+   we'll impose a structure from the mappings in the architecture-specific
+   mappings note.  A BFD section is allocated and initialized for each
+   file-backed mapping.
+
+   We take care to not share already open bfds with other parts of
+   GDB; in particular, we don't want to add new sections to existing
+   BFDs.  We do, however, ensure that the BFDs that we allocate here
+   will go away (be deallocated) when the core target is detached.  */
+
+void
+core_target::build_file_mappings ()
+{
+  std::unordered_map<std::string, struct bfd *> bfd_map;
+
+  /* See linux_read_core_file_mappings() in linux-tdep.c for an example
+     read_core_file_mappings method.  */
+  gdbarch_read_core_file_mappings (m_core_gdbarch, core_bfd,
+
+    /* After determining the number of mappings, read_core_file_mappings
+       will invoke this lambda which allocates target_section storage for
+       the mappings.  */
+    [&] (ULONGEST count)
+      {
+	m_core_file_mappings.sections = XNEWVEC (struct target_section, count);
+	m_core_file_mappings.sections_end = m_core_file_mappings.sections;
+      },
+
+    /* read_core_file_mappings will invoke this lambda for each mapping
+       that it finds.  */
+    [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
+         const char *filename, const void *other)
+      {
+	/* Architecture-specific read_core_mapping methods are expected to
+	   weed out non-file-backed mappings.  */
+	gdb_assert (filename != nullptr);
+
+	struct bfd *bfd = bfd_map[filename];
+	if (bfd == nullptr)
+	  {
+	    /* Use exec_file_find() to do sysroot expansion.  It'll
+	       also strip the potential sysroot "target:" prefix.  If
+	       there is no sysroot, an equivalent (possibly more
+	       canonical) pathname will be provided.  */
+	    gdb::unique_xmalloc_ptr<char> expanded_fname
+	      = exec_file_find (filename, NULL);
+	    if (expanded_fname == nullptr)
+	      {
+		warning (_("Can't open file %s during file-backed mapping "
+			   "note processing"),
+			 expanded_fname.get ());
+		return;
+	      }
+
+	    bfd = bfd_map[filename] = bfd_openr (expanded_fname.get (),
+	                                         "binary");
+
+	    if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
+	      {
+		/* 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
+		   file was removed in between checking for its
+		   existence during the expansion in exec_file_find()
+		   and the calls to bfd_openr() / bfd_check_format(). 
+		   Output both the path from the core file note along
+		   with its expansion to make debugging this problem
+		   easier.  */
+		warning (_("Can't open file %s which was expanded to %s "
+			   "during file-backed mapping note processing"),
+			 filename, expanded_fname.get ());
+		if (bfd != nullptr)
+		  bfd_close (bfd);
+		return;
+	      }
+	    /* Ensure that the bfd will be closed when core_bfd is closed. 
+	       This can be checked before/after a core file detach via
+	       "maint info bfds".  */
+	    gdb_bfd_record_inclusion (core_bfd, bfd);
+	  }
+
+	/* Make new BFD section.  All sections have the same name,
+	   which is permitted by bfd_make_section_anyway().  */
+	asection *sec = bfd_make_section_anyway (bfd, "load");
+	if (sec == nullptr)
+	  error (_("Can't make section"));
+	sec->filepos = file_ofs;
+	bfd_set_section_flags (sec, SEC_READONLY | SEC_HAS_CONTENTS);
+	bfd_set_section_size (sec, end - start);
+	bfd_set_section_vma (sec, start);
+	bfd_set_section_lma (sec, start);
+	bfd_set_section_alignment (sec, 2);
+
+	/* Set target_section fields.  */
+	struct target_section *ts = m_core_file_mappings.sections_end++;
+	ts->addr = start;
+	ts->endaddr = end;
+	ts->owner = nullptr;
+	ts->the_bfd_section = sec;
+      });
 }
 
 static void add_to_thread_list (bfd *, asection *, void *);
@@ -630,10 +748,21 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 	if (xfer_status == TARGET_XFER_OK)
 	  return TARGET_XFER_OK;
 
-	/* Now check the stratum beneath us; this should be file_stratum.  */
-	xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
-						      writebuf, offset, len,
-						      xfered_len);
+	/* 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.  */
+	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);
+	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;
 
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index fd4337f100..2ca9f599a5 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1018,106 +1018,174 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
     }
 }
 
-/* Implement "info proc mappings" for a corefile.  */
+/* Implementation of `gdbarch_read_core_file_mappings', as defined in
+   gdbarch.h.
+   
+   This function reads the NT_FILE note (which BFD turns into the
+   section ".note.linuxcore.file").  The format of this note / section
+   is described as follows in the Linux kernel sources in
+   fs/binfmt_elf.c:
+   
+      long count     -- how many files are mapped
+      long page_size -- units for file_ofs
+      array of [COUNT] elements of
+	long start
+	long end
+	long file_ofs
+      followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...
+      
+   CBFD is the BFD of the core file.
+
+   PRE_LOOP_CB is the callback function to invoke prior to starting
+   the loop which processes individual entries.  This callback will
+   only be executed after the note has been examined in enough
+   detail to verify that it's not malformed in some way.
+   
+   LOOP_CB is the callback function that will be executed once
+   for each mapping.  */
 
 static void
-linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
+linux_read_core_file_mappings (struct gdbarch *gdbarch,
+			       struct bfd *cbfd,
+			       gdb::function_view<void (ULONGEST count)>
+			         pre_loop_cb,
+			       gdb::function_view<void (int num,
+			                                ULONGEST start,
+							ULONGEST end,
+							ULONGEST file_ofs,
+							const char *filename,
+							const void *other)>
+				 loop_cb)
 {
-  asection *section;
-  ULONGEST count, page_size;
-  unsigned char *descdata, *filenames, *descend;
-  size_t note_size;
-  unsigned int addr_size_bits, addr_size;
-  struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
-  /* We assume this for reading 64-bit core files.  */
+  /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
 
-  section = bfd_get_section_by_name (core_bfd, ".note.linuxcore.file");
-  if (section == NULL)
-    {
-      warning (_("unable to find mappings in core file"));
-      return;
-    }
+  /* It's not required that the NT_FILE note exists, so return silently
+     if it's not found.  Beyond this point though, we'll complain
+     if problems are found.  */
+  asection *section = bfd_get_section_by_name (cbfd, ".note.linuxcore.file");
+  if (section == nullptr)
+    return;
 
-  addr_size_bits = gdbarch_addr_bit (core_gdbarch);
-  addr_size = addr_size_bits / 8;
-  note_size = bfd_section_size (section);
+  unsigned int addr_size_bits = gdbarch_addr_bit (gdbarch);
+  unsigned int addr_size = addr_size_bits / 8;
+  size_t note_size = bfd_section_size (section);
 
   if (note_size < 2 * addr_size)
-    error (_("malformed core note - too short for header"));
+    {
+      warning (_("malformed core note - too short for header"));
+      return;
+    }
 
-  gdb::def_vector<unsigned char> contents (note_size);
+  gdb::def_vector<gdb_byte> contents (note_size);
   if (!bfd_get_section_contents (core_bfd, section, contents.data (),
 				 0, note_size))
-    error (_("could not get core note contents"));
+    {
+      warning (_("could not get core note contents"));
+      return;
+    }
 
-  descdata = contents.data ();
-  descend = descdata + note_size;
+  gdb_byte *descdata = contents.data ();
+  char *descend = (char *) descdata + note_size;
 
   if (descdata[note_size - 1] != '\0')
-    error (_("malformed note - does not end with \\0"));
+    {
+      warning (_("malformed note - does not end with \\0"));
+      return;
+    }
 
-  count = bfd_get (addr_size_bits, core_bfd, descdata);
+  ULONGEST count = bfd_get (addr_size_bits, core_bfd, descdata);
   descdata += addr_size;
 
-  page_size = bfd_get (addr_size_bits, core_bfd, descdata);
+  ULONGEST page_size = bfd_get (addr_size_bits, core_bfd, descdata);
   descdata += addr_size;
 
   if (note_size < 2 * addr_size + count * 3 * addr_size)
-    error (_("malformed note - too short for supplied file count"));
-
-  printf_filtered (_("Mapped address spaces:\n\n"));
-  if (gdbarch_addr_bit (gdbarch) == 32)
-    {
-      printf_filtered ("\t%10s %10s %10s %10s %s\n",
-		       "Start Addr",
-		       "  End Addr",
-		       "      Size", "    Offset", "objfile");
-    }
-  else
     {
-      printf_filtered ("  %18s %18s %10s %10s %s\n",
-		       "Start Addr",
-		       "  End Addr",
-		       "      Size", "    Offset", "objfile");
+      warning (_("malformed note - too short for supplied file count"));
+      return;
     }
 
-  filenames = descdata + count * 3 * addr_size;
-  while (--count > 0)
+  char *filenames = (char *) descdata + count * 3 * addr_size;
+
+  /* Make sure that the correct number of filenames exist.  Complain
+     if there aren't enough or are too many.  */
+  char *f = filenames;
+  for (int i = 0; i < count; i++)
     {
-      ULONGEST start, end, file_ofs;
+      if (f >= descend)
+        {
+	  warning (_("malformed note - filename area is too small"));
+	  return;
+	}
+      f += strnlen (f, descend - f) + 1;
+    }
+  /* Complain, but don't return early if the filename area is too big.  */
+  if (f != descend)
+    warning (_("malformed note - filename area is too big"));
 
-      if (filenames == descend)
-	error (_("malformed note - filenames end too early"));
+  pre_loop_cb (count);
 
-      start = bfd_get (addr_size_bits, core_bfd, descdata);
+  for (int i = 0; i < count; i++)
+    {
+      ULONGEST start = bfd_get (addr_size_bits, core_bfd, descdata);
       descdata += addr_size;
-      end = bfd_get (addr_size_bits, core_bfd, descdata);
+      ULONGEST end = bfd_get (addr_size_bits, core_bfd, descdata);
       descdata += addr_size;
-      file_ofs = bfd_get (addr_size_bits, core_bfd, descdata);
+      ULONGEST file_ofs
+        = bfd_get (addr_size_bits, core_bfd, descdata) * page_size;
       descdata += addr_size;
+      char * filename = filenames;
+      filenames += strlen ((char *) filenames) + 1;
 
-      file_ofs *= page_size;
-
-      if (gdbarch_addr_bit (gdbarch) == 32)
-	printf_filtered ("\t%10s %10s %10s %10s %s\n",
-			 paddress (gdbarch, start),
-			 paddress (gdbarch, end),
-			 hex_string (end - start),
-			 hex_string (file_ofs),
-			 filenames);
-      else
-	printf_filtered ("  %18s %18s %10s %10s %s\n",
-			 paddress (gdbarch, start),
-			 paddress (gdbarch, end),
-			 hex_string (end - start),
-			 hex_string (file_ofs),
-			 filenames);
-
-      filenames += 1 + strlen ((char *) filenames);
+      loop_cb (i, start, end, file_ofs, filename, nullptr);
     }
 }
 
+/* Implement "info proc mappings" for a corefile.  */
+
+static void
+linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
+{
+  linux_read_core_file_mappings (gdbarch, core_bfd,
+    [=] (ULONGEST count)
+      {
+	printf_filtered (_("Mapped address spaces:\n\n"));
+	if (gdbarch_addr_bit (gdbarch) == 32)
+	  {
+	    printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			     "Start Addr",
+			     "  End Addr",
+			     "      Size", "    Offset", "objfile");
+	  }
+	else
+	  {
+	    printf_filtered ("  %18s %18s %10s %10s %s\n",
+			     "Start Addr",
+			     "  End Addr",
+			     "      Size", "    Offset", "objfile");
+	  }
+      },
+    [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
+         const char *filename, const void *other)
+      {
+	if (gdbarch_addr_bit (gdbarch) == 32)
+	  printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (file_ofs),
+			   filename);
+	else
+	  printf_filtered ("  %18s %18s %10s %10s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (file_ofs),
+			   filename);
+      });
+}
+
 /* Implement "info proc" for a corefile.  */
 
 static void
@@ -2472,6 +2540,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_info_proc (gdbarch, linux_info_proc);
   set_gdbarch_core_info_proc (gdbarch, linux_core_info_proc);
   set_gdbarch_core_xfer_siginfo (gdbarch, linux_core_xfer_siginfo);
+  set_gdbarch_read_core_file_mappings (gdbarch, linux_read_core_file_mappings);
   set_gdbarch_find_memory_regions (gdbarch, linux_find_memory_regions);
   set_gdbarch_make_corefile_notes (gdbarch, linux_make_corefile_notes);
   set_gdbarch_has_shared_address_space (gdbarch,
-- 
2.26.2

[PATCH v5 09/14] Add test for accessing read-only mmapped data in a core file

[Kevin Buettner]

This test passes when run using a GDB with my corefile patches.  When
run against a GDB without my patches, I see the following failures,
the first of which is due to the test added by this commit:

FAIL: gdb.base/corefile.exp: accessing read-only mmapped data in core file (mapping address not found in core file)
FAIL: gdb.base/corefile.exp: accessing anonymous, unwritten-to mmap data

gdb/testsuite/ChangeLog:

	* gdb.base/corefile.exp: Add test "accessing read-only mmapped
	data in core file".
	* gdb.base/coremaker.c (buf2ro): New global.
	(mmapdata): Add a read-only mmap mapping.
---
 gdb/testsuite/gdb.base/corefile.exp | 18 +++++++++++++++++-
 gdb/testsuite/gdb.base/coremaker.c  | 14 ++++++++++++--
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index eaabe6c0f8..8abf62b51f 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -34,7 +34,10 @@ if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
     return -1
 }
 
-set corefile [core_find $binfile {coremmap.data}]
+# Do not delete coremap.data when calling core_find.  This file is
+# required for GDB to find mmap'd data in the "accessing read-only
+# mmapped data in core file" test.
+set corefile [core_find $binfile {}]
 if {$corefile == ""} {
     return 0
 }
@@ -175,6 +178,19 @@ gdb_test_multiple "x/8bd buf2" "$test" {
     }
 }
 
+set test "accessing read-only mmapped data in core file"
+gdb_test_multiple "x/8bd buf2ro" "$test" {
+    -re ".*:.*0.*1.*2.*3.*4.*5.*6.*7.*$gdb_prompt $" {
+	pass "$test"
+    }
+    -re "0x\[f\]*:.*Cannot access memory at address 0x\[f\]*.*$gdb_prompt $" {
+	fail "$test (mapping failed at runtime)"
+    }
+    -re "0x.*:.*Cannot access memory at address 0x.*$gdb_prompt $" {
+	fail "$test (mapping address not found in core file)"
+    }
+}
+
 # Test ability to read anonymous and, more importantly, unwritten-to
 # mmap'd data.
 
diff --git a/gdb/testsuite/gdb.base/coremaker.c b/gdb/testsuite/gdb.base/coremaker.c
index 0981b21738..3fc13e9287 100644
--- a/gdb/testsuite/gdb.base/coremaker.c
+++ b/gdb/testsuite/gdb.base/coremaker.c
@@ -38,6 +38,7 @@
 
 char *buf1;
 char *buf2;
+char *buf2ro;
 char *buf3;
 
 int coremaker_data = 1;	/* In Data section */
@@ -90,16 +91,25 @@ mmapdata ()
       return;
     }
 
+  /* Map in another copy, read-only.  We won't write to this copy so it
+     will likely not end up in the core file.  */
+  buf2ro = (char *) mmap (0, MAPSIZE, PROT_READ, MAP_PRIVATE, fd, 0);
+  if (buf2ro == (char *) -1)
+    {
+      perror ("mmap failed");
+      return;
+    }
+
   /* Verify that the original data and the mapped data are identical.
      If not, we'd rather fail now than when trying to access the mapped
      data from the core file. */
 
   for (j = 0; j < MAPSIZE; ++j)
     {
-      if (buf1[j] != buf2[j])
+      if (buf1[j] != buf2[j] || buf1[j] != buf2ro[j])
 	{
 	  fprintf (stderr, "mapped data is incorrect");
-	  buf2 = (char *) -1;
+	  buf2 = buf2ro = (char *) -1;
 	  return;
 	}
     }
-- 
2.26.2

[PATCH v5 10/14] gcore command: Place all file-backed mappings in NT_FILE note

[Kevin Buettner]

When making a core file with the GDB's gcore command on Linux,
the same criteria used for determining which mappings should be
dumped were also being used for determining which entries should
be placed in the NT_FILE note.  This is wrong; we want to place
all file-backed mappings in this note.

The predicate function, dump_mapping_p, was used to determine whether
or not to dump a mapping from within linux_find_memory_regions_full.
This commit leaves this predicate in place, but adds a new parameter,
should_dump_mapping_p, to linux_find_memory_regions_full.  It then
calls should_dump_mapping_p instead of dump_mapping_p.  dump_mapping_p
is passed to linux_find_memory_regions_full at one call site; at the
other call site, dump_note_entry_p is passed instead.

gdb/ChangeLog:

	* linux-tdep.c (dump_note_entry_p): New function.
	(linux_dump_mapping_p_ftype): New typedef.
	(linux_find_memory_regions_full): Add new parameter,
	should_dump_mapping_p.
	(linux_find_memory_regions): Adjust call to
	linux_find_memory_regions_full.
	(linux_make_mappings_core_file_notes): Use dump_note_entry_p in
	call to linux_find_memory_regions_full.
---
 gdb/linux-tdep.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 2ca9f599a5..0b2b032dc7 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -726,6 +726,25 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
   return dump_p;
 }
 
+/* As above, but return true only when we should dump the NT_FILE
+   entry.  */
+
+static int
+dump_note_entry_p (filter_flags filterflags, const struct smaps_vmflags *v,
+		int maybe_private_p, int mapping_anon_p, int mapping_file_p,
+		const char *filename, ULONGEST addr, ULONGEST offset)
+{
+  /* vDSO and vsyscall mappings will end up in the core file.  Don't
+     put them in the NT_FILE note.  */
+  if (strcmp ("[vdso]", filename) == 0
+      || strcmp ("[vsyscall]", filename) == 0)
+    return 0;
+
+  /* Otherwise, any other file-based mapping should be placed in the
+     note.  */
+  return filename != nullptr;
+}
+
 /* Implement the "info proc" command.  */
 
 static void
@@ -1240,10 +1259,20 @@ typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
 					    const char *filename,
 					    void *data);
 
+typedef int linux_dump_mapping_p_ftype (filter_flags filterflags,
+					const struct smaps_vmflags *v,
+					int maybe_private_p,
+					int mapping_anon_p,
+					int mapping_file_p,
+					const char *filename,
+					ULONGEST addr,
+					ULONGEST offset);
+
 /* List memory regions in the inferior for a corefile.  */
 
 static int
 linux_find_memory_regions_full (struct gdbarch *gdbarch,
+				linux_dump_mapping_p_ftype *should_dump_mapping_p,
 				linux_find_memory_region_ftype *func,
 				void *obfd)
 {
@@ -1394,9 +1423,10 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 	    }
 
 	  if (has_anonymous)
-	    should_dump_p = dump_mapping_p (filterflags, &v, priv,
-					    mapping_anon_p, mapping_file_p,
-					    filename, addr, offset);
+	    should_dump_p = should_dump_mapping_p (filterflags, &v, priv,
+					           mapping_anon_p,
+						   mapping_file_p,
+					           filename, addr, offset);
 	  else
 	    {
 	      /* Older Linux kernels did not support the "Anonymous:" counter.
@@ -1460,6 +1490,7 @@ linux_find_memory_regions (struct gdbarch *gdbarch,
   data.obfd = obfd;
 
   return linux_find_memory_regions_full (gdbarch,
+					 dump_mapping_p,
 					 linux_find_memory_regions_thunk,
 					 &data);
 }
@@ -1543,7 +1574,9 @@ linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
   pack_long (buf, long_type, 1);
   obstack_grow (&data_obstack, buf, TYPE_LENGTH (long_type));
 
-  linux_find_memory_regions_full (gdbarch, linux_make_mappings_callback,
+  linux_find_memory_regions_full (gdbarch, 
+				  dump_note_entry_p,
+				  linux_make_mappings_callback,
 				  &mapping_data);
 
   if (mapping_data.file_count != 0)
-- 
2.26.2

[PATCH v5 11/14] Adjust coredump-filter.exp to account for NT_FILE note handling

[Kevin Buettner]

This commit makes adjustments to coredump-filter.exp to account
for the fact that NT_FILE file-backed mappings are now available
when a core file is loaded.  Thus, a test which was expected
to PASS when a memory region was determined to be unavailable
(due to no file-backed mappings being available) will now FAIL
due to those mappings being available from having loaded the
NT_FILE note.

I had originally marked the test as XFAIL, but Mihails Strasuns
suggested a much better approach:

    1) First test that it still works if file is accessible in the
       filesystem.
    2) Temporarily move / rename the file and test that disassembly
       doesn't work anymore.

That's what this commit implements.

gdb/testsuite/ChangeLog:

	* gdb.base/coredump-filter.exp: Add second
	non-Private-Shared-Anon-File test.
	(test_disasm): Rename binfile for test which is expected
	to fail.
---
 gdb/testsuite/gdb.base/coredump-filter.exp | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index ff398f2b85..b723b62490 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -80,15 +80,26 @@ proc do_load_and_test_core { core var working_var working_value dump_excluded }
 # disassemble of a function (i.e., the binary's .text section).  GDB
 # should fail in this case.  However, it must succeed if the binary is
 # provided along with the corefile.  This is what we test here.
+#
+# A further complication is that Linux NT_FILE notes are now read from
+# the corefile.  This note allows GDB to find the binary for file
+# backed mappings even though the binary wasn't loaded by GDB in the
+# conventional manner.  In order to see the expected failure for this
+# case, we rename the binary in order to perform this test.
 
 proc test_disasm { core address should_fail } {
-    global testfile hex
+    global testfile hex binfile
 
     # Restart GDB without loading the binary.
     with_test_prefix "no binary" {
 	gdb_exit
 	gdb_start
 
+	set hide_binfile [standard_output_file "${testfile}.hide"]
+	if { $should_fail == 1 } {
+	    file rename -force $binfile $hide_binfile
+	}
+
 	set core_loaded [gdb_core_cmd "$core" "load core"]
 	if { $core_loaded == -1 } {
 	    fail "loading $core"
@@ -96,6 +107,7 @@ proc test_disasm { core address should_fail } {
 	}
 
 	if { $should_fail == 1 } {
+	    file rename -force $hide_binfile $binfile
 	    gdb_test "x/i \$pc" "=> $hex:\tCannot access memory at address $hex" \
 		"disassemble function with corefile and without a binary"
 	} else {
@@ -225,5 +237,9 @@ foreach item $all_anon_corefiles {
 }
 
 with_test_prefix "loading and testing corefile for non-Private-Shared-Anon-File" {
+    test_disasm $non_private_shared_anon_file_core $main_addr 0
+}
+
+with_test_prefix "loading and testing corefile for non-Private-Shared-Anon-File with renamed binary" {
     test_disasm $non_private_shared_anon_file_core $main_addr 1
 }
-- 
2.26.2

[PATCH v5 12/14] Add new command "maint print core-file-backed-mappings"

[Kevin Buettner]

I wrote a read_core_file_mappings method for FreeBSD and then registered
this gdbarch method.  I saw some strange behavior while testing it and
wanted a way to make sure that mappings were being correctly loaded
into corelow.c, so I wrote the new command which is the topic of this
commit.  I think it might be occasionally useful for debugging strange
corefile behavior.

With regard to FreeBSD, my work isn't ready yet.  Unlike Linux,
FreeBSD puts all mappings into its core file note.  And, unlike Linux,
it doesn't dump load segments which occupy no space in the file.  So
my (perhaps naive) implementation of a FreeBSD read_core_file_mappings
didn't work all that well:  I saw more failures in the corefile2.exp
tests than without it.  I think it should be possible to make FreeBSD
work as well as Linux, but it will require doing something with all of
the mappings, not just the file based mappings that I was considering.

In the v4 series, Pedro asked the following:

    I don't understand what this command provides that "info proc
    mappings" doesn't?  Can you give an example of when you'd use this
    command over "info proc mappings" ?

On Linux, "info proc mappings" and "maint print core-file-backed-mappings"
will produce similar, possibly identical, output.  This need not be
the case for other OSes.  E.g. on FreeBSD, had I finished the
implementation, the output from these commands would have been very
different.  The FreeBSD "info proc mappings" command would show
additional (non-file-backed) mappings in addition to at least one
additional field (memory permissions) for each mapping.

As noted earlier, I was seeing some unexpected behavior while working
on the FreeBSD implementation and wanted to be certain that the
mappings were being correctly loaded by corelow.c.  "info proc
mappings" prints the core file mappings, but doesn't tell us anything
about whether they've been loaded by corelow.c This new maintenance
command directly interrogates the data structures and prints the
values found there.

gdb/ChangeLog:

	* corelow.c (gdbcmd.h): Include.
	(core_target::info_proc_mappings): New method.
	(get_current_core_target): New function.
	(maintenance_print_core_file_backed_mappings): New function.
	(_initialize_corelow): Add core-file-backed-mappings to
	"maint print" commands.
---
 gdb/corelow.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index aeddcccc5d..f9648c5b47 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -47,6 +47,7 @@
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
 #include <unordered_map>
+#include "gdbcmd.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -113,6 +114,9 @@ class core_target final : public process_stratum_target
 				  const char *human_name,
 				  bool required);
 
+  /* See definition.  */
+  void info_proc_mappings (struct gdbarch *gdbarch);
+
 private: /* per-core data */
 
   /* The core's section table.  Note that these target sections are
@@ -1026,9 +1030,92 @@ core_target::info_proc (const char *args, enum info_proc_what request)
   return true;
 }
 
+/* Get a pointer to the current core target.  If not connected to a
+   core target, return NULL.  */
+
+static core_target *
+get_current_core_target ()
+{
+  target_ops *proc_target = current_inferior ()->process_target ();
+  return dynamic_cast<core_target *> (proc_target);
+}
+
+/* Display file backed mappings from core file.  */
+
+void
+core_target::info_proc_mappings (struct gdbarch *gdbarch)
+{
+  if (m_core_file_mappings.sections != m_core_file_mappings.sections_end)
+    {
+      printf_filtered (_("Mapped address spaces:\n\n"));
+      if (gdbarch_addr_bit (gdbarch) == 32)
+	{
+	  printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "objfile");
+	}
+      else
+	{
+	  printf_filtered ("  %18s %18s %10s %10s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "objfile");
+	}
+    }
+
+  for (const struct target_section *tsp = m_core_file_mappings.sections;
+       tsp < m_core_file_mappings.sections_end;
+       tsp++)
+    {
+      ULONGEST start = tsp->addr;
+      ULONGEST end = tsp->endaddr;
+      ULONGEST file_ofs = tsp->the_bfd_section->filepos;
+      const char *filename = bfd_get_filename (tsp->the_bfd_section->owner);
+
+      if (gdbarch_addr_bit (gdbarch) == 32)
+	printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			 paddress (gdbarch, start),
+			 paddress (gdbarch, end),
+			 hex_string (end - start),
+			 hex_string (file_ofs),
+			 filename);
+      else
+	printf_filtered ("  %18s %18s %10s %10s %s\n",
+			 paddress (gdbarch, start),
+			 paddress (gdbarch, end),
+			 hex_string (end - start),
+			 hex_string (file_ofs),
+			 filename);
+    }
+}
+
+/* Implement "maintenance print core-file-backed-mappings" command.  
+
+   If mappings are loaded, the results should be similar to the
+   mappings shown by "info proc mappings".  This command is mainly a
+   debugging tool for GDB developers to make sure that the expected
+   mappings are present after loading a core file.  For Linux, the
+   output provided by this command will be very similar (if not
+   identical) to that provided by "info proc mappings".  This is not
+   necessarily the case for other OSes which might provide
+   more/different information in the "info proc mappings" output.  */
+
+static void
+maintenance_print_core_file_backed_mappings (const char *args, int from_tty)
+{
+  core_target *targ = get_current_core_target ();
+  if (targ != nullptr)
+    targ->info_proc_mappings (targ->core_gdbarch ());
+}
+
 void _initialize_corelow ();
 void
 _initialize_corelow ()
 {
   add_target (core_target_info, core_target_open, filename_completer);
+  add_cmd ("core-file-backed-mappings", class_maintenance,
+           maintenance_print_core_file_backed_mappings,
+	   _("Print core file's file-backed mappings"),
+	   &maintenanceprintlist);
 }
-- 
2.26.2

[PATCH v5 13/14] Add documentation for "maint print core-file-backed-mappings"

[Kevin Buettner]

gdb/ChangeLog:

	* NEWS (New commands): Mention new command
	"maintenance print core-file-backed-mappings".

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Add documentation for
	new command "maintenance print core-file-backed-mappings".
---
 gdb/NEWS            | 4 ++++
 gdb/doc/gdb.texinfo | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 001dc5e468..ff0624ce6e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -85,6 +85,10 @@ maintenance print xml-tdesc [FILE]
   the target description is read from FILE into GDB, and then
   reprinted.
 
+maintenance print core-file-backed-mappings
+  Prints file-backed mappings loaded from a core file's note section.
+  Output is expected to be similar to that of "info proc mappings".
+
 * Changed commands
 
 alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a002084d5b..b1dc6c0998 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38532,6 +38532,14 @@ library.  This exercises all @code{libthread_db} functionality used by
 @code{libthread_db} uses.  Note that parts of the test may be skipped
 on some platforms when debugging core files.
 
+@kindex maint print core-file-backed-mappings
+@cindex memory address space mappings
+@item maint print core-file-backed-mappings
+Print the file-backed mappings which were loaded from a core file note.
+This output represents state internal to @value{GDBN} and should be
+similar to the mappings displayed by the @code{info proc mappings}
+command.
+
 @kindex maint print dummy-frames
 @item maint print dummy-frames
 Prints the contents of @value{GDBN}'s internal dummy-frame stack.
-- 
2.26.2

[PATCH v5 14/14] New core file tests with mappings over existing program memory

[Kevin Buettner]

This test case was inspired by Pedro's demonstration of a problem
with my v2 patches.  It can be found here:

    https://sourceware.org/pipermail/gdb-patches/2020-May/168826.html

In a nutshell, my earlier patches could not handle the case in
which a read-only mapping created with mmap() was created at
an address used by other file-backed read-only memory in use by
the process.

This problem has been fixed (for Linux, anyway) by the commit "Use
NT_FILE note section for reading core target memory".

When I run this test without any of my recent corefile patches,
I see these failures:

FAIL: gdb.base/corefile2.exp: kernel core: print/x mbuf_ro[0]@4
FAIL: gdb.base/corefile2.exp: kernel core: print/x mbuf_ro[pagesize-4]@4
FAIL: gdb.base/corefile2.exp: kernel core: print/x mbuf_ro[-3]@6
FAIL: gdb.base/corefile2.exp: kernel core: print/x mbuf_rw[pagesize-3]@6
FAIL: gdb.base/corefile2.exp: kernel core: print/x mbuf_ro[pagesize-3]@6
FAIL: gdb.base/corefile2.exp: maint print core-file-backed-mappings
FAIL: gdb.base/corefile2.exp: gcore core: print/x mbuf_ro[-3]@6

The ones involving mbuf_ro will almost certainly fail when run on
non-Linux systems; I've used setup_xfail on those tests to prevent
them from outright FAILing when not run on Linux.  For a time, I
had considered skipping these tests altogether when not run on
Linux, but I changed my mind due to this failure...

FAIL: gdb.base/corefile2.exp: print/x mbuf_rw[pagesize-3]@6

I think it *should* pass without my recent corefile patches.  The fact
that it doesn't is likely due to a bug in GDB.  The following
interaction with GDB demonstrates the problem:

(gdb) print/x mbuf_rw[pagesize-3]@6
$1 = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
(gdb) print/x mbuf_rw[pagesize]@3
$2 = {0x6b, 0x6b, 0x6b}

The last three values in display of $1 should be the same as those
shown by $2.  Like this...

(gdb) print/x mbuf_rw[pagesize-3]@6
$1 = {0x0, 0x0, 0x0, 0x6b, 0x6b, 0x6b}
(gdb) print/x mbuf_rw[pagesize]@3
$2 = {0x6b, 0x6b, 0x6b}

That latter output was obtained with the use of all of my current
corefile patches.  I see no failures on Linux when running this test
with my current set of corefile patches.  I tested 3 architectures:
x86_64, s390x, and aarch64.

I also tested on FreeBSD 12.1-RELEASE.  I see the following results
both with and without the current set of core file patches:

    # of expected passes		26
    # of expected failures		8

Of particular interest is that I did *not* see the problematic mbuf_rw
failure noted earlier (both with and without the core file patches).
I still don't have an explanation for why this failure occurred on
Linux.  Prior to running the tests, I had hypothesized that I'd see
this failure on FreeBSD too, but testing shows that this is not the
case.

Also of importance is that we see no FAILs with this test on FreeBSD
which indicates that I XFAILed the correct tests.

This version runs the interesting tests twice, once with a kernel
created core file and another time with a gcore created core file.

It also does a very minimal test of the new command "maint print
core-file-backed-mappings".

gdb/testsuite/ChangeLog:

	* gdb.base/corefile2.exp: New file.
	* gdb.base/coremaker2.exp: New file.
---
 gdb/testsuite/gdb.base/corefile2.exp | 185 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/coremaker2.c  | 150 ++++++++++++++++++++++
 2 files changed, 335 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/corefile2.exp
 create mode 100644 gdb/testsuite/gdb.base/coremaker2.c

diff --git a/gdb/testsuite/gdb.base/corefile2.exp b/gdb/testsuite/gdb.base/corefile2.exp
new file mode 100644
index 0000000000..5de7ead4d4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/corefile2.exp
@@ -0,0 +1,185 @@
+# Copyright 2020 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/>.
+
+# Tests of core file memory accesses when mmap() has been used to
+# create a "hole" of zeroes over pre-existing memory regions.  See
+# coremaker2.c for details.
+
+# are we on a target board
+if ![isnative] then {
+    return
+}
+
+# Some of these tests will only work on GNU/Linux due to the
+# fact that Linux core files includes a section describing
+# memory address to file mappings.  We'll use set_up_xfail for the
+# affected tests.  As other targets become supported, the condition
+# can be changed accordingly.
+
+set xfail 0
+if { ![istarget *-linux*] } {
+    set xfail 1
+}
+
+standard_testfile coremaker2.c
+
+if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
+    untested "failed to compile"
+    return -1
+}
+
+set corefile [core_find $binfile {}]
+if {$corefile == ""} {
+    return 0
+}
+
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Attempt to load the core file.
+
+gdb_test_multiple "core-file $corefile" "core-file command" {
+    -re ".* program is being debugged already.*y or n. $" {
+	# gdb_load may connect us to a gdbserver.
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "Core was generated by .*corefile.*\r\n\#0  .*\(\).*\r\n$gdb_prompt $" {
+	pass "core-file command"
+    }
+    -re "Core was generated by .*\r\n\#0  .*\(\).*\r\n$gdb_prompt $" {
+	pass "core-file command (with bad program name)"
+    }
+    -re ".*registers from core file: File in wrong format.* $" {
+	fail "core-file command (could not read registers from core file)"
+    }
+}
+
+# Perform the "interesting" tests which check the contents of certain
+# memory regions.
+
+proc do_tests { } {
+    global xfail
+
+    # Check contents of beginning of buf_rw and buf_ro.
+
+    gdb_test {print/x buf_rw[0]@4} {\{0x6b, 0x6b, 0x6b, 0x6b\}}
+    gdb_test {print/x buf_ro[0]@4} {\{0xc5, 0xc5, 0xc5, 0xc5\}}
+
+    # Check for correct contents at beginning of mbuf_rw and mbuf_ro.
+
+    gdb_test {print/x mbuf_rw[0]@4} {\{0x0, 0x0, 0x0, 0x0\}}
+
+    if { $xfail } { setup_xfail "*-*-*" }
+    gdb_test {print/x mbuf_ro[0]@4} {\{0x0, 0x0, 0x0, 0x0\}}
+
+    # Check contents of mbuf_rw and mbuf_ro at the end of these regions.
+
+    gdb_test {print/x mbuf_rw[pagesize-4]@4} {\{0x0, 0x0, 0x0, 0x0\}}
+
+    if { $xfail } { setup_xfail "*-*-*" }
+    gdb_test {print/x mbuf_ro[pagesize-4]@4} {\{0x0, 0x0, 0x0, 0x0\}}
+
+    # Check contents of mbuf_rw and mbuf_ro, right before the hole,
+    # overlapping into the beginning of these mmap'd regions.
+
+    gdb_test {print/x mbuf_rw[-3]@6} {\{0x6b, 0x6b, 0x6b, 0x0, 0x0, 0x0\}}
+
+    if { $xfail } { setup_xfail "*-*-*" }
+    gdb_test {print/x mbuf_ro[-3]@6} {\{0xc5, 0xc5, 0xc5, 0x0, 0x0, 0x0\}}
+
+    # Likewise, at the end of the mbuf_rw and mbuf_ro, with overlap.
+
+    # If this test FAILs, it's probably a genuine bug unrelated to whether
+    # the core file includes a section describing memory address to file
+    # mappings or not.  (So don't xfail it!)
+    gdb_test {print/x mbuf_rw[pagesize-3]@6} {\{0x0, 0x0, 0x0, 0x6b, 0x6b, 0x6b\}}
+
+    if { $xfail } { setup_xfail "*-*-*" }
+    gdb_test {print/x mbuf_ro[pagesize-3]@6} {\{0x0, 0x0, 0x0, 0xc5, 0xc5, 0xc5\}}
+
+    # Check contents of (what should be) buf_rw and buf_ro immediately after
+    # mbuf_rw and mbuf_ro holes.
+
+    gdb_test {print/x mbuf_rw[pagesize]@4} {\{0x6b, 0x6b, 0x6b, 0x6b\}}
+    gdb_test {print/x mbuf_ro[pagesize]@4} {\{0xc5, 0xc5, 0xc5, 0xc5\}}
+
+    # Check contents at ends of buf_rw and buf_rw.
+
+    gdb_test {print/x buf_rw[sizeof(buf_rw)-4]@4} {\{0x6b, 0x6b, 0x6b, 0x6b\}}
+    gdb_test {print/x buf_ro[sizeof(buf_ro)-4]@4} {\{0xc5, 0xc5, 0xc5, 0xc5\}}
+}
+
+# Run tests with kernel-produced core file.
+
+with_test_prefix "kernel core" {
+    do_tests
+}
+
+# Verify that "maint print core-file-backed-mappings" exists and does
+# not crash GDB.  If it produces any output at all, make sure that
+# that output at least mentions binfile.
+
+set test "maint print core-file-backed-mappings"
+gdb_test_multiple $test "" {
+    -re ".*$binfile.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "^$test\[\r\n\]*$gdb_prompt $" {
+	pass "$test (no output)"
+    }
+}
+
+# Restart and run to the abort call.
+
+clean_restart $binfile
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return
+}
+
+gdb_breakpoint [gdb_get_line_number "abort"]
+gdb_continue_to_breakpoint "at abort"
+
+# Do not execute abort call; instead, invoke gcore command to make a
+# gdb-produced core file.
+
+set corefile [standard_output_file gcore.test]
+set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"]
+if {!$core_supported} {
+  return
+}
+
+# maint print-core-file-backed-mappings shouldn't produce any output
+# when not debugging a core file.
+
+gdb_test_no_output "maint print core-file-backed-mappings" \
+    "maint print core-file-backed-mapping with no core file"
+
+clean_restart $binfile
+
+set core_loaded [gdb_core_cmd "$corefile" "re-load generated corefile"]
+if { $core_loaded == -1 } {
+    # No use proceeding from here.
+    return
+}
+
+# Run tests using gcore-produced core file.
+
+with_test_prefix "gcore core" {
+    do_tests
+}
diff --git a/gdb/testsuite/gdb.base/coremaker2.c b/gdb/testsuite/gdb.base/coremaker2.c
new file mode 100644
index 0000000000..ecba247f50
--- /dev/null
+++ b/gdb/testsuite/gdb.base/coremaker2.c
@@ -0,0 +1,150 @@
+/* Copyright 1992-2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+/*  This test has two large memory areas buf_rw and buf_ro. 
+
+    buf_rw is written to by the program while buf_ro is initialized at
+    compile / load time.  Thus, when a core file is created, buf_rw's
+    memory should reside in the core file, but buf_ro probably won't be.
+    Instead, the contents of buf_ro are available from the executable.
+
+    Now, for the wrinkle:  We create a one page read-only mapping over
+    both of these areas.  This will create a one page "hole" of all
+    zeros in each area.
+
+    Will GDB be able to correctly read memory from each of the four
+    (or six, if you count the regions on the other side of each hole)
+    memory regions?  */
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+
+/* These are globals so that we can find them easily when debugging
+   the core file.  */
+long pagesize;
+unsigned long long addr;
+char *mbuf_ro;
+char *mbuf_rw;
+
+/* 24 KiB buffer.  */
+char buf_rw[24 * 1024];
+
+/* 24 KiB worth of data.  For this test case, we can't allocate a
+   buffer and then fill it; we want GDB to have to read this data
+   from the executable; it should NOT find it in the core file.  */
+
+#define C5_16 \
+  0xc5, 0xc5, 0xc5, 0xc5, \
+  0xc5, 0xc5, 0xc5, 0xc5, \
+  0xc5, 0xc5, 0xc5, 0xc5, \
+  0xc5, 0xc5, 0xc5, 0xc5
+
+#define C5_256 \
+  C5_16, C5_16, C5_16, C5_16, \
+  C5_16, C5_16, C5_16, C5_16, \
+  C5_16, C5_16, C5_16, C5_16, \
+  C5_16, C5_16, C5_16, C5_16
+
+#define C5_1k \
+  C5_256, C5_256, C5_256, C5_256
+
+#define C5_24k \
+  C5_1k, C5_1k, C5_1k, C5_1k, \
+  C5_1k, C5_1k, C5_1k, C5_1k, \
+  C5_1k, C5_1k, C5_1k, C5_1k, \
+  C5_1k, C5_1k, C5_1k, C5_1k, \
+  C5_1k, C5_1k, C5_1k, C5_1k, \
+  C5_1k, C5_1k, C5_1k, C5_1k
+
+const char buf_ro[] = { C5_24k };
+
+int
+main (int argc, char **argv)
+{
+  int i, bitcount;
+
+#ifdef _SC_PAGESIZE
+  pagesize = sysconf (_SC_PAGESIZE);
+#else
+  pagesize = 8192;
+#endif
+
+  /* Verify that pagesize is a power of 2.  */
+  bitcount = 0;
+  for (i = 0; i < 4 * sizeof (pagesize); i++)
+    if (pagesize & (1 << i))
+      bitcount++;
+
+  if (bitcount != 1)
+    {
+      fprintf (stderr, "pagesize is not a power of 2.\n");
+      exit (1);
+    }
+
+  /* Compute an address that should be within buf_ro.  Complain if not.  */
+  addr = ((unsigned long long) buf_ro + pagesize) & ~(pagesize - 1);
+
+  if (addr <= (unsigned long long) buf_ro
+      || addr >= (unsigned long long) buf_ro + sizeof (buf_ro))
+    {
+      fprintf (stderr, "Unable to compute a suitable address within buf_ro.\n");
+      exit (1);
+    }
+
+  mbuf_ro = mmap ((void *) addr, pagesize, PROT_READ,
+               MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+
+  if (mbuf_ro == MAP_FAILED)
+    {
+      fprintf (stderr, "mmap #1 failed: %s.\n", strerror (errno));
+      exit (1);
+    }
+
+  /* Write (and fill) the R/W region.  */
+  for (i = 0; i < sizeof (buf_rw); i++)
+    buf_rw[i] = 0x6b;
+
+  /* Compute an mmap address within buf_rw.  Complain if it's somewhere
+     else.  */
+  addr = ((unsigned long long) buf_rw + pagesize) & ~(pagesize - 1);
+
+  if (addr <= (unsigned long long) buf_rw
+      || addr >= (unsigned long long) buf_rw + sizeof (buf_rw))
+    {
+      fprintf (stderr, "Unable to compute a suitable address within buf_rw.\n");
+      exit (1);
+    }
+
+  mbuf_rw = mmap ((void *) addr, pagesize, PROT_READ,
+               MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+
+  if (mbuf_rw == MAP_FAILED)
+    {
+      fprintf (stderr, "mmap #2 failed: %s.\n", strerror (errno));
+      exit (1);
+    }
+
+  /* With correct ulimit, etc. this should cause a core dump.  */
+  abort ();
+}
-- 
2.26.2

Re: [PATCH v5 13/14] Add documentation for "maint print core-file-backed-mappings"

[Eli Zaretskii]

> Date: Tue, 21 Jul 2020 17:58:31 -0700
> From: Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org>
> 
> gdb/ChangeLog:
> 
> 	* NEWS (New commands): Mention new command
> 	"maintenance print core-file-backed-mappings".
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Maintenance Commands): Add documentation for
> 	new command "maintenance print core-file-backed-mappings".

This is OK, thanks.

Re: [PATCH v5 11/14] Adjust coredump-filter.exp to account for NT_FILE note handling

[Pedro Alves]

On 7/22/20 1:58 AM, Kevin Buettner via Gdb-patches wrote:

> +	set hide_binfile [standard_output_file "${testfile}.hide"]
> +	if { $should_fail == 1 } {
> +	    file rename -force $binfile $hide_binfile

Shouldn't this be instead:

  remote_exec host "mv -f $binfile $hide_binfile"

so that it works on remote host configurations?

> +	}
> +
>  	set core_loaded [gdb_core_cmd "$core" "load core"]
>  	if { $core_loaded == -1 } {
>  	    fail "loading $core"
> @@ -96,6 +107,7 @@ proc test_disasm { core address should_fail } {
>  	}
>  
>  	if { $should_fail == 1 } {
> +	    file rename -force $hide_binfile $binfile

Ditto.

Otherwise LGTM.

Pedro Alves

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Pedro Alves]

On 7/22/20 1:58 AM, Kevin Buettner via Gdb-patches wrote:
> This series fixes several core file related bugs.  The bug which
> started this work can be viewed here:
> 
>     https://sourceware.org/bugzilla/show_bug.cgi?id=25631
> 
> Other problems were found either during review or during development. 
> I discuss these in my commit log remarks.
> 
> This v5 series has only minor changes from v4.  Unless noted
> otherwise, these changes all address Pedro's concerns:
> 
> o Patch #5, Test ability to access unwritten-to mmap data in core file:
> 
>   - Add setup_xfail for non-Linux OSes.  (I spotted this while testing
>     on FreeBSD.)
> 
> o Patch #8, Use NT_FILE note section for reading core target memory:
> 
>   - Formatting fixes.
>   - Close bfd after failed bfd_check_format_check.
>   - Fix SECNAME leak.
> 
> o Patch #11, Adjust coredump-filter.exp to account for NT_FILE note handling:
> 
>   - Instead of XFAILing test, use Mihails Strasuns's suggestion
>     instead.
> 
> o Patch #12, Add new command "maint print core-file-backed-mappings":
> 
>   - Formatting fixes.
>   - Don't crash GDB with gdb_assert().
>   - Add comments (in both code and commit log) regarding utility
>     of this new command.
> 
> o Patch #14, New core file tests with mappings over existing program memory:
> 
>   - Formatting fixes.
>   - Add another "maint print core-file-backed-mappings" test requested
>     by Pedro in patch #12 review.  This one makes sure that we don't
>     crash GDB when NOT debugging a core file.
> 
> I believe that all patches not listed above have been approved. 
> Actually, I think that #5 is still approved too; I doubt that adding
> an XFAIL changes that.

I sent a minor comment to patch #11.  Everything else looks great to me.

Thanks for doing all this.  Great job.

Pedro Alves

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Kevin Buettner]

On Wed, 22 Jul 2020 19:58:59 +0100
Pedro Alves <pedro@palves.net> wrote:

> I sent a minor comment to patch #11.  Everything else looks great to me.

I've made the recommended changes and have pushed this series.

Thanks for all of the reviews!

Kevin

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Luis Machado]

Hi,

I'm seeing the following regressions on AArch64 Linux running Ubuntu 
18.04 (GCC 7).

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)

On a quick look, the test doesn't go according to what is expected.

--

(gdb) PASS: gdb.base/corefile.exp: print coremaker_ro
print func2::coremaker_local^M
No frame is currently executing in block func2.^M
(gdb) FAIL: gdb.base/corefile.exp: print func2::coremaker_local
...
(gdb) PASS: gdb.base/corefile.exp: $_exitcode is void
bt^M
#0  0x0000ffff80d1c4d8 in raise () from /lib/aarch64-linux-gnu/libc.so.6^M
Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
(gdb) FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
up^M
Initial frame selected; you cannot go up.^M
(gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp
...
(gdb) file gdb/testsuite/outputs/gdb.base/corefile/corefile^M
Load new symbol table from 
"gdb/testsuite/outputs/gdb.base/corefile/corefile"? (y or n) y^M
Reading symbols from gdb/testsuite/outputs/gdb.base/corefile/corefile...^M
(gdb) up^M
Initial frame selected; you cannot go up.^M
(gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)

--

On 7/21/20 9:58 PM, Kevin Buettner via Gdb-patches wrote:
> This series fixes several core file related bugs.  The bug which
> started this work can be viewed here:
> 
>      https://sourceware.org/bugzilla/show_bug.cgi?id=25631
> 
> Other problems were found either during review or during development.
> I discuss these in my commit log remarks.
> 
> This v5 series has only minor changes from v4.  Unless noted
> otherwise, these changes all address Pedro's concerns:
> 
> o Patch #5, Test ability to access unwritten-to mmap data in core file:
> 
>    - Add setup_xfail for non-Linux OSes.  (I spotted this while testing
>      on FreeBSD.)
> 
> o Patch #8, Use NT_FILE note section for reading core target memory:
> 
>    - Formatting fixes.
>    - Close bfd after failed bfd_check_format_check.
>    - Fix SECNAME leak.
> 
> o Patch #11, Adjust coredump-filter.exp to account for NT_FILE note handling:
> 
>    - Instead of XFAILing test, use Mihails Strasuns's suggestion
>      instead.
> 
> o Patch #12, Add new command "maint print core-file-backed-mappings":
> 
>    - Formatting fixes.
>    - Don't crash GDB with gdb_assert().
>    - Add comments (in both code and commit log) regarding utility
>      of this new command.
> 
> o Patch #14, New core file tests with mappings over existing program memory:
> 
>    - Formatting fixes.
>    - Add another "maint print core-file-backed-mappings" test requested
>      by Pedro in patch #12 review.  This one makes sure that we don't
>      crash GDB when NOT debugging a core file.
> 
> I believe that all patches not listed above have been approved.
> Actually, I think that #5 is still approved too; I doubt that adding
> an XFAIL changes that.
> 
> Kevin Buettner (14):
>    Remove hack for GDB which sets the section size to 0
>    Adjust corefile.exp test to show regression after bfd hack removal
>    section_table_xfer_memory: Replace section name with callback
>      predicate
>    Provide access to non SEC_HAS_CONTENTS core file sections
>    Test ability to access unwritten-to mmap data in core file
>    Update binary_get_section_contents to seek using section's file
>      position
>    Add new gdbarch method, read_core_file_mappings
>    Use NT_FILE note section for reading core target memory
>    Add test for accessing read-only mmapped data in a core file
>    gcore command: Place all file-backed mappings in NT_FILE note
>    Adjust coredump-filter.exp to account for NT_FILE note handling
>    Add new command "maint print core-file-backed-mappings"
>    Add documentation for "maint print core-file-backed-mappings"
>    New core file tests with mappings over existing program memory
> 
>   bfd/binary.c                               |  12 +-
>   bfd/elf.c                                  |   8 -
>   gdb/NEWS                                   |   4 +
>   gdb/arch-utils.c                           |  16 ++
>   gdb/arch-utils.h                           |  12 +
>   gdb/bfd-target.c                           |   3 +-
>   gdb/corelow.c                              | 264 ++++++++++++++++++++-
>   gdb/doc/gdb.texinfo                        |   8 +
>   gdb/exec.c                                 |   8 +-
>   gdb/exec.h                                 |  13 +-
>   gdb/gdbarch.c                              |  23 ++
>   gdb/gdbarch.h                              |   6 +
>   gdb/gdbarch.sh                             |   3 +
>   gdb/linux-tdep.c                           | 244 +++++++++++++------
>   gdb/target.c                               |  18 +-
>   gdb/testsuite/gdb.base/coredump-filter.exp |  18 +-
>   gdb/testsuite/gdb.base/corefile.exp        |  27 ++-
>   gdb/testsuite/gdb.base/corefile2.exp       | 185 +++++++++++++++
>   gdb/testsuite/gdb.base/coremaker.c         |  30 ++-
>   gdb/testsuite/gdb.base/coremaker2.c        | 150 ++++++++++++
>   20 files changed, 942 insertions(+), 110 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.base/corefile2.exp
>   create mode 100644 gdb/testsuite/gdb.base/coremaker2.c
> 

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Kevin Buettner]

On Wed, 22 Jul 2020 19:40:18 -0300
Luis Machado <luis.machado@linaro.org> wrote:

> I'm seeing the following regressions on AArch64 Linux running Ubuntu 
> 18.04 (GCC 7).
> 
> 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)
> 
> On a quick look, the test doesn't go according to what is expected.
> 
> --
> 
> (gdb) PASS: gdb.base/corefile.exp: print coremaker_ro
> print func2::coremaker_local^M
> No frame is currently executing in block func2.^M
> (gdb) FAIL: gdb.base/corefile.exp: print func2::coremaker_local
> ...
> (gdb) PASS: gdb.base/corefile.exp: $_exitcode is void
> bt^M
> #0  0x0000ffff80d1c4d8 in raise () from /lib/aarch64-linux-gnu/libc.so.6^M
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
> (gdb) FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
> up^M
> Initial frame selected; you cannot go up.^M
> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp
> ...
> (gdb) file gdb/testsuite/outputs/gdb.base/corefile/corefile^M
> Load new symbol table from 
> "gdb/testsuite/outputs/gdb.base/corefile/corefile"? (y or n) y^M
> Reading symbols from gdb/testsuite/outputs/gdb.base/corefile/corefile...^M
> (gdb) up^M
> Initial frame selected; you cannot go up.^M
> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)

Hi Luis,

I'm not seeing these failures on my Raspberry Pi 4 running aarch64
rawhide.  I have a spare Pi upon which I'll install Ubuntu 18.04.4.

In the interim, would it be possible for you to show me the same
sections of the log file from a test run using a GDB build prior to my
core file patches?

Thanks,

Kevin

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

On 7/23/20 12:49 AM, Kevin Buettner wrote:
> On Wed, 22 Jul 2020 19:40:18 -0300
> Luis Machado <luis.machado@linaro.org> wrote:
> 
>> I'm seeing the following regressions on AArch64 Linux running Ubuntu
>> 18.04 (GCC 7).
>>
>> 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)
>>
>> On a quick look, the test doesn't go according to what is expected.
>>
>> --
>>
>> (gdb) PASS: gdb.base/corefile.exp: print coremaker_ro
>> print func2::coremaker_local^M
>> No frame is currently executing in block func2.^M
>> (gdb) FAIL: gdb.base/corefile.exp: print func2::coremaker_local
>> ...
>> (gdb) PASS: gdb.base/corefile.exp: $_exitcode is void
>> bt^M
>> #0  0x0000ffff80d1c4d8 in raise () from /lib/aarch64-linux-gnu/libc.so.6^M
>> Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
>> (gdb) FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
>> up^M
>> Initial frame selected; you cannot go up.^M
>> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp
>> ...
>> (gdb) file gdb/testsuite/outputs/gdb.base/corefile/corefile^M
>> Load new symbol table from
>> "gdb/testsuite/outputs/gdb.base/corefile/corefile"? (y or n) y^M
>> Reading symbols from gdb/testsuite/outputs/gdb.base/corefile/corefile...^M
>> (gdb) up^M
>> Initial frame selected; you cannot go up.^M
>> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)
> 
> Hi Luis,
> 
> I'm not seeing these failures on my Raspberry Pi 4 running aarch64
> rawhide.  I have a spare Pi upon which I'll install Ubuntu 18.04.4.
> 
> In the interim, would it be possible for you to show me the same
> sections of the log file from a test run using a GDB build prior to my
> core file patches?

I've attached the complete log files for both runs. Hopefully that will 
be more useful.

Please let me know if you need anything else and I can fetch it.

[-- Attachment #2: corefile_logs.tar.bz2 --]
[-- Type: application/x-bzip, Size: 7999 bytes --]

Re: [PATCH v5 12/14] Add new command "maint print core-file-backed-mappings"

[Tom de Vries]

On 7/22/20 2:58 AM, Kevin Buettner via Gdb-patches wrote:
> +	   _("Print core file's file-backed mappings"),

$ gdb -batch -ex "help maintenance print core-file-backed-mappings"
Print core file's file-backed mappings

That apparently needs a terminating dot.  Filed
https://sourceware.org/bugzilla/show_bug.cgi?id=26294.

Thanks,
- Tom

Re: [PATCH v5 12/14] Add new command "maint print core-file-backed-mappings"

[Kevin Buettner]

On Thu, 23 Jul 2020 15:08:27 +0200
Tom de Vries <tdevries@suse.de> wrote:

> On 7/22/20 2:58 AM, Kevin Buettner via Gdb-patches wrote:
> > +	   _("Print core file's file-backed mappings"),  
> 
> $ gdb -batch -ex "help maintenance print core-file-backed-mappings"
> Print core file's file-backed mappings
> 
> That apparently needs a terminating dot.  Filed
> https://sourceware.org/bugzilla/show_bug.cgi?id=26294.

Should be fixed now...

https://sourceware.org/pipermail/gdb-patches/2020-July/170813.html

Kevin

Re: [PATCH v5 00/14] Fix BZ 25631 - core file memory access problem

[Kevin Buettner]

On Thu, 23 Jul 2020 07:37:42 -0300
Luis Machado <luis.machado@linaro.org> wrote:

> On 7/23/20 12:49 AM, Kevin Buettner wrote:
> > On Wed, 22 Jul 2020 19:40:18 -0300
> > Luis Machado <luis.machado@linaro.org> wrote:
> >   
> >> I'm seeing the following regressions on AArch64 Linux running Ubuntu
> >> 18.04 (GCC 7).
> >>
> >> 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)
> >>
> >> On a quick look, the test doesn't go according to what is expected.
> >>
> >> --
> >>
> >> (gdb) PASS: gdb.base/corefile.exp: print coremaker_ro
> >> print func2::coremaker_local^M
> >> No frame is currently executing in block func2.^M
> >> (gdb) FAIL: gdb.base/corefile.exp: print func2::coremaker_local
> >> ...
> >> (gdb) PASS: gdb.base/corefile.exp: $_exitcode is void
> >> bt^M
> >> #0  0x0000ffff80d1c4d8 in raise () from /lib/aarch64-linux-gnu/libc.so.6^M
> >> Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
> >> (gdb) FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
> >> up^M
> >> Initial frame selected; you cannot go up.^M
> >> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp
> >> ...
> >> (gdb) file gdb/testsuite/outputs/gdb.base/corefile/corefile^M
> >> Load new symbol table from
> >> "gdb/testsuite/outputs/gdb.base/corefile/corefile"? (y or n) y^M
> >> Reading symbols from gdb/testsuite/outputs/gdb.base/corefile/corefile...^M
> >> (gdb) up^M
> >> Initial frame selected; you cannot go up.^M
> >> (gdb) FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)  
> > 
> > Hi Luis,
> > 
> > I'm not seeing these failures on my Raspberry Pi 4 running aarch64
> > rawhide.  I have a spare Pi upon which I'll install Ubuntu 18.04.4.
> > 
> > In the interim, would it be possible for you to show me the same
> > sections of the log file from a test run using a GDB build prior to my
> > core file patches?  
> 
> I've attached the complete log files for both runs. Hopefully that will 
> be more useful.
> 
> Please let me know if you need anything else and I can fetch it.

Thanks for sending the log files.  One of the things I found puzzling
were these messages in after.log:

(gdb) core-file /home/user/work/build/binutils-gdb-master/gdb/testsuite/outputs/gdb.base/corefile/corefile.core
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing
warning: Can't open file (null) during file-backed mapping note processing

Something is going terribly wrong!

I've installed Ubuntu 18.04.4 on a Raspberry Pi 3 B+.

So far, I haven't been able to reproduce the failures that you're seeing.

kev@ubuntu:/ironwood1/sourceware-git/u18.04-rpi3-master/bld/gdb$ cat /etc/issue
Ubuntu 18.04.4 LTS \n \l

kev@ubuntu:/ironwood1/sourceware-git/u18.04-rpi3-master/bld/gdb$ uname -a
Linux ubuntu 5.3.0-1028-raspi2 #30~18.04.2-Ubuntu SMP Fri Jun 19
05:12:46 UTC 2020 aarch64 aarch64 aarch64 GNU/Linux

kev@ubuntu:/ironwood1/sourceware-git/u18.04-rpi3-master/bld/gdb$ make
check TESTS="gdb.base/corefile.exp"
...
		=== gdb Summary ===

# of expected passes		30

I'd like to see the following...

1) The output of "uname -a".

2) The output of "cat /etc/issue".

3) The output of "cat /proc/self/coredump_filter".  (Is there a better
way to know what the global filter is?)

4) The before / after contents of testsuite/outputs/gdb.base/corefile .

Thanks,

Kevin