public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v3 4/9] Provide access to non SEC_HAS_CONTENTS core file sections
Date: Wed, 17 Jun 2020 21:08:19 -0700	[thread overview]
Message-ID: <20200618040824.3405657-5-kevinb@redhat.com> (raw)
In-Reply-To: <20200618040824.3405657-1-kevinb@redhat.com>

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 e3bd0bc452..17e862aee9 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;
@@ -613,12 +613,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 516fec78c7..844ea20c4f 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -981,8 +981,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


  parent reply	other threads:[~2020-06-18  4:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-18  4:08 [PATCH v3 0/9] Fix BZ 25631 - core file memory access problem Kevin Buettner
2020-06-18  4:08 ` [PATCH v3 1/9] Remove hack for GDB which sets the section size to 0 Kevin Buettner
2020-06-18  4:08 ` [PATCH v3 2/9] Adjust corefile.exp test to show regression after bfd hack removal Kevin Buettner
2020-06-18  4:08 ` [PATCH v3 3/9] section_table_xfer_memory: Replace section name with callback predicate Kevin Buettner
2020-06-18  4:08 ` Kevin Buettner [this message]
2020-06-18  4:08 ` [PATCH v3 5/9] Test ability to access unwritten-to mmap data in core file Kevin Buettner
2020-06-25 14:32   ` Pedro Alves
2020-06-18  4:08 ` [PATCH v3 6/9] Update binary_get_section_contents to seek using section's file position Kevin Buettner
2020-06-18 10:25   ` Nick Clifton
2020-06-18  4:08 ` [PATCH v3 7/9] Use NT_FILE note section for reading core target memory Kevin Buettner
2020-06-25 14:11   ` Pedro Alves
2020-06-18  4:08 ` [PATCH v3 8/9] Add test for accessing read-only mmapped data in a core file Kevin Buettner
2020-06-18  4:08 ` [PATCH v3 9/9] New core file tests with mappings over existing program memory Kevin Buettner
2020-06-24  2:44   ` Kevin Buettner
2020-06-25 14:12     ` Pedro Alves
2020-06-25 14:15 ` [PATCH v3 0/9] Fix BZ 25631 - core file memory access problem Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200618040824.3405657-5-kevinb@redhat.com \
    --to=kevinb@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).