[PATCH 2/4] Document new 'set honor-dontdump-flag' command

[PATCH 2/4] Document new 'set honor-dontdump-flag' command

[Sergio Lopez]

2017-11-28  Sergio Lopez  <slp@redhat.com>

        * gdb.texinfo (gcore): Mention new command 'set
        honor-dontdump-flag'.
        (set honor-dontdump-flag): Document new command.
---
 gdb/doc/ChangeLog   |  6 ++++++
 gdb/doc/gdb.texinfo | 14 +++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 2a1eb76d15..cdde49a887 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2017-11-28  Sergio Lopez  <slp@redhat.com>
+
+	* gdb.texinfo (gcore): Mention new command 'set
+	honor-dontdump-flag'.
+	(set honor-dontdump-flag): Document new command.
+
 2017-11-26  Dominik Czarnota  <dominik.b.czarnota@gmail.com>
 
 	PR gdb/21945
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 675f6e7bc8..472a0fe8cc 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11543,7 +11543,9 @@ 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}).
+dump (@pxref{set use-coredump-filter}), and will honor the VM_DONTDUMP
+flag for sections where is present in the file
+@file{/proc/@var{pid}/smaps} (@pxref{set honor-dontdump-flag}).
 
 @kindex set use-coredump-filter
 @anchor{set use-coredump-filter}
@@ -11573,6 +11575,16 @@ 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.
+
+@kindex set honor-dontdump-flag
+@anchor{set honor-dontdump-flag}
+@item set honor-dontdump-flag on
+@itemx set honor-dontdump-flag off
+If @code{on} is specified, GDB will not dump memory regions marked
+with the VM_DONTDUMP flag. This flag is represented in
+@file{/proc/@var{pid}/smaps} with the acronym @code{dd}.
+
+The default value is @code{on}.
 @end table
 
 @node Character Sets
-- 
2.14.3

[PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Lopez]

Commit df8411da087dc05481926f4c4a82deabc5bc3859 implemented support for
checking /proc/PID/coredump_filter, and also changed gcore behavior to
unconditionally honor the VM_DONTDUMP flag, preventing sections marked
as such for being dumped into the core file.

This patch adds support for the 'set honor-dontdump-flag' command for
instructing gdb to ignore the VM_DONTDUMP flag. Combined with 'set
use-coredump-filter', this allows the user to restore the old behavior,
dumping all sections (except the ones marked as IO) unconditionally.

ChangeLog
2017-11-28  Sergio Lopez  <slp@redhat.com>

        * linux-tdep.c (honor_dontdump_flag): New variable.
        (dump_mapping_p): Use honor_dontdump_flag variable.
        (_initialize_linux_tdep): New command 'set honor-dontdump-flag'.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/linux-tdep.c | 19 ++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ebb969998c..ce68fee776 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-11-28  Sergio Lopez  <slp@redhat.com>
+
+	* linux-tdep.c (honor_dontdump_flag): New variable.
+	(dump_mapping_p): Use honor_dontdump_flag variable.
+	(_initialize_linux_tdep): New command 'set honor-dontdump-flag'.
+
 2017-11-27  Tom Tromey  <tom@tromey.com>
 
 	* Makefile.in (REMOTE_OBS): Remove.
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 24237b8d39..5f4a1cdad1 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -93,6 +93,11 @@ struct smaps_vmflags
 
 static int use_coredump_filter = 1;
 
+/* Whether to honor the VM_DONTDUMP flag in /proc/PID/smaps when
+   generating a corefile.  */
+
+static int honor_dontdump_flag = 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
@@ -655,7 +660,7 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
 	return 0;
 
       /* Check if we should exclude this mapping.  */
-      if (v->exclude_coredump)
+      if (honor_dontdump_flag && v->exclude_coredump)
 	return 0;
 
       /* Update our notion of whether this mapping is shared or
@@ -2517,4 +2522,16 @@ of /proc/PID/coredump_filter when generating the corefile.  For more information
 about this file, refer to the manpage of core(5)."),
 			   NULL, show_use_coredump_filter,
 			   &setlist, &showlist);
+
+  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
+			   &honor_dontdump_flag, _("\
+Set whether gcore should honor the VM_DONTDUMP flag."),
+			   _("\
+Show whether gcore should honor the VM_DONTDUMP flag."),
+			   _("\
+Use this command to set whether gcore should honor the VM_DONTDUMP\n\
+flag from /proc/PID/smaps when generating the corefile.  For more information\n\
+about this file, refer to the manpage of proc(5) and core(5)."),
+			   NULL, show_use_coredump_filter,
+			   &setlist, &showlist);
 }
-- 
2.14.3

[PATCH 4/4] Document the new "-a" argument for gcore

[Sergio Lopez]

2017-11-28  Sergio Lopez  <slp@redhat.com>
        * gdb.texinfo (gcore man): Document new "-a" argument.
---
 gdb/doc/ChangeLog   | 3 +++
 gdb/doc/gdb.texinfo | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index cdde49a887..491acb0a9a 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,6 @@
+2017-11-28  Sergio Lopez  <slp@redhat.com>
+	* gdb.texinfo (gcore man): Document new "-a" argument.
+
 2017-11-28  Sergio Lopez  <slp@redhat.com>
 
 	* gdb.texinfo (gcore): Mention new command 'set
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 472a0fe8cc..c0146c66dc 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42962,6 +42962,10 @@ running without any change.
 
 @c man begin OPTIONS gcore
 @table @env
+@item -a
+Instruct GDB to unconditionally dump all sections (except IO), ignoring the
+value of @file{/proc/@var{pid}/coredump_filter} and the VM_DONTDUMP flag.
+
 @item -o @var{filename}
 The optional argument
 @var{filename} specifies the file name where to put the core dump.
-- 
2.14.3

[PATCH 3/4] Add "-a" argument to gcore.in

[Sergio Lopez]

The new "-a" argument instructs the gcore script to disable both
use-coredump-filter and honor-dontdump-flag before generating the core
file.

This allows the user to unconditionally dump all memory regions,
mimicking the behavior of previous gdb/gcore versions (before support
for coredump_filter was implemented).

2017-11-28  Sergio Lopez  <slp@redhat.com>
        * gcore.in: Add "-a" argument for disabling both
        use-coredump-filter and honor-dontdump-flag.
---
 gdb/ChangeLog |  4 ++++
 gdb/gcore.in  | 25 +++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ce68fee776..ba8826c84c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-11-28  Sergio Lopez  <slp@redhat.com>
+	* gcore.in: Add "-a" argument for disabling both
+	use-coredump-filter and honor-dontdump-flag.
+
 2017-11-28  Sergio Lopez  <slp@redhat.com>
 
 	* linux-tdep.c (honor_dontdump_flag): New variable.
diff --git a/gdb/gcore.in b/gdb/gcore.in
index 632f21bdfa..32e597dc4f 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -22,10 +22,29 @@
 
 if [ "$#" -eq "0" ]
 then
-    echo "usage:  @GCORE_TRANSFORM_NAME@ [-o filename] pid"
+    echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
     exit 2
 fi
 
+# Check for -a option, default to honor coredump_filter and VM_DONTDUMP.
+use_coredump_filter=on
+honor_dontdump_flag=on
+
+if [ "$1" = "-a" ]
+then
+    if [ "$#" -lt "2" ]
+    then
+        # Not enough arguments.
+	echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
+	exit 2
+    fi
+    use_coredump_filter=off
+    honor_dontdump_flag=off
+
+    # Shift over to the next argument
+    shift;
+fi
+
 # Need to check for -o option, but set default basename to "core".
 name=core
 
@@ -34,7 +53,7 @@ then
     if [ "$#" -lt "3" ]
     then
 	# Not enough arguments.
-	echo "usage:  @GCORE_TRANSFORM_NAME@ [-o filename] pid"
+	echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
 	exit 2
     fi
     name=$2
@@ -87,6 +106,8 @@ do
 	# available but not accessible as GDB would get stopped on SIGTTIN.
 	$binary_path/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
 	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
+	    -ex "set use-coredump-filter $use_coredump_filter" \
+	    -ex "set honor-dontdump-flag $honor_dontdump_flag" \
 	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
 
 	if [ -r $name.$pid ] ; then 
-- 
2.14.3

[PATCH 0/4] Enable the user to dump all memory regions

[Sergio Lopez]

GDB versions prior to df8411da087dc05481926f4c4a82deabc5bc3859
unconditionally included all memory regions in the core dump.

After that change, while is still possible to ask GDB to ignore
/proc/PID/coredump_filter using the 'set use-coredump-filter' command,
there's no way to request it to dump regions marked with the VM_DONTDUMP
flag ("dd" in /proc/PID/smaps").

This patch series implement the new 'set honor-dontdump-flag' command
for GDB, and the "-a" argument for gcore, allowing the user to mimic the
behavior of previous GDB versions.

Sergio Lopez (4):
  Implement 'set honor-dontdump-flag' command
  Document new 'set honor-dontdump-flag' command
  Add "-a" argument to gcore.in
  Document the new "-a" argument for gcore

 gdb/ChangeLog       | 10 ++++++++++
 gdb/doc/ChangeLog   |  9 +++++++++
 gdb/doc/gdb.texinfo | 18 +++++++++++++++++-
 gdb/gcore.in        | 25 +++++++++++++++++++++++--
 gdb/linux-tdep.c    | 19 ++++++++++++++++++-
 5 files changed, 77 insertions(+), 4 deletions(-)

-- 
2.14.3

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Lopez]

On Tue, Nov 28, 2017 at 2:21 PM, Sergio Lopez <slp@redhat.com> wrote:
> +  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
> +                          &honor_dontdump_flag, _("\
> +Set whether gcore should honor the VM_DONTDUMP flag."),
> +                          _("\
> +Show whether gcore should honor the VM_DONTDUMP flag."),
> +                          _("\
> +Use this command to set whether gcore should honor the VM_DONTDUMP\n\
> +flag from /proc/PID/smaps when generating the corefile.  For more information\n\
> +about this file, refer to the manpage of proc(5) and core(5)."),
> +                          NULL, show_use_coredump_filter,
> +                          &setlist, &showlist);
>  }

A colleague spotted that I forgot to implement and use a different
"show" function for honor-dontdump-flag, wrongly reusing
show_use_coredump_filter.

I'm going to wait for feedback on the rest of the patchset. Will post
a v2 afterward.

-- 
Sergio

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Sergio Lopez wrote:

> Commit df8411da087dc05481926f4c4a82deabc5bc3859 implemented support for
> checking /proc/PID/coredump_filter, and also changed gcore behavior to
> unconditionally honor the VM_DONTDUMP flag, preventing sections marked
> as such for being dumped into the core file.
>
> This patch adds support for the 'set honor-dontdump-flag' command for
> instructing gdb to ignore the VM_DONTDUMP flag. Combined with 'set
> use-coredump-filter', this allows the user to restore the old behavior,
> dumping all sections (except the ones marked as IO) unconditionally.

Thanks for the patch.  A few comments below.

> ChangeLog

This should say "gdb/ChangeLog:".

> 2017-11-28  Sergio Lopez  <slp@redhat.com>
>
>         * linux-tdep.c (honor_dontdump_flag): New variable.
>         (dump_mapping_p): Use honor_dontdump_flag variable.
>         (_initialize_linux_tdep): New command 'set honor-dontdump-flag'.

Not sure if it was your MUA, but ChangeLog lines should be indented
using TABs.

> ---
>  gdb/ChangeLog    |  6 ++++++
>  gdb/linux-tdep.c | 19 ++++++++++++++++++-
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index ebb969998c..ce68fee776 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-11-28  Sergio Lopez  <slp@redhat.com>
> +
> +	* linux-tdep.c (honor_dontdump_flag): New variable.
> +	(dump_mapping_p): Use honor_dontdump_flag variable.
> +	(_initialize_linux_tdep): New command 'set honor-dontdump-flag'.

Here they're fine.  We don't usually include diffs to ChangeLogs in the
commit because they can cause conflicts when applying the patch, but
it's up to you.

> +
>  2017-11-27  Tom Tromey  <tom@tromey.com>
>  
>  	* Makefile.in (REMOTE_OBS): Remove.
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 24237b8d39..5f4a1cdad1 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -93,6 +93,11 @@ struct smaps_vmflags
>  
>  static int use_coredump_filter = 1;
>  
> +/* Whether to honor the VM_DONTDUMP flag in /proc/PID/smaps when
> +   generating a corefile.  */
> +
> +static int honor_dontdump_flag = 1;

No empty line between command and definition of variable.

> +
>  /* 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
> @@ -655,7 +660,7 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
>  	return 0;
>  
>        /* Check if we should exclude this mapping.  */
> -      if (v->exclude_coredump)
> +      if (honor_dontdump_flag && v->exclude_coredump)
>  	return 0;
>  
>        /* Update our notion of whether this mapping is shared or
> @@ -2517,4 +2522,16 @@ of /proc/PID/coredump_filter when generating the corefile.  For more information
>  about this file, refer to the manpage of core(5)."),
>  			   NULL, show_use_coredump_filter,
>  			   &setlist, &showlist);
> +
> +  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
> +			   &honor_dontdump_flag, _("\
> +Set whether gcore should honor the VM_DONTDUMP flag."),
> +			   _("\
> +Show whether gcore should honor the VM_DONTDUMP flag."),
> +			   _("\
> +Use this command to set whether gcore should honor the VM_DONTDUMP\n\
> +flag from /proc/PID/smaps when generating the corefile.  For more information\n\
> +about this file, refer to the manpage of proc(5) and core(5)."),
> +			   NULL, show_use_coredump_filter,

You've already spotted the mistake of using 'show_use_coredump_filter'
here.

> +			   &setlist, &showlist);

I'm not sure "honor-dontdump-flag" is a good name for this setting.
There's no indication that it relates to coredumps, and I think it
should.  A name like "honor-coredump-dontdump-flag" is a bit repetitive,
but IMHO is better than just "honor-dontdump-flag".  WDYT?

To be fair, my first thought was to suggest adding a new "set coredump"
super command, which would encompass "set coredump use-coredump-filter"
and "set coredump honor-dontdump-flag", but I wouldn't like to introduce
this change so close to a release.

Otherwise, the patch looks OK to me, but I'm not a global maintainer.

Thanks,

-- 
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: [PATCH 2/4] Document new 'set honor-dontdump-flag' command

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Sergio Lopez wrote:

> 2017-11-28  Sergio Lopez  <slp@redhat.com>
>
>         * gdb.texinfo (gcore): Mention new command 'set
>         honor-dontdump-flag'.
>         (set honor-dontdump-flag): Document new command.
> ---
>  gdb/doc/ChangeLog   |  6 ++++++
>  gdb/doc/gdb.texinfo | 14 +++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 2a1eb76d15..cdde49a887 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-11-28  Sergio Lopez  <slp@redhat.com>
> +
> +	* gdb.texinfo (gcore): Mention new command 'set
> +	honor-dontdump-flag'.
> +	(set honor-dontdump-flag): Document new command.
> +
>  2017-11-26  Dominik Czarnota  <dominik.b.czarnota@gmail.com>
>  
>  	PR gdb/21945
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 675f6e7bc8..472a0fe8cc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -11543,7 +11543,9 @@ 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}).
> +dump (@pxref{set use-coredump-filter}), and will honor the VM_DONTDUMP
> +flag for sections where is present in the file
> +@file{/proc/@var{pid}/smaps} (@pxref{set honor-dontdump-flag}).
>  
>  @kindex set use-coredump-filter
>  @anchor{set use-coredump-filter}
> @@ -11573,6 +11575,16 @@ 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.
> +
> +@kindex set honor-dontdump-flag
> +@anchor{set honor-dontdump-flag}
> +@item set honor-dontdump-flag on
> +@itemx set honor-dontdump-flag off
> +If @code{on} is specified, GDB will not dump memory regions marked
> +with the VM_DONTDUMP flag. This flag is represented in
                            ^^^

Two spaces after period.

I also think 'VM_DONTDUMP' should be marked as @code{}.

> +@file{/proc/@var{pid}/smaps} with the acronym @code{dd}.
> +
> +The default value is @code{on}.
>  @end table
>  
>  @node Character Sets
> -- 
> 2.14.3

-- 
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: [PATCH 4/4] Document the new "-a" argument for gcore

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Sergio Lopez wrote:

> 2017-11-28  Sergio Lopez  <slp@redhat.com>
>         * gdb.texinfo (gcore man): Document new "-a" argument.
> ---
>  gdb/doc/ChangeLog   | 3 +++
>  gdb/doc/gdb.texinfo | 4 ++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index cdde49a887..491acb0a9a 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,6 @@
> +2017-11-28  Sergio Lopez  <slp@redhat.com>
> +	* gdb.texinfo (gcore man): Document new "-a" argument.
> +
>  2017-11-28  Sergio Lopez  <slp@redhat.com>
>  
>  	* gdb.texinfo (gcore): Mention new command 'set
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 472a0fe8cc..c0146c66dc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42962,6 +42962,10 @@ running without any change.
>  
>  @c man begin OPTIONS gcore
>  @table @env
> +@item -a
> +Instruct GDB to unconditionally dump all sections (except IO), ignoring the
> +value of @file{/proc/@var{pid}/coredump_filter} and the VM_DONTDUMP flag.

@code{} around 'VM_DONTDUMP'.

> +
>  @item -o @var{filename}
>  The optional argument
>  @var{filename} specifies the file name where to put the core dump.
> -- 
> 2.14.3

-- 
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: [PATCH 3/4] Add "-a" argument to gcore.in

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Sergio Lopez wrote:

> The new "-a" argument instructs the gcore script to disable both
> use-coredump-filter and honor-dontdump-flag before generating the core
> file.
>
> This allows the user to unconditionally dump all memory regions,
> mimicking the behavior of previous gdb/gcore versions (before support
> for coredump_filter was implemented).



> 2017-11-28  Sergio Lopez  <slp@redhat.com>
>         * gcore.in: Add "-a" argument for disabling both
>         use-coredump-filter and honor-dontdump-flag.
> ---
>  gdb/ChangeLog |  4 ++++
>  gdb/gcore.in  | 25 +++++++++++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index ce68fee776..ba8826c84c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-11-28  Sergio Lopez  <slp@redhat.com>
> +	* gcore.in: Add "-a" argument for disabling both
> +	use-coredump-filter and honor-dontdump-flag.
> +
>  2017-11-28  Sergio Lopez  <slp@redhat.com>
>  
>  	* linux-tdep.c (honor_dontdump_flag): New variable.
> diff --git a/gdb/gcore.in b/gdb/gcore.in
> index 632f21bdfa..32e597dc4f 100644
> --- a/gdb/gcore.in
> +++ b/gdb/gcore.in
> @@ -22,10 +22,29 @@
>  
>  if [ "$#" -eq "0" ]
>  then
> -    echo "usage:  @GCORE_TRANSFORM_NAME@ [-o filename] pid"
> +    echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
>      exit 2
>  fi
>  
> +# Check for -a option, default to honor coredump_filter and VM_DONTDUMP.
> +use_coredump_filter=on
> +honor_dontdump_flag=on
> +
> +if [ "$1" = "-a" ]
> +then
> +    if [ "$#" -lt "2" ]
> +    then
> +        # Not enough arguments.
> +	echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
> +	exit 2
> +    fi
> +    use_coredump_filter=off
> +    honor_dontdump_flag=off
> +
> +    # Shift over to the next argument
> +    shift;
> +fi

This is forcing the '-a' flag to come first, right?  Can you implement a
better argument parsing here, so that the user can pass '-a' anywhere in
the command line?  You can use getopt or even do your own loop.

> +
>  # Need to check for -o option, but set default basename to "core".
>  name=core
>  
> @@ -34,7 +53,7 @@ then
>      if [ "$#" -lt "3" ]
>      then
>  	# Not enough arguments.
> -	echo "usage:  @GCORE_TRANSFORM_NAME@ [-o filename] pid"
> +	echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o filename] pid"
>  	exit 2
>      fi
>      name=$2
> @@ -87,6 +106,8 @@ do
>  	# available but not accessible as GDB would get stopped on SIGTTIN.
>  	$binary_path/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>  	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
> +	    -ex "set use-coredump-filter $use_coredump_filter" \
> +	    -ex "set honor-dontdump-flag $honor_dontdump_flag" \
>  	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
>  
>  	if [ -r $name.$pid ] ; then 
> -- 
> 2.14.3

Thanks,

-- 
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: [PATCH 0/4] Enable the user to dump all memory regions

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Sergio Lopez wrote:

> GDB versions prior to df8411da087dc05481926f4c4a82deabc5bc3859
> unconditionally included all memory regions in the core dump.
>
> After that change, while is still possible to ask GDB to ignore
> /proc/PID/coredump_filter using the 'set use-coredump-filter' command,
> there's no way to request it to dump regions marked with the VM_DONTDUMP
> flag ("dd" in /proc/PID/smaps").
>
> This patch series implement the new 'set honor-dontdump-flag' command
> for GDB, and the "-a" argument for gcore, allowing the user to mimic the
> behavior of previous GDB versions.

Thanks for the patch, Sergio.

Overall I agree with the approach, but I've made a few comments here and
there about things I think should be addressed.  The most important of
them is the command name.

I would really like to see a test for this.  We already carry a testcase
for the use-coredump-filter command (see gdb.base/coredump-filter.exp),
so I think you can just extend it to test this specific feature.

Thanks,

> Sergio Lopez (4):
>   Implement 'set honor-dontdump-flag' command
>   Document new 'set honor-dontdump-flag' command
>   Add "-a" argument to gcore.in
>   Document the new "-a" argument for gcore
>
>  gdb/ChangeLog       | 10 ++++++++++
>  gdb/doc/ChangeLog   |  9 +++++++++
>  gdb/doc/gdb.texinfo | 18 +++++++++++++++++-
>  gdb/gcore.in        | 25 +++++++++++++++++++++++--
>  gdb/linux-tdep.c    | 19 ++++++++++++++++++-
>  5 files changed, 77 insertions(+), 4 deletions(-)
>
> -- 
> 2.14.3

-- 
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: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Pedro Alves]

On 11/28/2017 03:42 PM, Sergio Durigan Junior wrote:

>> @@ -2517,4 +2522,16 @@ of /proc/PID/coredump_filter when generating the corefile.  For more information
>>  about this file, refer to the manpage of core(5)."),
>>  			   NULL, show_use_coredump_filter,
>>  			   &setlist, &showlist);
>> +
>> +  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
>> +			   &honor_dontdump_flag, _("\
>> +Set whether gcore should honor the VM_DONTDUMP flag."),
>> +			   _("\
>> +Show whether gcore should honor the VM_DONTDUMP flag."),
>> +			   _("\
>> +Use this command to set whether gcore should honor the VM_DONTDUMP\n\
>> +flag from /proc/PID/smaps when generating the corefile.  For more information\n\
>> +about this file, refer to the manpage of proc(5) and core(5)."),
>> +			   NULL, show_use_coredump_filter,
> 
> You've already spotted the mistake of using 'show_use_coredump_filter'
> here.
> 
>> +			   &setlist, &showlist);
> 
> I'm not sure "honor-dontdump-flag" is a good name for this setting.
> There's no indication that it relates to coredumps, and I think it
> should.  A name like "honor-coredump-dontdump-flag" is a bit repetitive,
> but IMHO is better than just "honor-dontdump-flag".  WDYT?

Personally, I don't find the "coredump" issue that much of a
problem, given "dump" is there.  "honor-coredump-dontdump" looks
like a mouthful to me.

I think using "ignore" in the name would some more natural
in GDB than "honor".  I.e., "set ignore-dontdump-flag on/off".
That mean default is off/0 ( and the control variable could go
to .bss :-) )

It also avoids our UK friends cursing at bad spelling of "honour".  :-)

"set dump-excluded-mappings on/off" could work too?

I skimmed the series and didn't find a gdb/NEWS entry; we need
one for new commands.

> 
> To be fair, my first thought was to suggest adding a new "set coredump"
> super command, which would encompass "set coredump use-coredump-filter"
> and "set coredump honor-dontdump-flag", but I wouldn't like to introduce
> this change so close to a release.
> 
> Otherwise, the patch looks OK to me, but I'm not a global maintainer.
> 

Thanks,
Pedro Alves

Re: [PATCH 2/4] Document new 'set honor-dontdump-flag' command

[Pedro Alves]

Hi Sergio,

Thanks much for scratching your own itch, btw.

>  2017-11-26  Dominik Czarnota  <dominik.b.czarnota@gmail.com>
>  
>  	PR gdb/21945
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 675f6e7bc8..472a0fe8cc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -11543,7 +11543,9 @@ 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}).
> +dump (@pxref{set use-coredump-filter}), and will honor the VM_DONTDUMP
> +flag for sections where is present in the file
> +@file{/proc/@var{pid}/smaps} (@pxref{set honor-dontdump-flag}).

s/will honor/by default honors/
s/sections where is/mappings where it is/

Thanks,
Pedro Alves

Re: [PATCH 4/4] Document the new "-a" argument for gcore

[Eli Zaretskii]

> From: Sergio Lopez <slp@redhat.com>
> Cc: Sergio Lopez <slp@redhat.com>
> Date: Tue, 28 Nov 2017 14:21:48 +0100
> 
> +@item -a
> +Instruct GDB to unconditionally dump all sections (except IO), ignoring the
            ^^^
"@value{GDBN}"

> +value of @file{/proc/@var{pid}/coredump_filter} and the VM_DONTDUMP flag.

Is this Linux-specific?  Because AFAIK 'gcore' isn't, and so we need
to document that this switch and the details you've put into its
description are specific to Linux.

Also, VM_DONTDUMP should be in @code.

Thanks.

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Durigan Junior]

On Tuesday, November 28 2017, Pedro Alves wrote:

> On 11/28/2017 03:42 PM, Sergio Durigan Junior wrote:
>
>>> @@ -2517,4 +2522,16 @@ of /proc/PID/coredump_filter when generating the corefile.  For more information
>>>  about this file, refer to the manpage of core(5)."),
>>>  			   NULL, show_use_coredump_filter,
>>>  			   &setlist, &showlist);
>>> +
>>> +  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
>>> +			   &honor_dontdump_flag, _("\
>>> +Set whether gcore should honor the VM_DONTDUMP flag."),
>>> +			   _("\
>>> +Show whether gcore should honor the VM_DONTDUMP flag."),
>>> +			   _("\
>>> +Use this command to set whether gcore should honor the VM_DONTDUMP\n\
>>> +flag from /proc/PID/smaps when generating the corefile.  For more information\n\
>>> +about this file, refer to the manpage of proc(5) and core(5)."),
>>> +			   NULL, show_use_coredump_filter,
>> 
>> You've already spotted the mistake of using 'show_use_coredump_filter'
>> here.
>> 
>>> +			   &setlist, &showlist);
>> 
>> I'm not sure "honor-dontdump-flag" is a good name for this setting.
>> There's no indication that it relates to coredumps, and I think it
>> should.  A name like "honor-coredump-dontdump-flag" is a bit repetitive,
>> but IMHO is better than just "honor-dontdump-flag".  WDYT?
>
> Personally, I don't find the "coredump" issue that much of a
> problem, given "dump" is there.  "honor-coredump-dontdump" looks
> like a mouthful to me.

It is a mouthful, but at least it leaves no room for doubt.  The "dump"
in the name doesn't necessarily translate to "coredump", I think.  Or at
least that's what I would feel.

> I think using "ignore" in the name would some more natural
> in GDB than "honor".  I.e., "set ignore-dontdump-flag on/off".
> That mean default is off/0 ( and the control variable could go
> to .bss :-) )
>
> It also avoids our UK friends cursing at bad spelling of "honour".  :-)
>
> "set dump-excluded-mappings on/off" could work too?

This one is better IMO.

-- 
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: [PATCH 2/4] Document new 'set honor-dontdump-flag' command

[Eli Zaretskii]

> From: Sergio Lopez <slp@redhat.com>
> Cc: Sergio Lopez <slp@redhat.com>
> Date: Tue, 28 Nov 2017 14:21:46 +0100
> 
>  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}).
> +dump (@pxref{set use-coredump-filter}), and will honor the VM_DONTDUMP

VM_DONTDUMP should be in @code.

> +@kindex set honor-dontdump-flag
> +@anchor{set honor-dontdump-flag}
> +@item set honor-dontdump-flag on
> +@itemx set honor-dontdump-flag off
> +If @code{on} is specified, GDB will not dump memory regions marked

"@value{GDBN}"

> +with the VM_DONTDUMP flag. This flag is represented in
            ^^^^^^^^^^^
@code{VM_DONTDUMP}, and 2 spaces between sentences.

What about a NEWS entry?

Thanks.

Re: [PATCH 4/4] Document the new "-a" argument for gcore

[John Baldwin]

On Tuesday, November 28, 2017 06:35:37 PM Eli Zaretskii wrote:
> > From: Sergio Lopez <slp@redhat.com>
> > Cc: Sergio Lopez <slp@redhat.com>
> > Date: Tue, 28 Nov 2017 14:21:48 +0100
> > 
> > +@item -a
> > +Instruct GDB to unconditionally dump all sections (except IO), ignoring the
>             ^^^
> "@value{GDBN}"
> 
> > +value of @file{/proc/@var{pid}/coredump_filter} and the VM_DONTDUMP flag.
> 
> Is this Linux-specific?  Because AFAIK 'gcore' isn't, and so we need
> to document that this switch and the details you've put into its
> description are specific to Linux.
> 
> Also, VM_DONTDUMP should be in @code.

It is OS-specific.  FreeBSD has a similar notion (it excludes memory regions
marked with a KVME_FLAG_NOCOREDUMP in fbsd_find_memory_regions in fbsd-nat.c
that are created by passing MAP_NOCORE to mmap()).  I could make FreeBSD's
native target honor the same flag name once this is pushed in, though
FreeBSD's kernel always honors the NOCOREDUMP flag (there is no way to force
a kernel-generated coredump to include those regions).

-- 
John Baldwin

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Lopez]

On Tue, Nov 28, 2017 at 4:42 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Tuesday, November 28 2017, Sergio Lopez wrote:
>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>> index 24237b8d39..5f4a1cdad1 100644
>> --- a/gdb/linux-tdep.c
>> +++ b/gdb/linux-tdep.c
>> @@ -93,6 +93,11 @@ struct smaps_vmflags
>>
>>  static int use_coredump_filter = 1;
>>
>> +/* Whether to honor the VM_DONTDUMP flag in /proc/PID/smaps when
>> +   generating a corefile.  */
>> +
>> +static int honor_dontdump_flag = 1;
>
> No empty line between command and definition of variable.

This is the only suggestion that I haven't applied because it would
break the coding style of the previous lines:

gdb/linux-tdep.c:
  89   };
  90
  91 /* Whether to take the /proc/PID/coredump_filter into account when
  92    generating a corefile.  */
  93
  94 static int use_coredump_filter = 1;
  95
  96 /* Whether the value of smaps_vmflags->exclude_coredump should be
  97    ignored, including mappings marked with the VM_DONTDUMP flag in
  98    the dump.  */
  99
 100 static int dump_excluded_mappings = 0;
 101
 102 /* This enum represents the signals' numbers on a generic architecture

-- 
Sergio

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Durigan Junior]

On Wednesday, November 29 2017, Sergio Lopez wrote:

> On Tue, Nov 28, 2017 at 4:42 PM, Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>> On Tuesday, November 28 2017, Sergio Lopez wrote:
>>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>>> index 24237b8d39..5f4a1cdad1 100644
>>> --- a/gdb/linux-tdep.c
>>> +++ b/gdb/linux-tdep.c
>>> @@ -93,6 +93,11 @@ struct smaps_vmflags
>>>
>>>  static int use_coredump_filter = 1;
>>>
>>> +/* Whether to honor the VM_DONTDUMP flag in /proc/PID/smaps when
>>> +   generating a corefile.  */
>>> +
>>> +static int honor_dontdump_flag = 1;
>>
>> No empty line between command and definition of variable.
>
> This is the only suggestion that I haven't applied because it would
> break the coding style of the previous lines:
>
> gdb/linux-tdep.c:
>   89   };
>   90
>   91 /* Whether to take the /proc/PID/coredump_filter into account when
>   92    generating a corefile.  */
>   93
>   94 static int use_coredump_filter = 1;
>   95
>   96 /* Whether the value of smaps_vmflags->exclude_coredump should be
>   97    ignored, including mappings marked with the VM_DONTDUMP flag in
>   98    the dump.  */
>   99
>  100 static int dump_excluded_mappings = 0;
>  101
>  102 /* This enum represents the signals' numbers on a generic architecture

The previous line is the one breaking our coding style.  Unfortunately
GDB is full of these inconsistencies, but please remove the empty line
in your patch anyway.

Thanks,

-- 
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: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Lopez]

On Wed, Nov 29, 2017 at 8:39 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> The previous line is the one breaking our coding style.  Unfortunately
> GDB is full of these inconsistencies, but please remove the empty line
> in your patch anyway.

Ack, not a problem.

Is there something else you'd like to see fixed in v3? In case you
don't, do you prefer me to send the whole patchset as v3, or just
patch 1/4?

-- 
Sergio

Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Sergio Durigan Junior]

On Wednesday, November 29 2017, Sergio Lopez wrote:

> On Wed, Nov 29, 2017 at 8:39 PM, Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>> The previous line is the one breaking our coding style.  Unfortunately
>> GDB is full of these inconsistencies, but please remove the empty line
>> in your patch anyway.
>
> Ack, not a problem.
>
> Is there something else you'd like to see fixed in v3? In case you
> don't, do you prefer me to send the whole patchset as v3, or just
> patch 1/4?

The only thing I'd like is a testcase for this feature.  This is
required since it is a regression.  If you need any help to write one,
please let me know.

Once you have a testcase, you could just send patch 1/4 again.

Thanks,

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