[PATCH v4 0/2] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902)

[PATCH v4 0/2] Improve corefile generation by using /proc/PID/coredump_filter (PR corefile/16902)

[Sergio Durigan Junior]

Hello,

This is the fifth version of this patch series.  It basically includes
fixes to make GDB mimic the Linux kernel's behavior when dealing with
"file-backed mappings with anonymous pages".  As explained on:

  <https://sourceware.org/ml/gdb-patches/2015-03/msg00814.html>

in this situation, the Linux kernel dumps this "special" mapping both
when the user has requested file-backed or anonymous mappings to be
included in the corefile.  Now, GDB does the same thing.

The patch also extends the testcase in order to make sure that,
depending on the type of the corefile we are dealing with, we can or
cannot disassemble a section of the code without loading a binary.  The
approach I took for this is:

  - For corefiles that include the file-backed mappings, GDB should be
    able to disassemble.

  - For corefiles that do not include file-backed nor anonymous
    mappings, GDB should not be able to disassemble.

It was necessary to "relax" the second test because, as explained in the
message linked above, when you load a binary inside GDB its .text
segment contains anonymous pages, so if you ask for a corefile without
file-backed mappings (which, in theory, should exclude the .text
segment), GDB will still include the .text segment because this kind of
mapping is *also* considered an anonymous mapping (the Linux kernel does
the same thing here).

Other than that, the code is the same.  The documentation has been
approved already, but I am including a NEWS entry this time (I wasn't
sure if it was worth it or not, so I decided to go ahead and do it).

OK to apply?

Sergio Durigan Junior (2):
  Implement support for checking /proc/PID/coredump_filter
  Documentation and testcase

 gdb/doc/gdb.texinfo                        |  33 +++
 gdb/linux-tdep.c                           | 455 +++++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/coredump-filter.c   |  61 ++++
 gdb/testsuite/gdb.base/coredump-filter.exp | 198 +++++++++++++
 4 files changed, 719 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/coredump-filter.c
 create mode 100644 gdb/testsuite/gdb.base/coredump-filter.exp

-- 
1.9.3

[PATCH 2/2] Documentation and testcase

[Sergio Durigan Junior]

This patch implements the testcase and the documentation bits for the
feature.  It will be merged with the first patch when everything is
approved, and I will push a single commit with the whole feature.

The testcase makes sure that, based on different types of corefiles,
we can or cannot access certain memory regions.  There is also a test
to make sure that we can or cannot disassemble the main function, also
depending on which pages a corefile contains.

The documentation, which has already been approved by Eli, explains
how to use the new feature.  However, I included a NEWS entry this
time, because I think it is worth mentioning this feature there.

Changes from v4:

  - Added tests to make sure that, when loading a corefile without a
    binary on GDB, disassembling the main function works or not
    depending on the type of corefile used.

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

	* NEWS: Mention the possibility of using the
	'/proc/PID/coredump_filter' file when generating a corefile.
	Mention new command 'set use-coredump-filter'.

gdb/doc/ChangeLog:
2015-03-24  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-24  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR corefiles/16092
	* gdb.base/coredump-filter.c: New file.
	* gdb.base/coredump-filter.exp: Likewise.
---
 gdb/NEWS                                   |   8 ++
 gdb/doc/gdb.texinfo                        |  33 +++++
 gdb/testsuite/gdb.base/coredump-filter.c   |  61 +++++++++
 gdb/testsuite/gdb.base/coredump-filter.exp | 198 +++++++++++++++++++++++++++++
 4 files changed, 300 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/coredump-filter.c
 create mode 100644 gdb/testsuite/gdb.base/coredump-filter.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 3fa33c9..96fb85c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,14 @@
 
 *** Changes since GDB 7.9
 
+* GDB now honors the content of the file /proc/PID/coredump_filter
+  (PID is the process ID) on GNU/Linux systems.  This file can be used
+  to specify the types of memory mappings that will be included in a
+  corefile.  For more information, please refer to the manual page of
+  "core(5)".  GDB also has a new command: "set use-coredump-filter
+  on|off".  It allows to set whether GDB will read the content of the
+  /proc/PID/coredump_filter file when generating a corefile.
+
 * GDB has two new commands: "set serial parity odd|even|none" and
   "show serial parity".  These allows to set or show parity for the
   remote serial I/O.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c03ecb0..f750ed5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10951,6 +10951,39 @@ 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.  @var{pid} is the process ID of a currently running process.
+
+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.  This
+configuration is inherited by child processes.  For more information
+about the bits that can be set in the
+@file{/proc/@var{pid}/coredump_filter} file, please refer to the
+manpage of @code{core(5)}.
+
+By default, this option is @code{on}.  If this option is turned
+@code{off}, @value{GDBN} does not read the @file{coredump_filter} file
+and instead 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 is currently @code{0x33}, which means that bits @code{0}
+(anonymous private mappings), @code{1} (anonymous shared mappings),
+@code{4} (ELF headers) and @code{5} (private huge pages) are active.
+This will cause these memory mappings to be dumped automatically.
 @end table
 
 @node Character Sets
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..f3203be
--- /dev/null
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -0,0 +1,198 @@
+# 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 "could not compile test program"
+    return -1
+}
+
+if { ![runto_main] } {
+    untested "could not run to main"
+    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"
+
+    remote_exec target "sh -c \"echo $filter_flag > /proc/$ipid/coredump_filter\""
+
+    # Generate a corefile.
+    gdb_gcore_cmd "$core" "save corefile"
+}
+
+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
+    }
+
+    # Access the memory the addresses point to.
+    gdb_test "print/x *(char *) $addr($var)" "\(\\\$$decimal = <error: \)?Cannot access memory at address $hex\(>\)?" \
+	"printing $var when core is loaded (should not work)"
+    gdb_test "print/x *(char *) $addr($working_var)" " = $working_value.*" \
+	"print/x *$working_var ( = $working_value)"
+}
+
+# We do not do file-backed mappings in the test program, but it is
+# important to test this anyway.  One way of performing the test is to
+# load GDB with a corefile but without a binary, and then ask for the
+# 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.
+
+proc test_disasm { core address should_fail } {
+    global testfile hex
+
+    # Restart GDB without loading the binary.
+    with_test_prefix "no binary" {
+	gdb_exit
+	gdb_start
+
+	set core_loaded [gdb_core_cmd "$core" "load core"]
+	if { $core_loaded == -1 } {
+	    fail "loading $core"
+	    return
+	}
+
+	if { $should_fail == 1 } {
+	    gdb_test "x/i \$pc" "=> $hex:\tCannot access memory at address $hex" \
+		"disassemble function with corefile and without a binary"
+	} else {
+	    gdb_test "x/i \$pc" "=> $hex:\t\[^C\].*" \
+		"disassemble function with corefile and without a binary"
+	}
+    }
+
+    with_test_prefix "with binary" {
+	clean_restart $testfile
+
+	set core_loaded [gdb_core_cmd "$core" "load core"]
+	if { $core_loaded == -1 } {
+	    fail "loading $core"
+	    return
+	}
+
+	gdb_test "disassemble $address" "Dump of assembler code for function.*" \
+	    "disassemble function with corefile and with a binary"
+    }
+}
+
+set non_private_anon_core [standard_output_file non-private-anon.gcore]
+set non_shared_anon_core [standard_output_file non-shared-anon.gcore]
+# A corefile without {private,shared} {anonymous,file-backed} pages
+set non_private_shared_anon_file_core [standard_output_file non-private-shared-anon-file.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
+#
+# This list refers to the corefiles generated by MAP_ANONYMOUS in the
+# test program.
+
+set all_anon_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" } }
+
+# If corefile loading is not supported, we do not even try to run the
+# tests.
+set core_supported [gdb_gcore_cmd "$non_private_anon_core" "save a corefile"]
+if { !$core_supported } {
+    untested "corefile generation is not supported"
+    return -1
+}
+
+# Get the inferior's PID.
+set infpid ""
+gdb_test_multiple "info inferiors" "getting inferior pid" {
+    -re "process \($decimal\).*\r\n$gdb_prompt $" {
+	set infpid $expect_out(1,string)
+    }
+}
+
+# Get the main function's address.
+set main_addr ""
+gdb_test_multiple "print/x &main" "getting main's address" {
+    -re "$decimal = \($hex\)\r\n$gdb_prompt $" {
+	set main_addr $expect_out(1,string)
+    }
+}
+
+# Obtain the address of each variable that will be checked on each
+# case.
+foreach item $all_anon_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)
+	    }
+	}
+    }
+}
+
+# Generate corefiles for the "anon" case.
+foreach item $all_anon_corefiles {
+    with_test_prefix "saving corefile for [lindex $item 0]" {
+	do_save_core [lindex $item 1] [subst [lindex $item 2]] $infpid
+    }
+}
+
+with_test_prefix "saving corefile for non-Private-Shared-Anon-File" {
+    do_save_core "0x60" $non_private_shared_anon_file_core $infpid
+}
+
+clean_restart $testfile
+
+foreach item $all_anon_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]
+    }
+
+    with_test_prefix "disassembling function main for [lindex $item 0]" {
+	test_disasm [subst [lindex $item 2]] $main_addr 0
+    }
+}
+
+with_test_prefix "loading and testing corefile for non-Private-Shared-Anon-File" {
+    test_disasm $non_private_shared_anon_file_core $main_addr 1
+}
-- 
1.9.3

[PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Sergio Durigan Junior]

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, 4 and 5 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 Nesterov'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 will always dump
"[vsyscall]" or "[vdso]" mappings (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.
  There is a special case in this, though: if the memory mapping is a
  file-backed one, but *also* contains "Anonymous:" or
  "AnonHugePages:" pages, then GDB considers this mapping to be *both*
  anonymous and file-backed, just like the Linux kernel does.  What
  that means is simple: this mapping will be dumped if the user
  requested anonymous mappings *or* if the user requested file-backed
  mappings to be present in the corefile.

  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 (as it has been discussed, GDB would need to run as root,
  and would need to check the contents of the /proc/PID/map_files/
  directory in order to determine whether the deleted was a hardlink
  or not).  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 Linux kernels (see
  the code for more details) 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.

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.

Changes from v4:

  - GDB is now handling the "file-backed mapping with anonymous pages"
    case just like the Linux kernel.

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

	PR corefiles/16092
	* 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_regions_full): New variables
	'coredumpfilter_name', 'coredumpfilterdata', 'pid', 'filterflags'.
	Removed variable 'modified'.  Read /proc/<PID>/smaps file; improve
	parsing of its information.  Implement memory mapping filtering
	based on its contents.
	(show_use_coredump_filter): New function.
	(_initialize_linux_tdep): New command 'set use-coredump-filter'.
---
 gdb/linux-tdep.c | 455 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 427 insertions(+), 28 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ea0d4cd..4af1d01 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -35,9 +35,61 @@
 #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,
+  };
+
+/* This struct is used to map flags found in the "VmFlags:" field (in
+   the /proc/<PID>/smaps file).  */
+
+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 +433,248 @@ 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 (3.10) do not have this
+   field on smaps.  */
+
+static void
+decode_vmflags (char *p, struct smaps_vmflags *v)
+{
+  char *saveptr;
+  const 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);
+
+      /* Let's be pessimistic and assume there will be an error while
+	 compiling the regex'es.  */
+      init_regex_p = -1;
+
+      /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or
+	 without the "(deleted)" string in the end).  We know for
+	 sure, based on the Linux kernel code, that memory mappings
+	 whose associated filename is "/dev/zero" are guaranteed to be
+	 MAP_ANONYMOUS.  */
+      compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match /dev/zero "
+			     "filename"));
+      /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or
+	 without the "(deleted)" string in the end).  These filenames
+	 refer to shared memory (shmem), and memory mappings
+	 associated with them are MAP_ANONYMOUS as well.  */
+      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 we reached this point, then everything succeeded.  */
+      init_regex_p = 1;
+    }
+
+  if (init_regex_p == -1)
+    {
+      const char deleted[] = " (deleted)";
+      size_t del_len = sizeof (deleted) - 1;
+      size_t filename_len = strlen (filename);
+
+      /* There was an error while compiling the regex'es above.  In
+	 order to try to give some reliable information to the caller,
+	 we just try to find the string " (deleted)" in the filename.
+	 If we managed to find it, then we assume the mapping is
+	 anonymous.  */
+      return (filename_len >= del_len
+	      && strcmp (filename + filename_len - del_len, deleted) == 0);
+    }
+
+  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.
+
+   In a nutshell, this is the logic that we follow in order to decide
+   if a mapping should be dumped or not.
+
+   - 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 (and normally) do this check (iff running as
+     root, it could find the mapping in /proc/PID/map_files/ and
+     determine whether there still are other hard links to the
+     inode/file).  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,
+     Linux kernels before commit
+     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10) 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.  */
+
+static int
+dump_mapping_p (unsigned int filterflags, const struct smaps_vmflags *v,
+		int maybe_private_p, int mapping_anon_p, int mapping_file_p,
+		const char *filename)
+{
+  /* Initially, we trust in what we received from our caller.  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 until we take a look at the "VmFlags:" field
+     (assuming that the version of the Linux kernel being used
+     supports it, of course).  */
+  int private_p = maybe_private_p;
+
+  /* We always dump vDSO and vsyscall mappings, because it's likely that
+     there'll be no file to read the contents from at core load time.
+     The kernel does the same.  */
+  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;
+
+      /* Update 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 && mapping_file_p)
+	{
+	  /* This is a special situation.  It can happen when we see a
+	     mapping that is file-backed, but that contains anonymous
+	     pages.  */
+	  return ((filterflags & COREFILTER_ANON_PRIVATE) != 0
+		  || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0);
+	}
+      else if (mapping_anon_p)
+	return (filterflags & COREFILTER_ANON_PRIVATE) != 0;
+      else
+	return (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
+    }
+  else
+    {
+      if (mapping_anon_p && mapping_file_p)
+	{
+	  /* This is a special situation.  It can happen when we see a
+	     mapping that is file-backed, but that contains anonymous
+	     pages.  */
+	  return ((filterflags & COREFILTER_ANON_SHARED) != 0
+		  || (filterflags & COREFILTER_MAPPED_SHARED) != 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
@@ -819,48 +1113,97 @@ 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;
+	  int has_anonymous = 0;
+	  int should_dump_p = 0;
+	  int mapping_anon_p;
+	  int mapping_file_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);
+	  /* If the mapping is not anonymous, then we can consider it
+	     to be file-backed.  These two states (anonymous or
+	     file-backed) seem to be exclusive, but they can actually
+	     coexist.  For example, if a file-backed mapping has
+	     "Anonymous:" pages (see more below), then the Linux
+	     kernel will dump this mapping when the user specified
+	     that she only wants anonymous mappings in the corefile
+	     (*even* when she explicitly disabled the dumping of
+	     file-backed mappings).  */
+	  mapping_file_p = !mapping_anon_p;
 
 	  /* Decode permissions.  */
 	  read = (memchr (permissions, 'r', permissions_len) != 0);
 	  write = (memchr (permissions, 'w', permissions_len) != 0);
 	  exec = (memchr (permissions, 'x', 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"))
+	  /* '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 a Linux kernel released before the commit
+	     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10), 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 should be dumped by parsing smaps
+	     counters.  */
+	  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 +1212,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 +1233,46 @@ 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 *also* 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;
+
+			 Note that if the mapping is already marked as
+			 file-backed (i.e., mapping_file_p is
+			 non-zero), then this is a special case, and
+			 this mapping will be dumped either when the
+			 user wants to dump file-backed *or* anonymous
+			 mappings.  */
+		      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 (has_anonymous)
+	    should_dump_p = dump_mapping_p (filterflags, &v, private,
+					    mapping_anon_p, mapping_file_p,
+					    filename);
+	  else
+	    {
+	      /* Older Linux kernels did not support the "Anonymous:" counter.
+		 If it is missing, we can't be sure - dump all the pages.  */
+	      should_dump_p = 1;
+	    }
 
 	  /* Invoke the callback function to create the corefile segment.  */
-	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+	  if (should_dump_p)
+	    func (addr, endaddr - addr, offset, inode,
+		  read, write, exec, 1, /* MODIFIED is true because we
+					   want to dump the mapping.  */
+		  filename, obfd);
 	}
 
       do_cleanups (cleanup);
@@ -1972,6 +2348,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.  */
 
@@ -2008,4 +2395,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);
 }
-- 
1.9.3

Re: [PATCH 2/2] Documentation and testcase

[Pedro Alves]

OK.

Thanks,
Pedro Alves

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Pedro Alves]

OK.

Thanks,
Pedro Alves

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Sergio Durigan Junior]

On Friday, March 27 2015, Pedro Alves wrote:

> OK.

Thanks, pushed both patches:

  <https://sourceware.org/ml/gdb-cvs/2015-03/msg00275.html>
  df8411da087dc05481926f4c4a82deabc5bc3859

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

Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pierre Muller]

  Hi all,

The approved patch "Implement support for checking /proc/PID/coredump_filter
commit"
committed April 1. 2015:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=df84
11da087dc05481926f4c4a82deabc5bc3859

breaks mingw64 with --enable-targets=all configure option.

The reason is the introduction of strtok_r function in gdb/linux-tdep.c
source.
This function is not present in my version of x86_64-w64-mingw32
cross-system (on a Cygwin system).

The function is found in string.h header on native Cygwin, but inside a
conditional:

/usr/include/string.h-46-#if __POSIX_VISIBLE
/usr/include/string.h:47:char   *_EXFUN(strtok_r,(char *__restrict, const
char *__restrict, char **__restrict));
/usr/include/string.h-48-#endif

I did not find any explicit inclusion of string header inside linux-tdep.c
source...

There is a workaround in mingw64 pthread.h:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/pthread.h:455:#ifndef
strtok_r
/usr/x86_64-w64-mingw32/sys-root/mingw/include/pthread.h:456:#define
strtok_r(__s, __sep, __last)  (*(__last) = strtok((__s), (__sep)))
/usr/x86_64-w64-mingw32/sys-root/mingw/include/pthread.h-457-#endif

Does this suggest that strtok_r is not a required string.h function?
If yes, should this function be tested by configure...

Or maybe we should use the gnulib version of strtok_r?
I don't really know how to do this, but if someone can tell me,
I can test the patch.


Pierre Muller



> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Sergio Durigan Junior
> Envoyé : mercredi 1 avril 2015 01:40
> À : Pedro Alves
> Cc : GDB Patches
> Objet : Re: [PATCH 1/2] Implement support for checking
> /proc/PID/coredump_filter
> 
> On Friday, March 27 2015, Pedro Alves wrote:
> 
> > OK.
> 
> Thanks, pushed both patches:
> 
>   <https://sourceware.org/ml/gdb-cvs/2015-03/msg00275.html>
>   df8411da087dc05481926f4c4a82deabc5bc3859
> 
> --
> Sergio
> GPG key ID: 0x65FC5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pedro Alves]

On 04/08/2015 03:07 PM, Pierre Muller wrote:

> Or maybe we should use the gnulib version of strtok_r?

Yes.

> I don't really know how to do this, but if someone can tell me,
> I can test the patch.

Let me give that a try, and I'll post a branch for you to test.

There's also the "users/palves/gnulib-update" branch that
updates gdb's copy of gnulib to recent gnulib master.  It'd
be great if someone confirmed that a gdb build of that branch
actually works on Windows, so we can move forward with that.
(the branch predates Sergio's patch, so isn't affected by the strtok_r
issue).  I'd be nice to update our gnulib copy before we pull in
more modules, to avoid tripping on bugs that have meanwhile already
been fixed upstream, or avoid module dependencies that may
have been reduced upstream...

Thanks,
Pedro Alves

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pedro Alves]

On 04/08/2015 03:43 PM, Pedro Alves wrote:
> On 04/08/2015 03:07 PM, Pierre Muller wrote:
> 
>> Or maybe we should use the gnulib version of strtok_r?
> 
> Yes.
> 
>> I don't really know how to do this, but if someone can tell me,
>> I can test the patch.
> 
> Let me give that a try, and I'll post a branch for you to test.

Please try the users/palves/gnulib-strtok_r branch.

A nice thing is that the strtok_r module didn't pull in any
other new module.

> There's also the "users/palves/gnulib-update" branch that
> updates gdb's copy of gnulib to recent gnulib master.  It'd
> be great if someone confirmed that a gdb build of that branch
> actually works on Windows, so we can move forward with that.
> (the branch predates Sergio's patch, so isn't affected by the strtok_r
> issue).  I'd be nice to update our gnulib copy before we pull in
> more modules, to avoid tripping on bugs that have meanwhile already
> been fixed upstream, or avoid module dependencies that may
> have been reduced upstream...

Thanks,
Pedro Alves

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Sergio Durigan Junior]

On Wednesday, April 08 2015, Pedro Alves wrote:

> On 04/08/2015 03:43 PM, Pedro Alves wrote:
>> On 04/08/2015 03:07 PM, Pierre Muller wrote:
>> 
>>> Or maybe we should use the gnulib version of strtok_r?
>> 
>> Yes.
>> 
>>> I don't really know how to do this, but if someone can tell me,
>>> I can test the patch.
>> 
>> Let me give that a try, and I'll post a branch for you to test.
>
> Please try the users/palves/gnulib-strtok_r branch.
>
> A nice thing is that the strtok_r module didn't pull in any
> other new module.

Sorry about the breakage, Pierre.  And thanks for taking care of that,
Pedro.

It seems that if this gnulib import succeed everything will be fine, so
I won't do anything for now.  But I will keep an eye on the discussion.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> There's also the "users/palves/gnulib-update" branch that
> updates gdb's copy of gnulib to recent gnulib master.  It'd
> be great if someone confirmed that a gdb build of that branch
> actually works on Windows, so we can move forward with that.
> (the branch predates Sergio's patch, so isn't affected by the strtok_r
> issue).  I'd be nice to update our gnulib copy before we pull in
> more modules, to avoid tripping on bugs that have meanwhile already
> been fixed upstream, or avoid module dependencies that may
> have been reduced upstream...

On the other hand, updating our gnulib copy from upstream may introduce
new bugs.  strtok_r exists in the gnulib
8d5bd1402003bd0153984b138735adf537d960b0 commit we are using, I don't
see why we need to update our gnulib copy, unless there are some known
bug fixes in gnulib upstream.

-- 
Yao (齐尧)

RE: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mercredi 8 avril 2015 17:14
> À : Pierre Muller; 'Sergio Durigan Junior'
> Cc : 'GDB Patches'
> Objet : Re: Build failure following: Implement support for checking
> /proc/PID/coredump_filter commit
> 
> On 04/08/2015 03:43 PM, Pedro Alves wrote:
> > On 04/08/2015 03:07 PM, Pierre Muller wrote:
> >
> >> Or maybe we should use the gnulib version of strtok_r?
> >
> > Yes.
> >
> >> I don't really know how to do this, but if someone can tell me,
> >> I can test the patch.
> >
> > Let me give that a try, and I'll post a branch for you to test.
> 
> Please try the users/palves/gnulib-strtok_r branch.

I can confirm that I am able to build mingw32 64bit gdb.exe executable,
and it contains gnulib/import/strtok_r.c source.

  I am not really able to test that the code works OK,
as I don't really know how to trigger that code...

  I have no idea what we should do now...

Pierre Muller

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pedro Alves]

On 04/08/2015 05:27 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> There's also the "users/palves/gnulib-update" branch that
>> updates gdb's copy of gnulib to recent gnulib master.  It'd
>> be great if someone confirmed that a gdb build of that branch
>> actually works on Windows, so we can move forward with that.
>> (the branch predates Sergio's patch, so isn't affected by the strtok_r
>> issue).  I'd be nice to update our gnulib copy before we pull in
>> more modules, to avoid tripping on bugs that have meanwhile already
>> been fixed upstream, or avoid module dependencies that may
>> have been reduced upstream...
> 
> On the other hand, updating our gnulib copy from upstream may introduce
> new bugs.

Sure.  And we can report and fix them, or downgrade a few revisions to
before when the bug was introduced.

> strtok_r exists in the gnulib
> 8d5bd1402003bd0153984b138735adf537d960b0 commit we are using, I don't

That's actually what I did in the branch I pointed Pierre at.

> see why we need to update our gnulib copy, unless there are some known
> bug fixes in gnulib upstream.

Why risk stumbling on bugs that might have been fixed in the 3
years since the last import?  The more modules we import off the older
copy, the wider the potential-bugs surface.  And then what to do if we
need to patch gnulib?  If the requirement is to send the fix upstream
first, and then refresh our import, it'll be much easier if the
re-import is just a small incremental update, rather than pulling in
multiple years of gnulib work.  The further apart our update is, the
harder it is to update.  If we keep our copy reasonably up to date, we
have a better chance of whoever introduced the bug upstream to still have
the issue fresh in mind and provide a prompt fix.  If we wait 3 years
to report a bug, then the chances are good that we get to fix
it ourselves.

I currently want to update gnulib for at least 3 reasons:

 - GDB-in-C++ on mingw:

   https://sourceware.org/ml/gdb-patches/2015-03/msg00447.html

   That will have to be fixed on gnulib.  Haven't checked if
   already fixed there, but most probably it is.

 - Antoine's use of canonicalize:

   https://sourceware.org/ml/gdb-patches/2015-03/msg00917.html

   That pulls in a truck load of modules.  I'd hope the dependencies
   are fewer in a newer gnulib, or at least, if the dependencies are the
   same, I'd much prefer importing up-to-date versions.  Otherwise, the
   poor soul that next updates gnulib is likely to have a "fun" time.

 - Pull in the fts module to get rid of the "system" call in compile/,
   as discussed a while ago.

I think now's the best time, as we still have time till the next
gdb release.

Thanks,
Pedro Alves

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Sergio Durigan Junior]

On Wednesday, April 08 2015, Pierre Muller wrote:

>> Please try the users/palves/gnulib-strtok_r branch.
>
> I can confirm that I am able to build mingw32 64bit gdb.exe executable,
> and it contains gnulib/import/strtok_r.c source.

That is great, thanks Pierre.

>   I am not really able to test that the code works OK,
> as I don't really know how to trigger that code...

The code will not work on native Windows debugging, so there is nothing
to test there.  If you debug a remote GNU/Linux target, you should be
able to tweak the value of /proc/PID/coredump_filter on the target and
generate different kinds of corefiles based on it.  But I don't know how
hard it would be for you to test this.

>   I have no idea what we should do now...

IMO the easiest and safest thing to do now would be to just import the
strtok_r module from gnulib (without updating it).

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Build failure following: Implement support for checking /proc/PID/coredump_filter commit

[Pedro Alves]

On 04/08/2015 05:46 PM, Sergio Durigan Junior wrote:
>> >   I have no idea what we should do now...
> IMO the easiest and safest thing to do now would be to just import the
> strtok_r module from gnulib (without updating it).

Yeah.  I posted the branch as proper patch series now.

 https://sourceware.org/ml/gdb-patches/2015-04/msg00288.html

Thanks,
Pedro Alves

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Pedro Alves]

On 03/23/2015 06:40 PM, Sergio Durigan Junior wrote:
> Done.  Patch attached.  I will wait until you look at the second patch
> (and approve it) in order to commit everything together.

Thanks.  FAOD, this version is OK.

On 03/23/2015 06:40 PM, Sergio Durigan Junior wrote:
> Jan has provided a good example of it before.  When you use stat -L, you
> can see if the 'Links' count is zero (which means the real file has been
> really deleted).  For example:
> 
>   lrw-------. 1 sergio sergio 64 Mar 23 14:34 7f2c5911b000-7f2c5911c000 -> /tmp/t2 (deleted)
>   [root@bla map_files]# stat -L 7f2c5911b000-7f2c5911c000 
>     File: ‘7f2c5911b000-7f2c5911c000’
>     Size: 0               Blocks: 0          IO Block: 4096   regular empty file
>   Device: 23h/35d         Inode: 217633770   Links: 1
> 
> The 'Links' is 1 because, despite the fact that 't2' has been deleted,
> the original file 't1' still exists.  However, if I remove 't1':
> 
>   [root@bla map_files]# stat -L 7f2c5911b000-7f2c5911c000 
>     File: ‘7f2c5911b000-7f2c5911c000’
>     Size: 0               Blocks: 0          IO Block: 4096   regular empty file
>   Device: 23h/35d         Inode: 217633770   Links: 0
> 
> Therefore, it should be possible to use stat even in this case.  But
> anyway, this was just FYI, we're not doing that.

Ah, yes, these files really work as symlinks, so ls -l was just
showing the symlink's own link count.  "ls -l -L" works too.

Thanks,
Pedro Alves

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Sergio Durigan Junior]

On Friday, March 20 2015, Pedro Alves wrote:

> On 03/20/2015 08:02 PM, Sergio Durigan Junior wrote:
>>>> >> +     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
>>> >
>>> > Cannot do it easily, or cannot do it at all?
>> It cannot do it easily.  If GDB is run by root, then it could (in
>> theory) inspect the contents of /proc/PID/map_files and determine
>> whether the " (deleted)" file was a hard link or not.
>
> Ah, could you include that in the comment please?
> Otherwise every time I read that I'll wonder what wasn't easy.  :-)
>
> Like:
>
> "(..) example.  The Linux kernel checks to see if "i_nlink == 0", but
> GDB cannot easily (and normally) do this check (iff running as root,
> it could find the mapping in /proc/PID/map_files/ and determine
> whether there still are other hard links to the inode/file).
> Therefore, we made a compromise (...)"

Done.  Patch attached.  I will wait until you look at the second patch
(and approve it) in order to commit everything together.

> I quickly tried something like that with "/proc/pid/fd/*", and
> unfortunately the link count is always 1, at least on F20's kernel:
>
> $ tail > a&
> [1] 12343
>
> [1]+  Stopped                 tail > a
> $ ls -l a
> -rw-rw-r--. 1 pedro pedro 0 Mar 20 21:29 a
> $ ln a b
> $ ls -l a
> -rw-rw-r--. 2 pedro pedro 0 Mar 20 21:29 a
> $ ls -l /proc/12343/fd/1
> l-wx------. 1 pedro pedro 64 Mar 20 21:30 /proc/12343/fd/1 -> /tmp/a
>
> If map_files/ works the same, I think we'd have to get the
> file's inode number, and look over the whole filesystem for
> other files with that inode number (the equivalent
> of 'ls -i' 'find / -inum').  Urgh!

Jan has provided a good example of it before.  When you use stat -L, you
can see if the 'Links' count is zero (which means the real file has been
really deleted).  For example:

  lrw-------. 1 sergio sergio 64 Mar 23 14:34 7f2c5911b000-7f2c5911c000 -> /tmp/t2 (deleted)
  [root@bla map_files]# stat -L 7f2c5911b000-7f2c5911c000 
    File: ‘7f2c5911b000-7f2c5911c000’
    Size: 0               Blocks: 0          IO Block: 4096   regular empty file
  Device: 23h/35d         Inode: 217633770   Links: 1

The 'Links' is 1 because, despite the fact that 't2' has been deleted,
the original file 't1' still exists.  However, if I remove 't1':

  [root@bla map_files]# stat -L 7f2c5911b000-7f2c5911c000 
    File: ‘7f2c5911b000-7f2c5911c000’
    Size: 0               Blocks: 0          IO Block: 4096   regular empty file
  Device: 23h/35d         Inode: 217633770   Links: 0

Therefore, it should be possible to use stat even in this case.  But
anyway, this was just FYI, we're not doing that.

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

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ea0d4cd..26edd5d 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -35,9 +35,61 @@
 #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,
+  };
+
+/* This struct is used to map flags found in the "VmFlags:" field (in
+   the /proc/<PID>/smaps file).  */
+
+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 +433,231 @@ 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 (3.10) do not have this
+   field on smaps.  */
+
+static void
+decode_vmflags (char *p, struct smaps_vmflags *v)
+{
+  char *saveptr;
+  const 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);
+
+      /* Let's be pessimistic and assume there will be an error while
+	 compiling the regex'es.  */
+      init_regex_p = -1;
+
+      /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or
+	 without the "(deleted)" string in the end).  We know for
+	 sure, based on the Linux kernel code, that memory mappings
+	 whose associated filename is "/dev/zero" are guaranteed to be
+	 MAP_ANONYMOUS.  */
+      compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match /dev/zero "
+			     "filename"));
+      /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or
+	 without the "(deleted)" string in the end).  These filenames
+	 refer to shared memory (shmem), and memory mappings
+	 associated with them are MAP_ANONYMOUS as well.  */
+      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 we reached this point, then everything succeeded.  */
+      init_regex_p = 1;
+    }
+
+  if (init_regex_p == -1)
+    {
+      const char deleted[] = " (deleted)";
+      size_t del_len = sizeof (deleted) - 1;
+      size_t filename_len = strlen (filename);
+
+      /* There was an error while compiling the regex'es above.  In
+	 order to try to give some reliable information to the caller,
+	 we just try to find the string " (deleted)" in the filename.
+	 If we managed to find it, then we assume the mapping is
+	 anonymous.  */
+      return (filename_len >= del_len
+	      && strcmp (filename + filename_len - del_len, deleted) == 0);
+    }
+
+  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.
+
+   In a nutshell, this is the logic that we follow in order to decide
+   if a mapping should be dumped or not.
+
+   - 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 (and normally) do this check (iff running as
+     root, it could find the mapping in /proc/PID/map_files/ and
+     determine whether there still are other hard links to the
+     inode/file).  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,
+     Linux kernels before commit
+     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10) 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.  */
+
+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 our caller.  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 until we take a look at the "VmFlags:" field
+     (assuming that the version of the Linux kernel being used
+     supports it, of course).  */
+  int private_p = maybe_private_p;
+
+  /* We always dump vDSO and vsyscall mappings, because it's likely that
+     there'll be no file to read the contents from at core load time.
+     The kernel does the same.  */
+  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;
+
+      /* Update 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
@@ -819,48 +1096,86 @@ 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;
+	  int has_anonymous = 0;
+	  int should_dump_p = 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);
-
-	  /* 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"))
+	  /* '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 a Linux kernel released before the commit
+	     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10), 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 should be dumped by parsing smaps
+	     counters.  */
+	  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 +1184,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 +1205,38 @@ 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 (has_anonymous)
+	    should_dump_p = dump_mapping_p (filterflags, &v, private,
+					    mapping_anon_p, filename);
+	  else
+	    {
+	      /* Older Linux kernels did not support the "Anonymous:" counter.
+		 If it is missing, we can't be sure - dump all the pages.  */
+	      should_dump_p = 1;
+	    }
 
 	  /* Invoke the callback function to create the corefile segment.  */
-	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+	  if (should_dump_p)
+	    func (addr, endaddr - addr, offset, inode,
+		  read, write, exec, 1, /* MODIFIED is true because we
+					   want to dump the mapping.  */
+		  filename, obfd);
 	}
 
       do_cleanups (cleanup);
@@ -1972,6 +2312,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.  */
 
@@ -2008,4 +2359,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);
 }

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Pedro Alves]

On 03/20/2015 08:02 PM, Sergio Durigan Junior wrote:
>>> >> +     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
>> >
>> > Cannot do it easily, or cannot do it at all?
> It cannot do it easily.  If GDB is run by root, then it could (in
> theory) inspect the contents of /proc/PID/map_files and determine
> whether the " (deleted)" file was a hard link or not.

Ah, could you include that in the comment please?
Otherwise every time I read that I'll wonder what wasn't easy.  :-)

Like:

"(..) example.  The Linux kernel checks to see if "i_nlink == 0", but
GDB cannot easily (and normally) do this check (iff running as root,
it could find the mapping in /proc/PID/map_files/ and determine
whether there still are other hard links to the inode/file).
Therefore, we made a compromise (...)"


I quickly tried something like that with "/proc/pid/fd/*", and
unfortunately the link count is always 1, at least on F20's kernel:

$ tail > a&
[1] 12343

[1]+  Stopped                 tail > a
$ ls -l a
-rw-rw-r--. 1 pedro pedro 0 Mar 20 21:29 a
$ ln a b
$ ls -l a
-rw-rw-r--. 2 pedro pedro 0 Mar 20 21:29 a
$ ls -l /proc/12343/fd/1
l-wx------. 1 pedro pedro 64 Mar 20 21:30 /proc/12343/fd/1 -> /tmp/a

If map_files/ works the same, I think we'd have to get the
file's inode number, and look over the whole filesystem for
other files with that inode number (the equivalent
of 'ls -i' 'find / -inum').  Urgh!

Anyway, with the comment tweak, the patch looks good to me.  Thanks
again for doing this.

Thanks,
Pedro Alves

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Sergio Durigan Junior]

On Friday, March 20 2015, Pedro Alves wrote:

> Thanks, this looks good to me now, just a few minor nits.

Thanks for the review again!

> On 03/19/2015 11:22 PM, Sergio Durigan Junior wrote:
>
>> +/* 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.  */
>
> Thanks for the version info.  Ideally, what I'd like with these version notes
> is at any point in the time in the future being able to look at the code
> and easily identify whether we're working around an old enough kernel that
> we potentially can stop worrying about backwards compatibility with
> such kernel.  If easy, could you also add the kernel version, like
> e.g., "$hash (x.y.z)"?

Yeah.  'git tag --contains 834f82e2aa9a8ede94b17b656329f850c1471514'
shows me that the first release to contain this commit was 3.10.  So I'm
putting it there.

>> +  if (init_regex_p == -1)
>> +    {
>> +      /* There was an error while compiling the regex'es above.  In
>> +	 order to try to give some reliable information to the caller,
>> +	 we just try to find the string "(deleted)" in the filename.
>> +	 If we managed to find it, then we assume the mapping is
>> +	 anonymous.  */
>> +      return strstr (filename, "(deleted)") != NULL;
>
> Might as well add the space before parens, and match tail
> end only, like e.g.,:
>
>    const char deleted[] = " (deleted)";
>    size_t del_len = strlen (deleted);
>    size_t filename_len = strlen (filename);
>
>    return (filename_len >= del_len
>            && strcmp (filename + filename_len - del_len, deleted) == 0);

Done.

>> +
>> +/* 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.
>> +
>> +   In a nutshell, this is the logic that we follow in order to decide
>> +   if a mapping should be dumped or not.
>> +
>> +   - If the mapping is associated to a file whose name ends with "
>> +     (deleted)"
>
> Nit: it'd be good to avoid breaking " (deleted)" over two lines.

Fixed.

>         > , 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
>
> Cannot do it easily, or cannot do it at all?

It cannot do it easily.  If GDB is run by root, then it could (in
theory) inspect the contents of /proc/PID/map_files and determine
whether the " (deleted)" file was a hard link or not.

>> +     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,
>> +     Linux kernels before commit
>> +     834f82e2aa9a8ede94b17b656329f850c1471514 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.  */
>> +
>> +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
>
> outside == the caller?  or the kernel?  The former, right?

Yes, the former.  I improved the comment.

>> +     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.  */
>
> "for now" refers to the flag being updated further down, or future
> kernel versions?

The former.  I improved the comment.

>> +  int private_p = maybe_private_p;
>> +
>> +  /* We always dump vDSO and vsyscall mappings.  */
>
> Suggest:
>
>    /* We always dump vDSO and vsyscall mappings, because it's likely that
>       there'll be no file to read the contents from at core load time.
>       The kernel does the same.  */

Right, improved the comment with you suggestion.

> Thanks,
> Pedro Alves

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

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

	PR corefiles/16092
	* 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_regions_full): New variables
	'coredumpfilter_name', 'coredumpfilterdata', 'pid', 'filterflags',
	'should_dump_p'.  Removed variable 'modified'.  Read
	/proc/<PID>/smaps file; improve parsing of its information.
	Implement memory mapping filtering based on its contents.
	(show_use_coredump_filter): New function.
	(_initialize_linux_tdep): New command 'set use-coredump-filter'.

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ea0d4cd..908c42c 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -35,9 +35,61 @@
 #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,
+  };
+
+/* This struct is used to map flags found in the "VmFlags:" field (in
+   the /proc/<PID>/smaps file).  */
+
+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 +433,229 @@ 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 (3.10) do not have this
+   field on smaps.  */
+
+static void
+decode_vmflags (char *p, struct smaps_vmflags *v)
+{
+  char *saveptr;
+  const 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);
+
+      /* Let's be pessimistic and assume there will be an error while
+	 compiling the regex'es.  */
+      init_regex_p = -1;
+
+      /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or
+	 without the "(deleted)" string in the end).  We know for
+	 sure, based on the Linux kernel code, that memory mappings
+	 whose associated filename is "/dev/zero" are guaranteed to be
+	 MAP_ANONYMOUS.  */
+      compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match /dev/zero "
+			     "filename"));
+      /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or
+	 without the "(deleted)" string in the end).  These filenames
+	 refer to shared memory (shmem), and memory mappings
+	 associated with them are MAP_ANONYMOUS as well.  */
+      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 we reached this point, then everything succeeded.  */
+      init_regex_p = 1;
+    }
+
+  if (init_regex_p == -1)
+    {
+      const char deleted[] = " (deleted)";
+      size_t del_len = sizeof (deleted) - 1;
+      size_t filename_len = strlen (filename);
+
+      /* There was an error while compiling the regex'es above.  In
+	 order to try to give some reliable information to the caller,
+	 we just try to find the string " (deleted)" in the filename.
+	 If we managed to find it, then we assume the mapping is
+	 anonymous.  */
+      return (filename_len >= del_len
+	      && strcmp (filename + filename_len - del_len, deleted) == 0);
+    }
+
+  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.
+
+   In a nutshell, this is the logic that we follow in order to decide
+   if a mapping should be dumped or not.
+
+   - 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,
+     Linux kernels before commit
+     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10) 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.  */
+
+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 our caller.  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 until we take a look at the "VmFlags:" field
+     (assuming that the version of the Linux kernel being used
+     supports it, of course).  */
+  int private_p = maybe_private_p;
+
+  /* We always dump vDSO and vsyscall mappings, because it's likely that
+     there'll be no file to read the contents from at core load time.
+     The kernel does the same.  */
+  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;
+
+      /* Update 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
@@ -819,48 +1094,86 @@ 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;
+	  int has_anonymous = 0;
+	  int should_dump_p = 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);
-
-	  /* 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"))
+	  /* '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 a Linux kernel released before the commit
+	     834f82e2aa9a8ede94b17b656329f850c1471514 (3.10), 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 should be dumped by parsing smaps
+	     counters.  */
+	  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 +1182,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 +1203,38 @@ 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 (has_anonymous)
+	    should_dump_p = dump_mapping_p (filterflags, &v, private,
+					    mapping_anon_p, filename);
+	  else
+	    {
+	      /* Older Linux kernels did not support the "Anonymous:" counter.
+		 If it is missing, we can't be sure - dump all the pages.  */
+	      should_dump_p = 1;
+	    }
 
 	  /* Invoke the callback function to create the corefile segment.  */
-	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+	  if (should_dump_p)
+	    func (addr, endaddr - addr, offset, inode,
+		  read, write, exec, 1, /* MODIFIED is true because we
+					   want to dump the mapping.  */
+		  filename, obfd);
 	}
 
       do_cleanups (cleanup);
@@ -1972,6 +2310,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.  */
 
@@ -2008,4 +2357,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);
 }

Re: [PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Pedro Alves]

Thanks, this looks good to me now, just a few minor nits.

On 03/19/2015 11:22 PM, Sergio Durigan Junior wrote:

> +/* 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.  */

Thanks for the version info.  Ideally, what I'd like with these version notes
is at any point in the time in the future being able to look at the code
and easily identify whether we're working around an old enough kernel that
we potentially can stop worrying about backwards compatibility with
such kernel.  If easy, could you also add the kernel version, like
e.g., "$hash (x.y.z)"?

> +  if (init_regex_p == -1)
> +    {
> +      /* There was an error while compiling the regex'es above.  In
> +	 order to try to give some reliable information to the caller,
> +	 we just try to find the string "(deleted)" in the filename.
> +	 If we managed to find it, then we assume the mapping is
> +	 anonymous.  */
> +      return strstr (filename, "(deleted)") != NULL;

Might as well add the space before parens, and match tail
end only, like e.g.,:

   const char deleted[] = " (deleted)";
   size_t del_len = strlen (deleted);
   size_t filename_len = strlen (filename);

   return (filename_len >= del_len
           && strcmp (filename + filename_len - del_len, deleted) == 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.
> +
> +   In a nutshell, this is the logic that we follow in order to decide
> +   if a mapping should be dumped or not.
> +
> +   - If the mapping is associated to a file whose name ends with "
> +     (deleted)"

Nit: it'd be good to avoid breaking " (deleted)" over two lines.

        > , 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

Cannot do it easily, or cannot do it at all?

> +     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,
> +     Linux kernels before commit
> +     834f82e2aa9a8ede94b17b656329f850c1471514 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.  */
> +
> +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

outside == the caller?  or the kernel?  The former, right?

> +     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.  */

"for now" refers to the flag being updated further down, or future
kernel versions?

> +  int private_p = maybe_private_p;
> +
> +  /* We always dump vDSO and vsyscall mappings.  */

Suggest:

   /* We always dump vDSO and vsyscall mappings, because it's likely that
      there'll be no file to read the contents from at core load time.
      The kernel does the same.  */

Thanks,
Pedro Alves

[PATCH 1/2] Implement support for checking /proc/PID/coredump_filter

[Sergio Durigan Junior]

This is the core patch.  It implements the support for honoring the
/proc/PID/coredump_filter file.

Changes from v3:

  - Removed the new 'enum memory_mapping_state'; now
    linux_find_memory_regions_full is either calling
    gcore_create_callback when the mapping should be dumped (with
    'modified = 1'), or not calling the function at all.

  - Reviewed comments.  Added new variable 'should_dump_p', and
    removed 'modified' from linux_find_memory_regions_full.

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

	PR corefiles/16092
	* 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_regions_full): New variables
	'coredumpfilter_name', 'coredumpfilterdata', 'pid', 'filterflags',
	'should_dump_p'.  Removed variable 'modified'.  Read
	/proc/<PID>/smaps file; improve parsing of its information.
	Implement memory mapping filtering based on its contents.
	(show_use_coredump_filter): New function.
	(_initialize_linux_tdep): New command 'set use-coredump-filter'.
---
 gdb/linux-tdep.c | 408 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 380 insertions(+), 28 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index ea0d4cd..3831580 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -35,9 +35,61 @@
 #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,
+  };
+
+/* This struct is used to map flags found in the "VmFlags:" field (in
+   the /proc/<PID>/smaps file).  */
+
+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 +433,220 @@ 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;
+  const 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);
+
+      /* Let's be pessimistic and assume there will be an error while
+	 compiling the regex'es.  */
+      init_regex_p = -1;
+
+      /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or
+	 without the "(deleted)" string in the end).  We know for
+	 sure, based on the Linux kernel code, that memory mappings
+	 whose associated filename is "/dev/zero" are guaranteed to be
+	 MAP_ANONYMOUS.  */
+      compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$",
+			   _("Could not compile regex to match /dev/zero "
+			     "filename"));
+      /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or
+	 without the "(deleted)" string in the end).  These filenames
+	 refer to shared memory (shmem), and memory mappings
+	 associated with them are MAP_ANONYMOUS as well.  */
+      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 we reached this point, then everything succeeded.  */
+      init_regex_p = 1;
+    }
+
+  if (init_regex_p == -1)
+    {
+      /* There was an error while compiling the regex'es above.  In
+	 order to try to give some reliable information to the caller,
+	 we just try to find the string "(deleted)" in the filename.
+	 If we managed to find it, then we assume the mapping is
+	 anonymous.  */
+      return strstr (filename, "(deleted)") != NULL;
+    }
+
+  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.
+
+   In a nutshell, this is the logic that we follow in order to decide
+   if a mapping should be dumped or not.
+
+   - 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,
+     Linux kernels before commit
+     834f82e2aa9a8ede94b17b656329f850c1471514 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.  */
+
+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;
+
+      /* Update 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
@@ -819,48 +1085,86 @@ 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;
+	  int has_anonymous = 0;
+	  int should_dump_p = 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);
-
-	  /* 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"))
+	  /* '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 a Linux kernel released before the commit
+	     834f82e2aa9a8ede94b17b656329f850c1471514, 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 should be dumped by parsing smaps
+	     counters.  */
+	  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 +1173,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 +1194,38 @@ 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 (has_anonymous)
+	    should_dump_p = dump_mapping_p (filterflags, &v, private,
+					    mapping_anon_p, filename);
+	  else
+	    {
+	      /* Older Linux kernels did not support the "Anonymous:" counter.
+		 If it is missing, we can't be sure - dump all the pages.  */
+	      should_dump_p = 1;
+	    }
 
 	  /* Invoke the callback function to create the corefile segment.  */
-	  func (addr, endaddr - addr, offset, inode,
-		read, write, exec, modified, filename, obfd);
+	  if (should_dump_p)
+	    func (addr, endaddr - addr, offset, inode,
+		  read, write, exec, 1, /* MODIFIED is true because we
+					   want to dump the mapping.  */
+		  filename, obfd);
 	}
 
       do_cleanups (cleanup);
@@ -1972,6 +2301,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.  */
 
@@ -2008,4 +2348,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);
 }
-- 
1.9.3