[PATCH] gdb: Add switch to disable DWARF stack unwinders

[PATCH] gdb: Add switch to disable DWARF stack unwinders

[Andrew Burgess]

Add a maintenance command to disable the DWARF stack unwinders.
Normal users would not need this feature, but it is useful to allow
extended testing of fallback stack unwinding strategies, for example,
prologue scanners.

gdb/ChangeLog:

	* dwarf2-frame-tailcall.c (tailcall_frame_sniffer): Exit early if
	dwarf unwinders are disabled.
	* dwarf2-frame.c (dwarf2_frame_sniffer): Likewise.
	(dwarf2_frame_unwinders_enabled_p): Define.
	(dwarf2_append_unwinders): Enable dwarf unwinders when they are
	first registered.
	* dwarf2-frame.h (dwarf2_frame_unwinders_enabled_p): Declare.
	* dwarf2read.c: Add dwarf2-frame.h include.
	(show_dwarf_unwinders_enabled_p): New function.
	(_initialize_dwarf2_read): Register switch to control unwinder
	use.
	* NEWS: Document new feature.

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Add description of
	maintenance command to control dwarf unwinders.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Add check that dwarf unwinders control flag
	is visible.
---
 gdb/ChangeLog                    | 15 +++++++++++++++
 gdb/NEWS                         |  6 ++++++
 gdb/doc/ChangeLog                |  5 +++++
 gdb/doc/gdb.texinfo              | 24 ++++++++++++++++++++++++
 gdb/dwarf2-frame-tailcall.c      |  3 +++
 gdb/dwarf2-frame.c               |  9 +++++++++
 gdb/dwarf2-frame.h               |  6 ++++++
 gdb/dwarf2read.c                 | 26 ++++++++++++++++++++++++++
 gdb/testsuite/ChangeLog          |  5 +++++
 gdb/testsuite/gdb.base/maint.exp |  4 ++++
 10 files changed, 103 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index acb9c34fb22..8b66cd8a9ad 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -7,6 +7,12 @@
   can be passed using the '[ADDRESS]:PORT' notation, or the regular
   'ADDRESS:PORT' method.
 
+* New commands
+
+maint set dwarf unwinders (on|off)
+maint show dwarf unwinders
+  Control whether DWARF unwinders can be used.
+
 *** Changes in GDB 8.2
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2e9d76227c3..eb148cedef0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35802,6 +35802,30 @@
 memory will be used.  Setting it to zero disables caching, which will
 slow down @value{GDBN} startup, but reduce memory consumption.
 
+@kindex maint set dwarf unwinders
+@kindex maint show dwarf unwinders
+@item maint set dwarf unwinders
+@itemx maint show dwarf unwinders
+Control use of the DWARF frame unwinders.
+
+@cindex DWARF frame unwinders
+Many targets that support DWARF debugging use @value{GDBN}'s DWARF
+frame unwinders to build the backtrace.  Many of these targets will
+also have a second mechanism for building the backtrace for use in
+cases where DWARF information is not available, this second mechanism
+is often an analysis of a functions prologue.
+
+In order to extend testing coverage of the second level stack
+unwinding mechanisms it is helpful to be able to disable the DWARF
+stack unwinders, this can be done with this switch.
+
+In normal use of @value{GDBN} disabling the DWARF unwinders is not
+advisable, there are cases that are better handled through DWARF than
+prologue analysis, and the debug experience is likely to be better
+with the DWARF frame unwinders enabled.
+
+If DWARF frame unwinders are not supported for a particular target
+architecture, then enabling this flag does not cause them to be used.
 @kindex maint set profile
 @kindex maint show profile
 @cindex profiling GDB
diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index 1d3e1f445bb..f565a2eecc8 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -318,6 +318,9 @@ tailcall_frame_sniffer (const struct frame_unwind *self,
   int next_levels;
   struct tailcall_cache *cache;
 
+  if (!dwarf2_frame_unwinders_enabled_p)
+    return 0;
+
   /* Inner tail call element does not make sense for a sentinel frame.  */
   next_frame = get_next_frame (this_frame);
   if (next_frame == NULL)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 91e16cf024e..fdd41b360e6 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -169,6 +169,9 @@ static CORE_ADDR read_encoded_value (struct comp_unit *unit, gdb_byte encoding,
 				     CORE_ADDR func_base);
 \f
 
+/* See dwarf2-frame.h.  */
+int dwarf2_frame_unwinders_enabled_p = 0;
+
 /* Store the length the expression for the CFA in the `cfa_reg' field,
    which is unused in that case.  */
 #define cfa_exp_len cfa_reg
@@ -1326,6 +1329,9 @@ static int
 dwarf2_frame_sniffer (const struct frame_unwind *self,
 		      struct frame_info *this_frame, void **this_cache)
 {
+  if (!dwarf2_frame_unwinders_enabled_p)
+    return 0;
+
   /* Grab an address that is guarenteed to reside somewhere within the
      function.  get_frame_pc(), with a no-return next function, can
      end up returning something past the end of this function's body.
@@ -1389,6 +1395,9 @@ dwarf2_append_unwinders (struct gdbarch *gdbarch)
 
   frame_unwind_append_unwinder (gdbarch, &dwarf2_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &dwarf2_signal_frame_unwind);
+
+  /* Mark dwarf frame unwinders as enabled.  */
+  dwarf2_frame_unwinders_enabled_p = 1;
 }
 \f
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 471281a2c3f..56c90be8ceb 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -210,6 +210,12 @@ struct dwarf2_frame_state
   bool armcc_cfa_offsets_reversed = false;
 };
 
+/* When this is true the dwarf frame unwinders can be used if they are
+   registered with the gdbarch.  Not all architectures can or do use the
+   dwarf unwinders.  Setting this to true on a target that does not
+   otherwise support the dwarf unwinders has no effect.  */
+extern int dwarf2_frame_unwinders_enabled_p;
+
 /* Set the architecture-specific register state initialization
    function for GDBARCH to INIT_REG.  */
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 372f45ee175..b49d4d9fbbc 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -90,6 +90,7 @@
 #include <forward_list>
 #include "rust-lang.h"
 #include "common/pathstuff.h"
+#include "dwarf2-frame.h"
 
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
@@ -1423,6 +1424,19 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
 			    "DWARF compilation units is %s.\n"),
 		    value);
 }
+
+/* Handle 'maintenance show dwarf unwinders'.  */
+
+static void
+show_dwarf_unwinders_enabled_p (struct ui_file *file, int from_tty,
+				struct cmd_list_element *c,
+				const char *value)
+{
+  fprintf_filtered (file,
+		    _("The DWARF stack unwinders are currently %s.\n"),
+		    value);
+}
+
 \f
 /* local function prototypes */
 
@@ -25363,6 +25377,18 @@ conversational style, when possible."),
 			   &set_dwarf_cmdlist,
 			   &show_dwarf_cmdlist);
 
+  add_setshow_boolean_cmd ("unwinders", class_obscure,
+			   &dwarf2_frame_unwinders_enabled_p , _("\
+Set whether the DWARF stack frame unwinders are used."), _("\
+Show whether the DWARF stack frame unwinders are used."), _("\
+When enabled the DWARF stack frame unwinders can be used for architectures\n\
+that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\
+architecture that doesn't support them will have no effect."),
+			   NULL,
+			   show_dwarf_unwinders_enabled_p,
+			   &set_dwarf_cmdlist,
+			   &show_dwarf_cmdlist);
+
   add_setshow_zuinteger_cmd ("dwarf-read", no_class, &dwarf_read_debug, _("\
 Set debugging of the DWARF reader."), _("\
 Show debugging of the DWARF reader."), _("\
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 92086eec862..6125b6ec6f2 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -526,6 +526,10 @@ gdb_test_no_output "maint info line-table xxx.c" \
 
 set timeout $oldtimeout
 
+# Just check that the DWARF unwinders control flag is visible.
+gdb_test "maint show dwarf unwinders" \
+    "The DWARF stack unwinders are currently (on\|off)\."
+
 #============test help on maint commands
 
 test_prefix_command_help {"maint info" "maintenance info"} {
-- 
2.14.4

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Fri, 13 Jul 2018 14:46:29 +0100
> 
> Add a maintenance command to disable the DWARF stack unwinders.
> Normal users would not need this feature, but it is useful to allow
> extended testing of fallback stack unwinding strategies, for example,
> prologue scanners.

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -7,6 +7,12 @@
>    can be passed using the '[ADDRESS]:PORT' notation, or the regular
>    'ADDRESS:PORT' method.
>  
> +* New commands
> +
> +maint set dwarf unwinders (on|off)
> +maint show dwarf unwinders
> +  Control whether DWARF unwinders can be used.
> +
>  *** Changes in GDB 8.2

OK for this part.

> +@cindex DWARF frame unwinders
> +Many targets that support DWARF debugging use @value{GDBN}'s DWARF
> +frame unwinders to build the backtrace.  Many of these targets will
> +also have a second mechanism for building the backtrace for use in
> +cases where DWARF information is not available, this second mechanism
> +is often an analysis of a functions prologue.
                             ^^^^^^^^^^^^^^^^^^
I think you meant "function's prologue" there.

The documentation parts are OK with the above nit fixed.

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Add a maintenance command to disable the DWARF stack unwinders.
Andrew> Normal users would not need this feature, but it is useful to allow
Andrew> extended testing of fallback stack unwinding strategies, for example,
Andrew> prologue scanners.

Andrew> gdb/ChangeLog:

Maybe this should mention PR 8434, since it seems like part of that idea.

Tom

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2018-07-13 09:34:24 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> Add a maintenance command to disable the DWARF stack unwinders.
> Andrew> Normal users would not need this feature, but it is useful to allow
> Andrew> extended testing of fallback stack unwinding strategies, for example,
> Andrew> prologue scanners.
> 
> Andrew> gdb/ChangeLog:
> 
> Maybe this should mention PR 8434, since it seems like part of that idea.

   PR8434: Add maintenance command to allow arbitrary unwinders to be
           enabled or disabled.

That's interesting.  I actually started working on a patch to do just
that.  In the end I stopped working on it because I was concerned that
the change would be too big, and of too little use to justify
merging.  The smaller change I posted got me the result I needed in a
much less intrusive change.

That said, if there is a wider interest in having this ability I'm
happy to dig out my old branch and get it ready for submission, but
I'd only want to put that effort in if this was a feature people would
like to see in GDB...

Thanks,
Andrew

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> That said, if there is a wider interest in having this ability I'm
Andrew> happy to dig out my old branch and get it ready for submission, but
Andrew> I'd only want to put that effort in if this was a feature people would
Andrew> like to see in GDB...

Yeah, I am not sure, your patch just reminded me of this.

Maybe if the DWARF patch goes in, the bug could be closed.
Given its age I would suppose this hasn't been needed very often.

Relatedly, somewhere I have a patch that adds a name to every unwinder,
so that you can tell what made the frame you're looking at.  This can be
handy when debugging unwinders.

Tom

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Simon Marchi]

Hi Andrew,

On 2018-07-13 09:46 AM, Andrew Burgess wrote:
> diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
> index 1d3e1f445bb..f565a2eecc8 100644
> --- a/gdb/dwarf2-frame-tailcall.c
> +++ b/gdb/dwarf2-frame-tailcall.c
> @@ -318,6 +318,9 @@ tailcall_frame_sniffer (const struct frame_unwind *self,
>    int next_levels;
>    struct tailcall_cache *cache;
>  
> +  if (!dwarf2_frame_unwinders_enabled_p)
> +    return 0;
> +
>    /* Inner tail call element does not make sense for a sentinel frame.  */
>    next_frame = get_next_frame (this_frame);
>    if (next_frame == NULL)
> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index 91e16cf024e..fdd41b360e6 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -169,6 +169,9 @@ static CORE_ADDR read_encoded_value (struct comp_unit *unit, gdb_byte encoding,
>  				     CORE_ADDR func_base);
>  \f
>  
> +/* See dwarf2-frame.h.  */
> +int dwarf2_frame_unwinders_enabled_p = 0;

Is there any reason why you initialize this to 0, and set it to 1 in
dwarf2_append_unwinders?  It can produce some quite unexpected results, IMO:

(gdb) maintenance set dwarf unwinders off
(gdb) maintenance show dwarf unwinders
The DWARF stack unwinders are currently off.
(gdb) file test
Reading symbols from test...done.
(gdb) maintenance show dwarf unwinders
The DWARF stack unwinders are currently on.

Is there something wrong with initializing it to 1 directly and not touching
it after?

> +
>  /* Store the length the expression for the CFA in the `cfa_reg' field,
>     which is unused in that case.  */
>  #define cfa_exp_len cfa_reg
> @@ -1326,6 +1329,9 @@ static int
>  dwarf2_frame_sniffer (const struct frame_unwind *self,
>  		      struct frame_info *this_frame, void **this_cache)
>  {
> +  if (!dwarf2_frame_unwinders_enabled_p)
> +    return 0;
> +
>    /* Grab an address that is guarenteed to reside somewhere within the
>       function.  get_frame_pc(), with a no-return next function, can
>       end up returning something past the end of this function's body.
> @@ -1389,6 +1395,9 @@ dwarf2_append_unwinders (struct gdbarch *gdbarch)
>  
>    frame_unwind_append_unwinder (gdbarch, &dwarf2_frame_unwind);
>    frame_unwind_append_unwinder (gdbarch, &dwarf2_signal_frame_unwind);
> +
> +  /* Mark dwarf frame unwinders as enabled.  */
> +  dwarf2_frame_unwinders_enabled_p = 1;
>  }
>  \f
>  
> diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
> index 471281a2c3f..56c90be8ceb 100644
> --- a/gdb/dwarf2-frame.h
> +++ b/gdb/dwarf2-frame.h
> @@ -210,6 +210,12 @@ struct dwarf2_frame_state
>    bool armcc_cfa_offsets_reversed = false;
>  };
>  
> +/* When this is true the dwarf frame unwinders can be used if they are
> +   registered with the gdbarch.  Not all architectures can or do use the
> +   dwarf unwinders.  Setting this to true on a target that does not
> +   otherwise support the dwarf unwinders has no effect.  */

dwarf -> DWARF

> +extern int dwarf2_frame_unwinders_enabled_p;
> +
>  /* Set the architecture-specific register state initialization
>     function for GDBARCH to INIT_REG.  */
>  
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 372f45ee175..b49d4d9fbbc 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -90,6 +90,7 @@
>  #include <forward_list>
>  #include "rust-lang.h"
>  #include "common/pathstuff.h"
> +#include "dwarf2-frame.h"
>  
>  /* When == 1, print basic high level tracing messages.
>     When > 1, be more verbose.
> @@ -1423,6 +1424,19 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
>  			    "DWARF compilation units is %s.\n"),
>  		    value);
>  }
> +
> +/* Handle 'maintenance show dwarf unwinders'.  */
> +
> +static void
> +show_dwarf_unwinders_enabled_p (struct ui_file *file, int from_tty,
> +				struct cmd_list_element *c,
> +				const char *value)
> +{
> +  fprintf_filtered (file,
> +		    _("The DWARF stack unwinders are currently %s.\n"),
> +		    value);
> +}

I don't think this is necessary, if you leave the "show" callback to NULL,
I think the message is clear enough:

(gdb) maintenance show dwarf unwinders
Whether the DWARF stack frame unwinders are used is on.

> +
>  \f
>  /* local function prototypes */
>  
> @@ -25363,6 +25377,18 @@ conversational style, when possible."),
>  			   &set_dwarf_cmdlist,
>  			   &show_dwarf_cmdlist);
>  
> +  add_setshow_boolean_cmd ("unwinders", class_obscure,
> +			   &dwarf2_frame_unwinders_enabled_p , _("\
> +Set whether the DWARF stack frame unwinders are used."), _("\
> +Show whether the DWARF stack frame unwinders are used."), _("\
> +When enabled the DWARF stack frame unwinders can be used for architectures\n\
> +that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\
> +architecture that doesn't support them will have no effect."),
> +			   NULL,
> +			   show_dwarf_unwinders_enabled_p,
> +			   &set_dwarf_cmdlist,
> +			   &show_dwarf_cmdlist);
> +

I think it would make sense to have the command registration in the same file as
dwarf2_frame_unwinders_enabled_p (dwarf2-frame.c).  set_dwarf_cmdlist/show_dwarf_cmdlist
would need to be exported though.  I guess their declarations can go in dwarf2read.h,
since they are defined in dwarf2read.c.

Simon

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Andrew Burgess]

Simon,

Thanks for your review.  It all looks great, except for one follow up
question I have which is inline below...

* Simon Marchi <simark@simark.ca> [2018-07-13 21:11:38 -0400]:

> Hi Andrew,
> 
> On 2018-07-13 09:46 AM, Andrew Burgess wrote:
> > diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
> > index 1d3e1f445bb..f565a2eecc8 100644
> > --- a/gdb/dwarf2-frame-tailcall.c
> > +++ b/gdb/dwarf2-frame-tailcall.c
> > @@ -318,6 +318,9 @@ tailcall_frame_sniffer (const struct frame_unwind *self,
> >    int next_levels;
> >    struct tailcall_cache *cache;
> >  
> > +  if (!dwarf2_frame_unwinders_enabled_p)
> > +    return 0;
> > +
> >    /* Inner tail call element does not make sense for a sentinel frame.  */
> >    next_frame = get_next_frame (this_frame);
> >    if (next_frame == NULL)
> > diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> > index 91e16cf024e..fdd41b360e6 100644
> > --- a/gdb/dwarf2-frame.c
> > +++ b/gdb/dwarf2-frame.c
> > @@ -169,6 +169,9 @@ static CORE_ADDR read_encoded_value (struct comp_unit *unit, gdb_byte encoding,
> >  				     CORE_ADDR func_base);
> >  \f
> >  
> > +/* See dwarf2-frame.h.  */
> > +int dwarf2_frame_unwinders_enabled_p = 0;
> 
> Is there any reason why you initialize this to 0, and set it to 1 in
> dwarf2_append_unwinders?  It can produce some quite unexpected results, IMO:
> 
> (gdb) maintenance set dwarf unwinders off
> (gdb) maintenance show dwarf unwinders
> The DWARF stack unwinders are currently off.
> (gdb) file test
> Reading symbols from test...done.
> (gdb) maintenance show dwarf unwinders
> The DWARF stack unwinders are currently on.
> 
> Is there something wrong with initializing it to 1 directly and not touching
> it after?
> 
> > +
> >  /* Store the length the expression for the CFA in the `cfa_reg' field,
> >     which is unused in that case.  */
> >  #define cfa_exp_len cfa_reg
> > @@ -1326,6 +1329,9 @@ static int
> >  dwarf2_frame_sniffer (const struct frame_unwind *self,
> >  		      struct frame_info *this_frame, void **this_cache)
> >  {
> > +  if (!dwarf2_frame_unwinders_enabled_p)
> > +    return 0;
> > +
> >    /* Grab an address that is guarenteed to reside somewhere within the
> >       function.  get_frame_pc(), with a no-return next function, can
> >       end up returning something past the end of this function's body.
> > @@ -1389,6 +1395,9 @@ dwarf2_append_unwinders (struct gdbarch *gdbarch)
> >  
> >    frame_unwind_append_unwinder (gdbarch, &dwarf2_frame_unwind);
> >    frame_unwind_append_unwinder (gdbarch, &dwarf2_signal_frame_unwind);
> > +
> > +  /* Mark dwarf frame unwinders as enabled.  */
> > +  dwarf2_frame_unwinders_enabled_p = 1;
> >  }
> >  \f
> >  
> > diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
> > index 471281a2c3f..56c90be8ceb 100644
> > --- a/gdb/dwarf2-frame.h
> > +++ b/gdb/dwarf2-frame.h
> > @@ -210,6 +210,12 @@ struct dwarf2_frame_state
> >    bool armcc_cfa_offsets_reversed = false;
> >  };
> >  
> > +/* When this is true the dwarf frame unwinders can be used if they are
> > +   registered with the gdbarch.  Not all architectures can or do use the
> > +   dwarf unwinders.  Setting this to true on a target that does not
> > +   otherwise support the dwarf unwinders has no effect.  */
> 
> dwarf -> DWARF
> 
> > +extern int dwarf2_frame_unwinders_enabled_p;
> > +
> >  /* Set the architecture-specific register state initialization
> >     function for GDBARCH to INIT_REG.  */
> >  
> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> > index 372f45ee175..b49d4d9fbbc 100644
> > --- a/gdb/dwarf2read.c
> > +++ b/gdb/dwarf2read.c
> > @@ -90,6 +90,7 @@
> >  #include <forward_list>
> >  #include "rust-lang.h"
> >  #include "common/pathstuff.h"
> > +#include "dwarf2-frame.h"
> >  
> >  /* When == 1, print basic high level tracing messages.
> >     When > 1, be more verbose.
> > @@ -1423,6 +1424,19 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
> >  			    "DWARF compilation units is %s.\n"),
> >  		    value);
> >  }
> > +
> > +/* Handle 'maintenance show dwarf unwinders'.  */
> > +
> > +static void
> > +show_dwarf_unwinders_enabled_p (struct ui_file *file, int from_tty,
> > +				struct cmd_list_element *c,
> > +				const char *value)
> > +{
> > +  fprintf_filtered (file,
> > +		    _("The DWARF stack unwinders are currently %s.\n"),
> > +		    value);
> > +}
> 
> I don't think this is necessary, if you leave the "show" callback to NULL,
> I think the message is clear enough:
> 
> (gdb) maintenance show dwarf unwinders
> Whether the DWARF stack frame unwinders are used is on.

I thought that was no longer the approved approach as the resulting
string is generated and not internationalised?

> 
> > +
> >  \f
> >  /* local function prototypes */
> >  
> > @@ -25363,6 +25377,18 @@ conversational style, when possible."),
> >  			   &set_dwarf_cmdlist,
> >  			   &show_dwarf_cmdlist);
> >  
> > +  add_setshow_boolean_cmd ("unwinders", class_obscure,
> > +			   &dwarf2_frame_unwinders_enabled_p , _("\
> > +Set whether the DWARF stack frame unwinders are used."), _("\
> > +Show whether the DWARF stack frame unwinders are used."), _("\
> > +When enabled the DWARF stack frame unwinders can be used for architectures\n\
> > +that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\
> > +architecture that doesn't support them will have no effect."),
> > +			   NULL,
> > +			   show_dwarf_unwinders_enabled_p,
> > +			   &set_dwarf_cmdlist,
> > +			   &show_dwarf_cmdlist);
> > +
> 
> I think it would make sense to have the command registration in the same file as
> dwarf2_frame_unwinders_enabled_p (dwarf2-frame.c).  set_dwarf_cmdlist/show_dwarf_cmdlist
> would need to be exported though.  I guess their declarations can go in dwarf2read.h,
> since they are defined in dwarf2read.c.
> 
> Simon

Thanks,
Andrew

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2018-07-13 16:57:35 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> That said, if there is a wider interest in having this ability I'm
> Andrew> happy to dig out my old branch and get it ready for submission, but
> Andrew> I'd only want to put that effort in if this was a feature people would
> Andrew> like to see in GDB...
> 
> Yeah, I am not sure, your patch just reminded me of this.
> 
> Maybe if the DWARF patch goes in, the bug could be closed.
> Given its age I would suppose this hasn't been needed very often.
> 
> Relatedly, somewhere I have a patch that adds a name to every unwinder,
> so that you can tell what made the frame you're looking at.  This can be
> handy when debugging unwinders.

Great minds... :)

I have a similar patch as part of my "make all unwinders switchable"
series.  I agree it's a nice feature to be able to see who unwound
each frame.

Maybe I should clean it up and submit it...

Thanks,
Andrew

> 
> Tom

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Simon Marchi]

On 2018-07-14 04:35 PM, Andrew Burgess wrote:
>> I don't think this is necessary, if you leave the "show" callback to NULL,
>> I think the message is clear enough:
>>
>> (gdb) maintenance show dwarf unwinders
>> Whether the DWARF stack frame unwinders are used is on.
> 
> I thought that was no longer the approved approach as the resulting
> string is generated and not internationalised?

Ah, I was not aware of that.  If you got that from a trustworthy source, then
I won't object :).

Simon

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Great minds... :)

Andrew> I have a similar patch as part of my "make all unwinders switchable"
Andrew> series.  I agree it's a nice feature to be able to see who unwound
Andrew> each frame.

Andrew> Maybe I should clean it up and submit it...

I think it would be good, now that ordinary users can write unwinders
via Python.

Tom

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Pedro Alves]

On 07/14/2018 10:39 PM, Simon Marchi wrote:
> On 2018-07-14 04:35 PM, Andrew Burgess wrote:
>>> I don't think this is necessary, if you leave the "show" callback to NULL,
>>> I think the message is clear enough:
>>>
>>> (gdb) maintenance show dwarf unwinders
>>> Whether the DWARF stack frame unwinders are used is on.
>>
>> I thought that was no longer the approved approach as the resulting
>> string is generated and not internationalised?
> 
> Ah, I was not aware of that.  If you got that from a trustworthy source, then
> I won't object :).
Here:

 void
 deprecated_show_value_hack (struct ui_file *ignore_file,
 			    int ignore_from_tty,
 			    struct cmd_list_element *c,
 			    const char *value)
 {
 ...
   /* Print doc minus "show" at start.  */
   print_doc_line (gdb_stdout, c->doc + 5);
 
That "c->doc + 5" trick only works in English.

Thanks,
Pedro Alves

Re: [PATCH] gdb: Add switch to disable DWARF stack unwinders

[Pedro Alves]

On 07/13/2018 02:46 PM, Andrew Burgess wrote:
> Add a maintenance command to disable the DWARF stack unwinders.
> Normal users would not need this feature, but it is useful to allow
> extended testing of fallback stack unwinding strategies, for example,
> prologue scanners.

I think a command to enable/disable unwinders is good, especially if
it supports all kinds of unwinders by name, but I thought I'd
also mention that the last time prologue testing came up, we ended up
with targeted unit tests.  See aarch64_analyze_prologue_test,
git commit 4d9a9006139d,
<https://sourceware.org/ml/gdb-patches/2016-12/msg00024.html>.

Thanks,
Pedro Alves

[PATCHv2] gdb: Add switch to disable DWARF stack unwinders

[Andrew Burgess]

This revision addresses the review feedback:

  - Minor doc change suggested by Eli.
  - DWARF unwinders start globally enabled.
  - Minor comment typo fixes
  - New maintenance command is now registered in dwarf2-frame.c

The commit message now mentions pr 8434, which is about being able to
enable or disable any/all unwinders.  After the discussions on the
mailing list, and on IRC I'm still proposing that this patch only
allow control of DWARF unwinders.  The thinking is that, this is only
a maintenance command, so we can change the UI later if we wish, and
allowing full unwinder control would be a much bigger, more invasive
change, and it's unclear how useful that feature would actually be.
Being able to switch of DWARF unwinders is useful, so that's all I'm
proposing right now.

--

Add a maintenance command to disable the DWARF stack unwinders.
Normal users would not need this feature, but it is useful to allow
extended testing of fallback stack unwinding strategies, for example,
prologue scanners.

This is a partial implementation of the idea discussed in pr gdb/8434,
which talks about a generic ability to disable any frame unwinder.

Being able to arbitrarily disable any frame unwinder would be a more
complex patch, and I was unsure how useful such a feature would really
be, however, I can see (and have) a real need to disable DWARF
unwinders.  That's why this patch only targets that specific set of
unwinders.

If in the future we find ourselves adding more switches to disable
different unwinders, then we should probably move to a more generic
solution, and remove this patch.

gdb/ChangeLog:

	* dwarf2-frame-tailcall.c (tailcall_frame_sniffer): Exit early if
	DWARF unwinders are disabled.
	* dwarf2-frame.c: Add dwarf2read.h include.
	(dwarf2_frame_sniffer): Exit early if DWARF unwinders are
	disabled.
	(dwarf2_frame_unwinders_enabled_p): Define.
	(show_dwarf_unwinders_enabled_p): New function.
	(_initialize_dwarf2_frame): Register switch to control DWARF
	unwinder use.
	* dwarf2-frame.h (dwarf2_frame_unwinders_enabled_p): Declare.
	* dwarf2read.c (set_dwarf_cmdlist): Remove static keyword.
	(show_dwarf_cmdlist): Remove static keyword.
	* dwarf2read.h (set_dwarf_cmdlist): Declare.
	(show_dwarf_cmdlist): Declare.
	* NEWS: Document new feature.

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Add description of
	maintenance command to control dwarf unwinders.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Add check that dwarf unwinders control flag
	is visible.
---
 gdb/ChangeLog                    | 18 ++++++++++++++++++
 gdb/NEWS                         |  4 ++++
 gdb/doc/ChangeLog                |  5 +++++
 gdb/doc/gdb.texinfo              | 24 ++++++++++++++++++++++++
 gdb/dwarf2-frame-tailcall.c      |  3 +++
 gdb/dwarf2-frame.c               | 31 +++++++++++++++++++++++++++++++
 gdb/dwarf2-frame.h               |  6 ++++++
 gdb/dwarf2read.c                 |  4 ++--
 gdb/dwarf2read.h                 |  4 ++++
 gdb/testsuite/ChangeLog          |  5 +++++
 gdb/testsuite/gdb.base/maint.exp |  4 ++++
 11 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 76b963e2bc1..669ed2d0ebd 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,10 @@ tfaas COMMAND
   output).
   Shortcut for 'thread apply all -s frame apply all -s COMMAND'.
 
+maint set dwarf unwinders (on|off)
+maint show dwarf unwinders
+  Control whether DWARF unwinders can be used.
+
 * Changed commands
 
 thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b36a39b5b4c..b95c2b4f49a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35986,6 +35986,30 @@
 memory will be used.  Setting it to zero disables caching, which will
 slow down @value{GDBN} startup, but reduce memory consumption.
 
+@kindex maint set dwarf unwinders
+@kindex maint show dwarf unwinders
+@item maint set dwarf unwinders
+@itemx maint show dwarf unwinders
+Control use of the DWARF frame unwinders.
+
+@cindex DWARF frame unwinders
+Many targets that support DWARF debugging use @value{GDBN}'s DWARF
+frame unwinders to build the backtrace.  Many of these targets will
+also have a second mechanism for building the backtrace for use in
+cases where DWARF information is not available, this second mechanism
+is often an analysis of a function's prologue.
+
+In order to extend testing coverage of the second level stack
+unwinding mechanisms it is helpful to be able to disable the DWARF
+stack unwinders, this can be done with this switch.
+
+In normal use of @value{GDBN} disabling the DWARF unwinders is not
+advisable, there are cases that are better handled through DWARF than
+prologue analysis, and the debug experience is likely to be better
+with the DWARF frame unwinders enabled.
+
+If DWARF frame unwinders are not supported for a particular target
+architecture, then enabling this flag does not cause them to be used.
 @kindex maint set profile
 @kindex maint show profile
 @cindex profiling GDB
diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c
index 1d3e1f445bb..f565a2eecc8 100644
--- a/gdb/dwarf2-frame-tailcall.c
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -318,6 +318,9 @@ tailcall_frame_sniffer (const struct frame_unwind *self,
   int next_levels;
   struct tailcall_cache *cache;
 
+  if (!dwarf2_frame_unwinders_enabled_p)
+    return 0;
+
   /* Inner tail call element does not make sense for a sentinel frame.  */
   next_frame = get_next_frame (this_frame);
   if (next_frame == NULL)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 91e16cf024e..2e1110bbe8b 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -35,6 +35,7 @@
 
 #include "complaints.h"
 #include "dwarf2-frame.h"
+#include "dwarf2read.h"
 #include "ax.h"
 #include "dwarf2loc.h"
 #include "dwarf2-frame-tailcall.h"
@@ -169,6 +170,9 @@ static CORE_ADDR read_encoded_value (struct comp_unit *unit, gdb_byte encoding,
 				     CORE_ADDR func_base);
 \f
 
+/* See dwarf2-frame.h.  */
+int dwarf2_frame_unwinders_enabled_p = 1;
+
 /* Store the length the expression for the CFA in the `cfa_reg' field,
    which is unused in that case.  */
 #define cfa_exp_len cfa_reg
@@ -1326,6 +1330,9 @@ static int
 dwarf2_frame_sniffer (const struct frame_unwind *self,
 		      struct frame_info *this_frame, void **this_cache)
 {
+  if (!dwarf2_frame_unwinders_enabled_p)
+    return 0;
+
   /* Grab an address that is guarenteed to reside somewhere within the
      function.  get_frame_pc(), with a no-return next function, can
      end up returning something past the end of this function's body.
@@ -2394,12 +2401,36 @@ dwarf2_build_frame_info (struct objfile *objfile)
   set_objfile_data (objfile, dwarf2_frame_objfile_data, fde_table2);
 }
 
+/* Handle 'maintenance show dwarf unwinders'.  */
+
+static void
+show_dwarf_unwinders_enabled_p (struct ui_file *file, int from_tty,
+				struct cmd_list_element *c,
+				const char *value)
+{
+  fprintf_filtered (file,
+		    _("The DWARF stack unwinders are currently %s.\n"),
+		    value);
+}
+
 void
 _initialize_dwarf2_frame (void)
 {
   dwarf2_frame_data = gdbarch_data_register_pre_init (dwarf2_frame_init);
   dwarf2_frame_objfile_data = register_objfile_data ();
 
+  add_setshow_boolean_cmd ("unwinders", class_obscure,
+			   &dwarf2_frame_unwinders_enabled_p , _("\
+Set whether the DWARF stack frame unwinders are used."), _("\
+Show whether the DWARF stack frame unwinders are used."), _("\
+When enabled the DWARF stack frame unwinders can be used for architectures\n\
+that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\
+architecture that doesn't support them will have no effect."),
+			   NULL,
+			   show_dwarf_unwinders_enabled_p,
+			   &set_dwarf_cmdlist,
+			   &show_dwarf_cmdlist);
+
 #if GDB_SELF_TEST
   selftests::register_test_foreach_arch ("execute_cfa_program",
 					 selftests::execute_cfa_program_test);
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 471281a2c3f..52316e5e168 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -210,6 +210,12 @@ struct dwarf2_frame_state
   bool armcc_cfa_offsets_reversed = false;
 };
 
+/* When this is true the DWARF frame unwinders can be used if they are
+   registered with the gdbarch.  Not all architectures can or do use the
+   DWARF unwinders.  Setting this to true on a target that does not
+   otherwise support the DWARF unwinders has no effect.  */
+extern int dwarf2_frame_unwinders_enabled_p;
+
 /* Set the architecture-specific register state initialization
    function for GDBARCH to INIT_REG.  */
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 45280fec410..8455d948412 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -25330,8 +25330,8 @@ partial_die_eq (const void *item_lhs, const void *item_rhs)
   return part_die_lhs->sect_off == part_die_rhs->sect_off;
 }
 
-static struct cmd_list_element *set_dwarf_cmdlist;
-static struct cmd_list_element *show_dwarf_cmdlist;
+struct cmd_list_element *set_dwarf_cmdlist;
+struct cmd_list_element *show_dwarf_cmdlist;
 
 static void
 set_dwarf_cmd (const char *args, int from_tty)
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 74335d77db0..6e37c5d06f5 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -23,6 +23,10 @@
 #include "filename-seen-cache.h"
 #include "gdb_obstack.h"
 
+/* Hold 'maintenance (set|show) dwarf' commands.  */
+extern struct cmd_list_element *set_dwarf_cmdlist;
+extern struct cmd_list_element *show_dwarf_cmdlist;
+
 typedef struct dwarf2_per_cu_data *dwarf2_per_cu_ptr;
 DEF_VEC_P (dwarf2_per_cu_ptr);
 
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 92086eec862..6125b6ec6f2 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -526,6 +526,10 @@ gdb_test_no_output "maint info line-table xxx.c" \
 
 set timeout $oldtimeout
 
+# Just check that the DWARF unwinders control flag is visible.
+gdb_test "maint show dwarf unwinders" \
+    "The DWARF stack unwinders are currently (on\|off)\."
+
 #============test help on maint commands
 
 test_prefix_command_help {"maint info" "maintenance info"} {
-- 
2.14.4

Re: [PATCHv2] gdb: Add switch to disable DWARF stack unwinders

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>,	Simon Marchi <simark@simark.ca>,	Tom Tromey <tom@tromey.com>
> Date: Wed, 25 Jul 2018 16:18:42 +0100
> 
> +  add_setshow_boolean_cmd ("unwinders", class_obscure,
> +			   &dwarf2_frame_unwinders_enabled_p , _("\
> +Set whether the DWARF stack frame unwinders are used."), _("\
> +Show whether the DWARF stack frame unwinders are used."), _("\
> +When enabled the DWARF stack frame unwinders can be used for architectures\n\
> +that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\
> +architecture that doesn't support them will have no effect."),

The last instance of "dwarf" should be up-cased.

Otherwise the documentation parts are OK.  Thanks.

Re: [PATCHv2] gdb: Add switch to disable DWARF stack unwinders

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The commit message now mentions pr 8434, which is about being able to
Andrew> enable or disable any/all unwinders.  After the discussions on the
Andrew> mailing list, and on IRC I'm still proposing that this patch only
Andrew> allow control of DWARF unwinders.

Thanks for doing this.
I have a couple little nits, but the patch is ok with these fixed.

Andrew> +that support the DWARF unwinders.  Enabling the dwarf unwinders for an\n\

s/dwarf/DWARF/ here.

Andrew> +# Just check that the DWARF unwinders control flag is visible.
Andrew> +gdb_test "maint show dwarf unwinders" \
Andrew> +    "The DWARF stack unwinders are currently (on\|off)\."

The backslashes aren't really doing their work here, I think it should read:

    "The DWARF stack unwinders are currently (on|off)\\."

thanks,
Tom