public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902)
@ 2015-03-05  3:48 Sergio Durigan Junior
  2015-03-05 15:48 ` Jan Kratochvil
                   ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Sergio Durigan Junior @ 2015-03-05  3:48 UTC (permalink / raw)
  To: GDB Patches; +Cc: Jan Kratochvil, Pedro Alves, Oleg Nesterov

Hello,

I have been working on this patch for quite some time, with some
interruptions here and there, but now I think it is ready to be
submitted and pushed upstream.

First of all, thanks to Jan Kratochvil for catching mistakes and to Oleg
Nesterov for helping us understanding this marvelous yet obscure world
of the Linux kernel memory mapping scheme.

This patch, as the subject says, extends GDB so that it is able to use
the contents of the file /proc/PID/coredump_filter when generating a
corefile.  This file contains a bit mask that is a representation of the
different types of memory mappings in the Linux kernel; the user can
choose to dump or not dump a certain type of memory mapping by
enabling/disabling the respective bit in the bit mask.  Currently, here
is what is supported:

  bit 0  Dump anonymous private mappings.
  bit 1  Dump anonymous shared mappings.
  bit 2  Dump file-backed private mappings.
  bit 3  Dump file-backed shared mappings.
  bit 4 (since Linux 2.6.24)
         Dump ELF headers.
  bit 5 (since Linux 2.6.28)
         Dump private huge pages.
  bit 6 (since Linux 2.6.28)
         Dump shared huge pages.

(This table has been taken from core(5), but you can also read about it
on Documentation/filesystems/proc.txt inside the Linux kernel source
tree).

The default value for this file, used by the Linux kernel, is 0x33,
which means that bits 0, 1 and 4 are enabled.  This is also the default
for GDB implemented in this patch, FWIW.

Well, reading the file is obviously trivial.  The hard part, mind you,
is how to determine the types of the memory mappings.  For that, I
extended the code of gdb/linux-tdep.c:linux_find_memory_regions_full and
made it rely *much more* on the information gathered from
/proc/<PID>/smaps.  This file contains a "verbose dump" of the
inferior's memory mappings, and we were not using as much information as
we could from it.  If you want to read more about this file, take a look
at the proc(5) manpage (I will also write a blog post soon about
everything I had to learn to get this patch done, and when I it is ready
I will post it here).

With Oleg's help, we could improve the current algorithm for determining
whether a memory mapping is anonymous/file-backed, private/shared.  GDB
now also respects the MADV_DONTDUMP flag and does not dump the memory
mapping marked as so, and won't try to dump "[vsyscall]" or "[vdso]"
mappings as before (just like the Linux kernel).

In a nutshell, what the new code is doing is:

- If the mapping is associated to a file whose name ends with "
  (deleted)", or if the file is "/dev/zero", or if it is "/SYSV%08x"
  (shared memory), or if there is no file associated with it, or if the
  AnonHugePages: or the Anonymous: fields in the /proc/PID/smaps have
  contents, then GDB considers this mapping to be anonymous.  Otherwise,
  GDB considers this mapping to be a file-backed mapping (because there
  will be a file associated with it).

  It is worth mentioning that, from all those checks described above,
  the most fragile is the one to see if the file name ends with "
  (deleted)".  This does not necessarily mean that the mapping is
  anonymous, because the deleted file associated with the mapping may
  have been a hard link to another file, for example.  The Linux kernel
  checks to see if "i_nlink == 0", but GDB cannot easily do this check.
  Therefore, we made a compromise here, and we assume that if the file
  name ends with " (deleted)", then the mapping is indeed anonymous.
  FWIW, this is something the Linux kernel could do better: expose this
  information in a more direct way.

- If we see the flag "sh" in the VmFlags: field (in /proc/PID/smaps),
  then certainly the memory mapping is shared (VM_SHARED).  If we have
  access to the VmFlags, and we don't see the "sh" there, then certainly
  the mapping is private.  However, older Linus kernels do not have the
  VmFlags field; in that case, we use another heuristic: if we see 'p'
  in the permission flags, then we assume that the mapping is private,
  even though the presence of the 's' flag there would mean VM_MAYSHARE,
  which means the mapping could still be private.  This should work OK
  enough, however.

As a side effect of this patch, gdb/gcore.c:gcore_create_callback is
also smarter when dumping mappings in the corefile and generating their
section headers.  Before, when we did not know if a memory mapping was
modified or not, we were passing '1' (true) to this function, which
would just assume that the mapping was modified and would be dumped.
Now, we are passing a new value, which represents "I don't know", and
will make the function use an existing heuristic to determine if we
should indeed dump this mapping, or just ignore it.

Finally, it is worth mentioning that I added a new command, 'set
use-coredump-filter on/off'.  When it is 'on', it will read the
coredump_filter' file (if it exists) and use its value; otherwise, it
will use the default value mentioned above (0x33) to decide which memory
mappings to dump.

I am submitting a documentation patch and a testsuite to exercise this
new feature.  I ran a regression test on a Fedora 20 machine (x86_64 and
native-gdbserver), and nothing was detected.

OK to apply?

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

	PR corefiles/16092
	* common/common-defs.h (enum memory_mapping_state): New enum.
	* defs.h (find_memory_region_ftype): Remove 'int modified'
	parameter, replacing by 'enum memory_mapping_state state'.
	* gcore.c (gcore_create_callback): Likewise.  Change 'if/else'
	statements and improve the logic of deciding when to ignore a
	memory mapping.
	(objfile_find_memory_regions): Passing
	'MEMORY_MAPPING_UNKNOWN_STATE' or 'MEMORY_MAPPING_MODIFIED' when
	needed to 'func' callback, instead of saying the memory mapping
	was modified even without knowing it.
	* gnu-nat.c (gnu_find_memory_regions): Likewise.
	* linux-tdep.c: Include 'gdbcmd.h' and 'gdb_regex.h'.
	New enum identifying the various options of the coredump_filter
	file.
	(struct smaps_vmflags): New struct.
	(use_coredump_filter): New variable.
	(decode_vmflags): New function.
	(mapping_is_anonymous_p): Likewise.
	(dump_mapping_p): Likewise.
	(linux_find_memory_region_ftype): Remove 'int modified' parameter,
	replacing by 'enum memory_mapping_state state'.
	(linux_find_memory_regions_full): New variables
	'coredumpfilter_name', 'coredumpfilterdata', 'pid',
	'filterflags'.  Read /proc/<PID>/smaps file; improve parsing of
	its information.  Implement memory mapping filtering based on its
	contents.
	(linux_find_memory_regions_thunk): Remove 'int modified'
	parameter, replacing by 'enum memory_mapping_state state'.
	(linux_make_mappings_callback): Likewise.
	(find_mapping_size): Likewise.
	(show_use_coredump_filter): New function.
	(_initialize_linux_tdep): New command 'set use-coredump-filter'.
	* procfs.c (find_memory_regions_callback): Passing
	'MEMORY_MAPPING_UNKNOWN_STATE' when needed to 'func' callback,
	instead of saying the memory mapping was modified even without
	knowing it.

gdb/doc/ChangeLog:
2015-03-04  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR corefiles/16092
	* gdb.texinfo (gcore): Mention new command 'set
	use-coredump-filter'.
	(set use-coredump-filter): Document new command.

gdb/testsuite/ChangeLog:
2015-03-04  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR corefiles/16092
	* gdb.base/coredump-filter.c: New file.
	* gdb.base/coredump-filter.exp: Likewise.

diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index 62d9de5..01b05f5 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -60,4 +60,14 @@
 # define EXTERN_C_POP
 #endif
 
+/* Enum used to inform the state of a memory mapping.  This is used in
+   functions implementing find_memory_region_ftype.  */
+
+enum memory_mapping_state
+  {
+    MEMORY_MAPPING_MODIFIED,
+    MEMORY_MAPPING_UNMODIFIED,
+    MEMORY_MAPPING_UNKNOWN_STATE,
+  };
+
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/defs.h b/gdb/defs.h
index 72512f6..4829b62 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -338,7 +338,8 @@ extern void init_source_path (void);
 
 typedef int (*find_memory_region_ftype) (CORE_ADDR addr, unsigned long size,
 					 int read, int write, int exec,
-					 int modified, void *data);
+					 enum memory_mapping_state state,
+					 void *data);
 
 /* * Possible lvalue types.  Like enum language, this should be in
    value.h, but needs to be here for the same reason.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4b76ce9..e575eae 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10952,6 +10952,67 @@ specified, the file name defaults to @file{core.@var{pid}}, where
 
 Note that this command is implemented only for some systems (as of
 this writing, @sc{gnu}/Linux, FreeBSD, Solaris, and S390).
+
+On @sc{gnu}/Linux, this command can take into account the value of the
+file @file{/proc/@var{pid}/coredump_filter} when generating the core
+dump (@pxref{set use-coredump-filter}).
+
+@kindex set use-coredump-filter
+@anchor{set use-coredump-filter}
+@item set use-coredump-filter on
+@itemx set use-coredump-filter off
+Enable or disable the use of the file
+@file{/proc/@var{pid}/coredump_filter} when generating core dump
+files.  This file is used by the Linux kernel to decide what types of
+memory mappings will be dumped or ignored when generating a core dump
+file.
+
+To make use of this feature, you have to write in the
+@file{/proc/@var{pid}/coredump_filter} file a value, in hexadecimal,
+which is a bit mask representing the memory mapping types.  If a bit
+is set in the bit mask, then the memory mappings of the corresponding
+types will be dumped; otherwise, they will be ignored.  The bits in
+this bit mask have the following meanings:
+
+@table @code
+@item bit 0
+Dump anonymous private mappings.
+@item bit 1
+Dump anonymous shared mappings.
+@item bit 2
+Dump file-backed private mappings.
+@item bit 3
+Dump file-backed shared mappings.
+@item bit 4
+(since Linux 2.6.24)
+Dump ELF headers. (@value{GDBN} does not take this bit into account)
+@item bit 5
+(since Linux 2.6.28)
+Dump private huge pages.
+@item bit 6
+(since Linux 2.6.28)
+Dump shared huge pages.
+@end table
+
+For example, supposing that the @code{pid} of the program being
+debugging is @code{1234}, if you wanted to dump everything except the
+anonymous private and the file-backed shared mappings, you would do:
+
+@smallexample
+$ echo 0x76 > /proc/1234/coredump_filter
+@end smallexample
+
+For more documentation about how to use the @file{coredump_filter}
+file, see the manpage of @code{proc(5)}.
+
+By default, this option is @code{on}.  If this option is turned
+@code{off}, @value{GDBN} will not read the @file{coredump_filter}
+file, but it uses the same default value as the Linux kernel in order
+to decide which pages will be dumped in the core dump file.  This
+value currently is @code{0x33}, which means that the bits @code{0}
+(anonymous private mappings), @code{1} (anonymous shared mappings) and
+@code{4} (ELF headers) are active.  This will cause these memory
+mappings to be dumped automatically.
 @end table
 
 @node Character Sets
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 1ebff2a..9edcf40 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -408,27 +408,22 @@ make_output_phdrs (bfd *obfd, asection *osec, void *ignored)
 
 static int
 gcore_create_callback (CORE_ADDR vaddr, unsigned long size, int read,
-		       int write, int exec, int modified, void *data)
+		       int write, int exec, enum memory_mapping_state state,
+		       void *data)
 {
   bfd *obfd = data;
   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 == 0)
-    {
-      if (info_verbose)
-        {
-          fprintf_filtered (gdb_stdout, "Ignore segment, %s bytes at %s\n",
-                            plongest (size), paddress (target_gdbarch (), vaddr));
-        }
-
-      return 0;
-    }
-
-  if (write == 0 && modified == 0 && !solib_keep_data_in_core (vaddr, size))
+  /* If the memory segment has no read permission set, or if it has
+     been marked as unmodified, 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.  */
+  if (read == 0 || state == MEMORY_MAPPING_UNMODIFIED)
+    flags &= ~(SEC_LOAD | SEC_HAS_CONTENTS);
+  else if (write == 0 && 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.  */
@@ -521,7 +516,8 @@ objfile_find_memory_regions (struct target_ops *self,
 			 1, /* All sections will be readable.  */
 			 (flags & SEC_READONLY) == 0, /* Writable.  */
 			 (flags & SEC_CODE) != 0, /* Executable.  */
-			 1, /* MODIFIED is unknown, pass it as true.  */
+			 MEMORY_MAPPING_UNKNOWN_STATE, /* MODIFIED is
+							 unknown.  */
 			 obfd);
 	  if (ret != 0)
 	    return ret;
@@ -534,7 +530,7 @@ objfile_find_memory_regions (struct target_ops *self,
 	     1, /* Stack section will be readable.  */
 	     1, /* Stack section will be writable.  */
 	     0, /* Stack section will not be executable.  */
-	     1, /* Stack section will be modified.  */
+	     MEMORY_MAPPING_MODIFIED, /* Stack section will be modified.  */
 	     obfd);
 
   /* Make a heap segment.  */
@@ -543,7 +539,7 @@ objfile_find_memory_regions (struct target_ops *self,
 	     1, /* Heap section will be readable.  */
 	     1, /* Heap section will be writable.  */
 	     0, /* Heap section will not be executable.  */
-	     1, /* Heap section will be modified.  */
+	     MEMORY_MAPPING_MODIFIED, /* Heap section will be modified.  */
 	     obfd);
 
   return 0;
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index d830773..60612a7 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2611,7 +2611,7 @@ gnu_find_memory_regions (struct target_ops *self,
 		     last_protection & VM_PROT_READ,
 		     last_protection & VM_PROT_WRITE,
 		     last_protection & VM_PROT_EXECUTE,
-		     1, /* MODIFIED is unknown, pass it as true.  */
+		     MEMORY_MAPPING_UNKNOWN_STATE, /* MODIFIED is unknown.  */
 		     data);
 	  last_region_address = region_address;
 	  last_region_end = region_address += region_length;
@@ -2625,7 +2625,7 @@ gnu_find_memory_regions (struct target_ops *self,
 	     last_protection & VM_PROT_READ,
 	     last_protection & VM_PROT_WRITE,
 	     last_protection & VM_PROT_EXECUTE,
-	     1, /* MODIFIED is unknown, pass it as true.  */
+	     MEMORY_MAPPING_UNKNOWN_STATE, /* MODIFIED is unknown.  */
 	     data);
 
   return 0;
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d9884f3..2225b81 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -35,9 +35,58 @@
 #include "observer.h"
 #include "objfiles.h"
 #include "infcall.h"
+#include "gdbcmd.h"
+#include "gdb_regex.h"
 
 #include <ctype.h>
 
+/* This enum represents the values that the user can choose when
+   informing the Linux kernel about which memory mappings will be
+   dumped in a corefile.  They are described in the file
+   Documentation/filesystems/proc.txt, inside the Linux kernel
+   tree.  */
+
+enum
+  {
+    COREFILTER_ANON_PRIVATE = 1 << 0,
+    COREFILTER_ANON_SHARED = 1 << 1,
+    COREFILTER_MAPPED_PRIVATE = 1 << 2,
+    COREFILTER_MAPPED_SHARED = 1 << 3,
+    COREFILTER_ELF_HEADERS = 1 << 4,
+    COREFILTER_HUGETLB_PRIVATE = 1 << 5,
+    COREFILTER_HUGETLB_SHARED = 1 << 6,
+  };
+
+struct smaps_vmflags
+  {
+    /* Zero if this structure has not been initialized yet.  It
+       probably means that the Linux kernel being used does not emit
+       the "VmFlags:" field on "/proc/PID/smaps".  */
+
+    unsigned int initialized_p : 1;
+
+    /* Memory mapped I/O area (VM_IO, "io").  */
+
+    unsigned int io_page : 1;
+
+    /* Area uses huge TLB pages (VM_HUGETLB, "ht").  */
+
+    unsigned int uses_huge_tlb : 1;
+
+    /* Do not include this memory region on the coredump (VM_DONTDUMP, "dd").  */
+
+    unsigned int exclude_coredump : 1;
+
+    /* Is this a MAP_SHARED mapping (VM_SHARED, "sh").  */
+
+    unsigned int shared_mapping : 1;
+  };
+
+/* Whether to take the /proc/PID/coredump_filter into account when
+   generating a corefile.  */
+
+static int use_coredump_filter = 1;
+
 /* This enum represents the signals' numbers on a generic architecture
    running the Linux kernel.  The definition of "generic" comes from
    the file <include/uapi/asm-generic/signal.h>, from the Linux kernel
@@ -381,6 +430,159 @@ read_mapping (const char *line,
   *filename = p;
 }
 
+/* Helper function to decode the "VmFlags" field in /proc/PID/smaps.
+
+   This function was based on the documentation found on
+   <Documentation/filesystems/proc.txt>, on the Linux kernel.
+
+   Linux kernels before commit
+   834f82e2aa9a8ede94b17b656329f850c1471514 do not have this field on
+   smaps.  */
+
+static void
+decode_vmflags (char *p, struct smaps_vmflags *v)
+{
+  char *saveptr;
+  char *s;
+
+  v->initialized_p = 1;
+  p = skip_to_space (p);
+  p = skip_spaces (p);
+
+  for (s = strtok_r (p, " ", &saveptr);
+       s != NULL;
+       s = strtok_r (NULL, " ", &saveptr))
+    {
+      if (strcmp (s, "io") == 0)
+	v->io_page = 1;
+      else if (strcmp (s, "ht") == 0)
+	v->uses_huge_tlb = 1;
+      else if (strcmp (s, "dd") == 0)
+	v->exclude_coredump = 1;
+      else if (strcmp (s, "sh") == 0)
+	v->shared_mapping = 1;
+    }
+}
+
+/* Return 1 if the memory mapping is anonymous, 0 otherwise.
+
+   FILENAME is the name of the file present in the first line of the
+   memory mapping, in the "/proc/PID/smaps" output.  For example, if
+   the first line is:
+
+   7fd0ca877000-7fd0d0da0000 r--p 00000000 fd:02 2100770   /path/to/file
+
+   Then FILENAME will be "/path/to/file".  */
+
+static int
+mapping_is_anonymous_p (const char *filename)
+{
+  static regex_t dev_zero_regex, shmem_file_regex, file_deleted_regex;
+  static int init_regex_p = 0;
+
+  if (!init_regex_p)
+    {
+      struct cleanup *c = make_cleanup (null_cleanup, NULL);
+
+      init_regex_p = 1;
+      compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match /dev/zero "
+			     "filename"));
+      compile_rx_or_error (&shmem_file_regex,
+			   "^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match shmem "
+			     "filenames"));
+      /* FILE_DELETED_REGEX is a heuristic we use to try to mimic the
+	 Linux kernel's 'n_link == 0' code, which is responsible to
+	 decide if it is dealing with a 'MAP_SHARED | MAP_ANONYMOUS'
+	 mapping.  In other words, if FILE_DELETED_REGEX matches, it
+	 does not necessarily mean that we are dealing with an
+	 anonymous shared mapping.  However, there is no easy way to
+	 detect this currently, so this is the best approximation we
+	 have.
+
+	 As a result, GDB will dump readonly pages of deleted
+	 executables when using the default value of coredump_filter
+	 (0x33), while the Linux kernel will not dump those pages.
+	 But we can live with that.  */
+      compile_rx_or_error (&file_deleted_regex, " (deleted)$",
+			   _("Could not compile regex to match "
+			     "'<file> (deleted)'"));
+      /* We will never release these regexes, so just discard the
+	 cleanups.  */
+      discard_cleanups (c);
+    }
+
+  if (*filename == '\0'
+      || regexec (&dev_zero_regex, filename, 0, NULL, 0) == 0
+      || regexec (&shmem_file_regex, filename, 0, NULL, 0) == 0
+      || regexec (&file_deleted_regex, filename, 0, NULL, 0) == 0)
+    return 1;
+
+  return 0;
+}
+
+/* Return 0 if the memory mapping (which is related to FILTERFLAGS, V,
+   MAYBE_PRIVATE_P, and MAPPING_ANONYMOUS_P) should not be dumped, or
+   greater than 0 if it should.  */
+
+static int
+dump_mapping_p (unsigned int filterflags, const struct smaps_vmflags *v,
+		int maybe_private_p, int mapping_anon_p, const char *filename)
+{
+  /* Initially, we trust in what we received from outside.  This value
+     may not be very precise (i.e., it was probably gathered from the
+     permission line in the /proc/PID/smaps list, which actually
+     refers to VM_MAYSHARE, and not VM_SHARED), but it is what we have
+     for now.  */
+  int private_p = maybe_private_p;
+
+  /* We always dump vDSO and vsyscall mappings.  */
+  if (strcmp ("[vdso]", filename) == 0
+      || strcmp ("[vsyscall]", filename) == 0)
+    return 1;
+
+  if (v->initialized_p)
+    {
+      /* We never dump I/O mappings.  */
+      if (v->io_page)
+	return 0;
+
+      /* Check if we should exclude this mapping.  */
+      if (v->exclude_coredump)
+	return 0;
+
+      /* Updating our notion of whether this mapping is shared or
+	 private based on a trustworthy value.  */
+      private_p = !v->shared_mapping;
+
+      /* HugeTLB checking.  */
+      if (v->uses_huge_tlb)
+	{
+	  if ((private_p && (filterflags & COREFILTER_HUGETLB_PRIVATE))
+	      || (!private_p && (filterflags & COREFILTER_HUGETLB_SHARED)))
+	    return 1;
+
+	  return 0;
+	}
+    }
+
+  if (private_p)
+    {
+      if (mapping_anon_p)
+	return (filterflags & COREFILTER_ANON_PRIVATE) != 0;
+      else
+	return (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
+    }
+  else
+    {
+      if (mapping_anon_p)
+	return (filterflags & COREFILTER_ANON_SHARED) != 0;
+      else
+	return (filterflags & COREFILTER_MAPPED_SHARED) != 0;
+    }
+}
+
 /* Implement the "info proc" command.  */
 
 static void
@@ -807,7 +1009,8 @@ linux_core_info_proc (struct gdbarch *gdbarch, const char *args,
 typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
 					    ULONGEST offset, ULONGEST inode,
 					    int read, int write,
-					    int exec, int modified,
+					    int exec,
+					    enum memory_mapping_state state,
 					    const char *filename,
 					    void *data);
 
@@ -819,48 +1022,84 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 				void *obfd)
 {
   char mapsfilename[100];
-  char *data;
+  char coredumpfilter_name[100];
+  char *data, *coredumpfilterdata;
+  pid_t pid;
+  /* Default dump behavior of coredump_filter (0x33), according to
+     Documentation/filesystems/proc.txt from the Linux kernel
+     tree.  */
+  unsigned int filterflags = (COREFILTER_ANON_PRIVATE
+			      | COREFILTER_ANON_SHARED
+			      | COREFILTER_ELF_HEADERS
+			      | COREFILTER_HUGETLB_PRIVATE);
 
   /* We need to know the real target PID to access /proc.  */
   if (current_inferior ()->fake_pid_p)
     return 1;
 
-  xsnprintf (mapsfilename, sizeof mapsfilename,
-	     "/proc/%d/smaps", current_inferior ()->pid);
+  pid = current_inferior ()->pid;
+
+  if (use_coredump_filter)
+    {
+      xsnprintf (coredumpfilter_name, sizeof (coredumpfilter_name),
+		 "/proc/%d/coredump_filter", pid);
+      coredumpfilterdata = target_fileio_read_stralloc (coredumpfilter_name);
+      if (coredumpfilterdata != NULL)
+	{
+	  sscanf (coredumpfilterdata, "%x", &filterflags);
+	  xfree (coredumpfilterdata);
+	}
+    }
+
+  xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/smaps", pid);
   data = target_fileio_read_stralloc (mapsfilename);
   if (data == NULL)
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
-      xsnprintf (mapsfilename, sizeof mapsfilename,
-		 "/proc/%d/maps", current_inferior ()->pid);
+      xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/maps", pid);
       data = target_fileio_read_stralloc (mapsfilename);
     }
-  if (data)
+
+  if (data != NULL)
     {
       struct cleanup *cleanup = make_cleanup (xfree, data);
-      char *line;
+      char *line, *t;
 
-      line = strtok (data, "\n");
-      while (line)
+      line = strtok_r (data, "\n", &t);
+      while (line != NULL)
 	{
 	  ULONGEST addr, endaddr, offset, inode;
 	  const char *permissions, *device, *filename;
+	  struct smaps_vmflags v;
 	  size_t permissions_len, device_len;
-	  int read, write, exec;
-	  int modified = 0, has_anonymous = 0;
+	  int read, write, exec, private;
+	  enum memory_mapping_state state;
+	  int has_anonymous = 0;
+	  int mapping_anon_p;
 
+	  memset (&v, 0, sizeof (v));
 	  read_mapping (line, &addr, &endaddr, &permissions, &permissions_len,
 			&offset, &device, &device_len, &inode, &filename);
+	  mapping_anon_p = mapping_is_anonymous_p (filename);
 
 	  /* Decode permissions.  */
 	  read = (memchr (permissions, 'r', permissions_len) != 0);
 	  write = (memchr (permissions, 'w', permissions_len) != 0);
 	  exec = (memchr (permissions, 'x', permissions_len) != 0);
+	  /* 'private' here actually means VM_MAYSHARE, and not
+	     VM_SHARED.  In order to know if a mapping is really
+	     private or not, we must check the flag "sh" in the
+	     VmFlags field.  This is done by decode_vmflags.  However,
+	     if we are using an old Linux kernel, we will not have the
+	     VmFlags there.  In this case, there is really no way to
+	     know if we are dealing with VM_SHARED, so we just assume
+	     that VM_MAYSHARE is enough.  */
+	  private = memchr (permissions, 'p', permissions_len) != 0;
 
 	  /* Try to detect if region was modified by parsing smaps counters.  */
-	  for (line = strtok (NULL, "\n");
-	       line && line[0] >= 'A' && line[0] <= 'Z';
-	       line = strtok (NULL, "\n"))
+	  for (line = strtok_r (NULL, "\n", &t);
+	       line != NULL && line[0] >= 'A' && line[0] <= 'Z';
+	       line = strtok_r (NULL, "\n", &t))
 	    {
 	      char keyword[64 + 1];
 
@@ -869,11 +1108,17 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 		  warning (_("Error parsing {s,}maps file '%s'"), mapsfilename);
 		  break;
 		}
+
 	      if (strcmp (keyword, "Anonymous:") == 0)
-		has_anonymous = 1;
-	      if (strcmp (keyword, "Shared_Dirty:") == 0
-		  || strcmp (keyword, "Private_Dirty:") == 0
-		  || strcmp (keyword, "Swap:") == 0
+		{
+		  /* Older Linux kernels did not support the
+		     "Anonymous:" counter.  Check it here.  */
+		  has_anonymous = 1;
+		}
+	      else if (strcmp (keyword, "VmFlags:") == 0)
+		decode_vmflags (line, &v);
+
+	      if (strcmp (keyword, "AnonHugePages:") == 0
 		  || strcmp (keyword, "Anonymous:") == 0)
 		{
 		  unsigned long number;
@@ -884,19 +1129,43 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 			       mapsfilename);
 		      break;
 		    }
-		  if (number != 0)
-		    modified = 1;
+		  if (number > 0)
+		    {
+		      /* Even if we are dealing with a file-backed
+			 mapping, if it contains anonymous pages we
+			 consider it to be an anonymous mapping,
+			 because this is what the Linux kernel does:
+
+			 // Dump segments that have been written to.
+			 if (vma->anon_vma && FILTER(ANON_PRIVATE))
+			 	goto whole;
+		      */
+		      mapping_anon_p = 1;
+		    }
 		}
 	    }
 
-	  /* Older Linux kernels did not support the "Anonymous:" counter.
-	     If it is missing, we can't be sure - dump all the pages.  */
-	  if (!has_anonymous)
-	    modified = 1;
+	  /* If a mapping should not be dumped we still should create
+	     a segment for it, just without SEC_LOAD (see
+	     gcore_create_callback).  */
+	  if (has_anonymous)
+	    {
+	      if (dump_mapping_p (filterflags, &v, private, mapping_anon_p,
+				  filename))
+		state = MEMORY_MAPPING_MODIFIED;
+	      else
+		state = MEMORY_MAPPING_UNMODIFIED;
+	    }
+	  else
+	    {
+	      /* Older Linux kernels did not support the "Anonymous:" counter.
+		 If it is missing, we can't be sure - dump all the pages.  */
+	      state = MEMORY_MAPPING_UNKNOWN_STATE;
+	    }
 
 	  /* Invoke the callback function to create the corefile segment.  */
 	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+		read, write, exec, state, filename, obfd);
 	}
 
       do_cleanups (cleanup);
@@ -926,12 +1195,13 @@ struct linux_find_memory_regions_data
 static int
 linux_find_memory_regions_thunk (ULONGEST vaddr, ULONGEST size,
 				 ULONGEST offset, ULONGEST inode,
-				 int read, int write, int exec, int modified,
+				 int read, int write, int exec,
+				 enum memory_mapping_state state,
 				 const char *filename, void *arg)
 {
   struct linux_find_memory_regions_data *data = arg;
 
-  return data->func (vaddr, size, read, write, exec, modified, data->obfd);
+  return data->func (vaddr, size, read, write, exec, state, data->obfd);
 }
 
 /* A variant of linux_find_memory_regions_full that is suitable as the
@@ -1074,7 +1344,8 @@ static linux_find_memory_region_ftype linux_make_mappings_callback;
 static int
 linux_make_mappings_callback (ULONGEST vaddr, ULONGEST size,
 			      ULONGEST offset, ULONGEST inode,
-			      int read, int write, int exec, int modified,
+			      int read, int write, int exec,
+			      enum memory_mapping_state state,
 			      const char *filename, void *data)
 {
   struct linux_make_mappings_data *map_data = data;
@@ -1869,7 +2140,8 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
 
 static int
 find_mapping_size (CORE_ADDR vaddr, unsigned long size,
-		   int read, int write, int exec, int modified,
+		   int read, int write, int exec,
+		   enum memory_mapping_state state,
 		   void *data)
 {
   struct mem_range *range = data;
@@ -1969,6 +2241,17 @@ linux_infcall_mmap (CORE_ADDR size, unsigned prot)
   return retval;
 }
 
+/* Display whether the gcore command is using the
+   /proc/PID/coredump_filter file.  */
+
+static void
+show_use_coredump_filter (struct ui_file *file, int from_tty,
+			  struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Use of /proc/PID/coredump_filter file to generate"
+			    " corefiles is %s.\n"), value);
+}
+
 /* To be called from the various GDB_OSABI_LINUX handlers for the
    various GNU/Linux architectures and machine types.  */
 
@@ -2005,4 +2288,16 @@ _initialize_linux_tdep (void)
   /* Observers used to invalidate the cache when needed.  */
   observer_attach_inferior_exit (invalidate_linux_cache_inf);
   observer_attach_inferior_appeared (invalidate_linux_cache_inf);
+
+  add_setshow_boolean_cmd ("use-coredump-filter", class_files,
+			   &use_coredump_filter, _("\
+Set whether gcore should consider /proc/PID/coredump_filter."),
+			   _("\
+Show whether gcore should consider /proc/PID/coredump_filter."),
+			   _("\
+Use this command to set whether gcore should consider the contents\n\
+of /proc/PID/coredump_filter when generating the corefile.  For more information\n\
+about this file, refer to the manpage of core(5)."),
+			   NULL, show_use_coredump_filter,
+			   &setlist, &showlist);
 }
diff --git a/gdb/procfs.c b/gdb/procfs.c
index b62539f..d074dd3 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4967,7 +4967,7 @@ find_memory_regions_callback (struct prmap *map,
 		  (map->pr_mflags & MA_READ) != 0,
 		  (map->pr_mflags & MA_WRITE) != 0,
 		  (map->pr_mflags & MA_EXEC) != 0,
-		  1, /* MODIFIED is unknown, pass it as true.  */
+		  MEMORY_MAPPING_UNKNOWN_STATE, /* MODIFIED is unknown.  */
 		  data);
 }
 
diff --git a/gdb/testsuite/gdb.base/coredump-filter.c b/gdb/testsuite/gdb.base/coredump-filter.c
new file mode 100644
index 0000000..192c469
--- /dev/null
+++ b/gdb/testsuite/gdb.base/coredump-filter.c
@@ -0,0 +1,61 @@
+/* Copyright 2015 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/>.  */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <assert.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/mman.h>
+#include <errno.h>
+#include <string.h>
+
+static void *
+do_mmap (void *addr, size_t size, int prot, int flags, int fd, off_t offset)
+{
+  void *ret = mmap (addr, size, prot, flags, fd, offset);
+
+  assert (ret != NULL);
+  return ret;
+}
+
+int
+main (int argc, char *argv[])
+{
+  const size_t size = 10;
+  const int default_prot = PROT_READ | PROT_WRITE;
+  char *private_anon, *shared_anon;
+  char *dont_dump;
+  int i;
+
+  private_anon = do_mmap (NULL, size, default_prot,
+			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  memset (private_anon, 0x11, size);
+
+  shared_anon = do_mmap (NULL, size, default_prot,
+			 MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+  memset (shared_anon, 0x22, size);
+
+  dont_dump = do_mmap (NULL, size, default_prot,
+		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  memset (dont_dump, 0x55, size);
+  i = madvise (dont_dump, size, MADV_DONTDUMP);
+  assert_perror (errno);
+  assert (i == 0);
+
+  return 0; /* break-here */
+}
diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
new file mode 100644
index 0000000..c7ae91d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -0,0 +1,129 @@
+# Copyright 2015 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+    untested $testfile.exp
+    return -1
+}
+
+if { ![runto_main] } {
+    untested $testfile.exp
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+proc do_save_core { filter_flag core ipid } {
+    verbose -log "writing $filter_flag to /proc/$ipid/coredump_filter"
+    if { [catch {open /proc/$ipid/coredump_filter w} fileid] } {
+	untested $testfile.exp
+	return -1
+    }
+
+    # Set coredump_filter to the value we want
+    puts $fileid $filter_flag
+    close $fileid
+
+    # Generate a corefile
+    gdb_gcore_cmd "$core" "save corefile $core"
+}
+
+proc do_load_and_test_core { core var working_var working_value } {
+    global hex decimal addr
+
+    set core_loaded [gdb_core_cmd "$core" "load $core"]
+    if { $core_loaded == -1 } {
+	fail "loading $core"
+	return
+    }
+
+    # Use 'int' as any variants of 'char' try to read the target bytes.
+    gdb_test "print *(unsigned int *) $addr($var)" "\(\\\$$decimal = <error: \)?Cannot access memory at address $hex\(>\)?" \
+	"printing $var when core is loaded (should not work)"
+    gdb_test "print/x *(unsigned int *) $addr($working_var)" " = $working_value.*" \
+	"print/x *$working_var ( = $working_value)"
+}
+
+set non_private_anon_core [standard_output_file non-private-anon.gcore]
+set non_shared_anon_core [standard_output_file non-shared-anon.gcore]
+set dont_dump_core [standard_output_file dont-dump.gcore]
+
+# We will generate a few corefiles
+#
+# This list is composed by sub-lists, and their elements are (in
+# order):
+#
+# - name of the test
+# - hexadecimal value to be put in the /proc/PID/coredump_filter file
+# - name of the variable that contains the name of the corefile to be
+#   generated (including the initial $).
+# - name of the variable in the C source code that points to the
+#   memory mapping that will NOT be present in the corefile.
+# - name of a variable in the C source code that points to a memory
+#   mapping that WILL be present in the corefile
+# - corresponding value expected for the above variable
+
+set all_corefiles { { "non-Private-Anonymous" "0x7e" \
+			  $non_private_anon_core \
+			  "private_anon" \
+			  "shared_anon" "0x22" }
+    { "non-Shared-Anonymous" "0x7d" \
+	  $non_shared_anon_core "shared_anon" \
+	  "private_anon" "0x11" }
+    { "DoNotDump" "0x33" \
+	  $dont_dump_core "dont_dump" \
+	  "shared_anon" "0x22" } }
+
+set core_supported [gdb_gcore_cmd "$non_private_anon_core" "save a corefile"]
+if { !$core_supported } {
+    untested $testfile.exp
+    return -1
+}
+
+# Getting the inferior's PID
+gdb_test_multiple "info inferiors" "getting inferior pid" {
+    -re "process \($decimal\).*\r\n$gdb_prompt $" {
+	set infpid $expect_out(1,string)
+    }
+}
+
+foreach item $all_corefiles {
+    foreach name [list [lindex $item 3] [lindex $item 4]] {
+	set test "print/x $name"
+	gdb_test_multiple $test $test {
+	    -re " = \($hex\)\r\n$gdb_prompt $" {
+		set addr($name) $expect_out(1,string)
+	    }
+	}
+    }
+}
+
+foreach item $all_corefiles {
+    with_test_prefix "saving corefile for [lindex $item 0]" {
+	do_save_core [lindex $item 1] [subst [lindex $item 2]] $infpid
+    }
+}
+
+clean_restart $testfile
+
+foreach item $all_corefiles {
+    with_test_prefix "loading and testing corefile for [lindex $item 0]" {
+	do_load_and_test_core [subst [lindex $item 2]] [lindex $item 3] \
+	    [lindex $item 4] [lindex $item 5]
+    }
+}

^ permalink raw reply	[flat|nested] 46+ messages in thread
end of thread, other threads:[~2015-03-18 19:39 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-05  3:48 [PATCH] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902) Sergio Durigan Junior
2015-03-05 15:48 ` Jan Kratochvil
2015-03-05 20:53   ` Sergio Durigan Junior
2015-03-05 20:57     ` Jan Kratochvil
2015-03-11 20:02       ` Oleg Nesterov
2015-03-12 11:31         ` Sergio Durigan Junior
2015-03-12 14:36         ` vvar, gup && coredump Oleg Nesterov
2015-03-12 16:29           ` Andy Lutomirski
2015-03-12 16:56             ` Oleg Nesterov
2015-03-12 17:18               ` Andy Lutomirski
2015-03-12 17:40                 ` Oleg Nesterov
2015-03-12 17:45                   ` Sergio Durigan Junior
2015-03-12 18:04                     ` Oleg Nesterov
2015-03-13  4:50                       ` Sergio Durigan Junior
2015-03-13 15:06                         ` Oleg Nesterov
2015-03-12 17:56                   ` Andy Lutomirski
2015-03-12 18:28                     ` Oleg Nesterov
2015-03-12 17:48               ` Oleg Nesterov
2015-03-12 17:55                 ` Andy Lutomirski
2015-03-12 18:16                   ` Oleg Nesterov
2015-03-12 18:23                     ` Sergio Durigan Junior
2015-03-12 18:20                 ` Pedro Alves
2015-03-12 18:26                   ` Andy Lutomirski
2015-03-16 19:03                 ` install_special_mapping && vm_pgoff (Was: vvar, gup && coredump) Oleg Nesterov
2015-03-16 19:20                   ` Andy Lutomirski
2015-03-16 19:46                     ` Oleg Nesterov
2015-03-17 13:45                       ` Oleg Nesterov
2015-03-18  1:45                         ` Andy Lutomirski
2015-03-18 18:08                           ` Oleg Nesterov
2015-03-16 19:40                   ` Pedro Alves
2015-03-12 15:02         ` [PATCH] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902) Oleg Nesterov
2015-03-12 15:46           ` Pedro Alves
2015-03-12 15:57             ` Jan Kratochvil
2015-03-12 16:19               ` Pedro Alves
2015-03-12 16:07             ` Oleg Nesterov
2015-03-12 16:28               ` Pedro Alves
2015-03-12 17:37           ` Sergio Durigan Junior
2015-03-12 21:39 ` [PATCH v2] " Sergio Durigan Junior
2015-03-13 19:34   ` Pedro Alves
2015-03-16 23:53     ` Sergio Durigan Junior
2015-03-18 19:10       ` Pedro Alves
2015-03-18 19:39         ` Sergio Durigan Junior
2015-03-14  9:40   ` Eli Zaretskii
2015-03-16  2:42     ` Sergio Durigan Junior
2015-03-13 19:37 ` [PATCH] " Pedro Alves
2015-03-13 19:48   ` Pedro Alves

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).