public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <sergiodj@redhat.com>
To: GDB Patches <gdb-patches@sourceware.org>
Cc: Pedro Alves <palves@redhat.com>,
	       Sergio Durigan Junior <sergiodj@redhat.com>
Subject: [PATCH 2/4] Update gcore_create_callback to better handle different states of mappings
Date: Wed, 18 Mar 2015 19:39:00 -0000	[thread overview]
Message-ID: <1426707523-6499-3-git-send-email-sergiodj@redhat.com> (raw)
In-Reply-To: <1426707523-6499-1-git-send-email-sergiodj@redhat.com>

This patch updates gdb/gcore.c's gcore_create_callback function and
changes its logic to improve the handling of different states of
memory mappings.

One of the major modifications is that mappings marked as UNMODIFIED
are now completely ignored by the function (i.e., not even the segment
headers are created anymore).  This is now possible because of the
introduction of the new UNKNOWN state (see below).  We now know that
when the mapping is UNMODIFIED, it means that the user has either
chose to ignore it via /proc/PID/coredump_filter, or that it is marked
as VM_DONTDUMP (and therefore should not be dumped anyway).  This is
what the Linux kernel does, too.

The new UNKNOWN state is being used to identify situations when we
don't really know whether the mapping should be dumped or not.  Based
on that, we run an existing heuristic responsible for deciding if we
should include the mapping's contents or not.

One last thing worth mentioning is that this check:

  if (read == 0 && write == 0 && exec == 0
      && modified_state == MEMORY_MAPPING_UNMODIFIED)

has been simplified to:

  if (read == 0)

This is because if the mapping has not 'read' permission set, it does
not make sense to include its contents in the corefile (GDB would
actually not be allowed to do that).  Therefore, we just create a
segment header for it.  The Linux kernel differs from GDB here because
it actually include the contents of this mapping in the corefile, but
this is because it can read its contents.

Changes from v2:

  - Return immediately if modified_state == MEMORY_MAPPING_UNMODIFIED

  - Improve comments explaining the case above, and also the case when
    "read == 0".

gdb/ChangeLog:
2015-03-18  Sergio Durigan Junior  <sergiodj@redhat.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Oleg Nesterov  <oleg@redhat.com>

	PR corefiles/16092
	* gcore.c (gcore_create_callback): Update code to handle the case
	when 'modified_state == MEMORY_MAPPING_UNMODIFIED'.  Simplify
	condition used to decide when to create only a segment header for
	the mapping.  Improve check to decide when to run a heuristic to
	decide whether to dump the mapping's contents.
---
 gdb/gcore.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/gdb/gcore.c b/gdb/gcore.c
index 751ddac..8dfcc02 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -422,23 +422,34 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size, int read,
   asection *osec;
   flagword flags = SEC_ALLOC | SEC_HAS_CONTENTS | SEC_LOAD;
 
-  /* If the memory segment has no permissions set, ignore it, otherwise
-     when we later try to access it for read/write, we'll get an error
-     or jam the kernel.  */
-  if (read == 0 && write == 0 && exec == 0
-      && modified_state == MEMORY_MAPPING_UNMODIFIED)
+  if (modified_state == MEMORY_MAPPING_UNMODIFIED)
     {
-      if (info_verbose)
-        {
-          fprintf_filtered (gdb_stdout, "Ignore segment, %s bytes at %s\n",
-                            plongest (size), paddress (target_gdbarch (), vaddr));
-        }
-
+      /* When the memory mapping is marked as unmodified, this means
+	 that it should not be included in the coredump file (either
+	 because it was marked as VM_DONTDUMP, or because the user
+	 explicitly chose to ignore it using the
+	 /proc/PID/coredump_filter mechanism).
+
+	 We could in theory create a section header for it (i.e., mark
+	 it as '~(SEC_LOAD | SEC_HAS_CONTENTS)', just like we do when
+	 the mapping does not have the 'read' permission set), but the
+	 Linux kernel itself ignores these mappings, and so do we.  */
       return 0;
     }
-
-  if (write == 0 && modified_state == MEMORY_MAPPING_UNMODIFIED
-      && !solib_keep_data_in_core (vaddr, size))
+  else if (read == 0)
+    {
+      /* If the memory segment has no read permission set, then we have to
+	 generate a segment header for it, but without contents (i.e.,
+	 FileSiz = 0), otherwise when we later try to access it for
+	 read/write, we'll get an error or jam the kernel.
+
+	 The Linux kernel differs from GDB in this point because it
+	 can actually read this mapping, so it dumps this mapping's
+	 contents in the corefile.  */
+      flags &= ~(SEC_LOAD | SEC_HAS_CONTENTS);
+    }
+  else if (write == 0 && modified_state == MEMORY_MAPPING_UNKNOWN_STATE
+	   && !solib_keep_data_in_core (vaddr, size))
     {
       /* See if this region of memory lies inside a known file on disk.
 	 If so, we can avoid copying its contents by clearing SEC_LOAD.  */
-- 
1.9.3

  parent reply	other threads:[~2015-03-18 19:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18 19:39 [PATCH v3 0/4] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902) Sergio Durigan Junior
2015-03-18 19:39 ` [PATCH 1/4] Improve identification of memory mappings Sergio Durigan Junior
2015-03-19 10:39   ` Pedro Alves
2015-03-19 23:07     ` Sergio Durigan Junior
2015-03-20 19:11       ` Pedro Alves
2015-03-20 20:14         ` Sergio Durigan Junior
2015-03-18 19:39 ` [PATCH 3/4] Implement support for checking /proc/PID/coredump_filter Sergio Durigan Junior
2015-03-19 10:41   ` Pedro Alves
2015-03-19 23:09     ` Sergio Durigan Junior
2015-03-18 19:39 ` Sergio Durigan Junior [this message]
2015-03-19 10:39   ` [PATCH 2/4] Update gcore_create_callback to better handle different states of mappings Pedro Alves
2015-03-19 23:08     ` Sergio Durigan Junior
2015-03-18 19:44 ` [PATCH 4/4] Documentation and testcase Sergio Durigan Junior
2015-03-18 20:08   ` Eli Zaretskii
2015-03-18 20:18     ` Sergio Durigan Junior

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=1426707523-6499-3-git-send-email-sergiodj@redhat.com \
    --to=sergiodj@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /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).