Re: [PATCH 4/4] GDB: introduce ability to disable frame unwinders

[Eli Zaretskii <eliz@gnu.org>]

> From: Guinevere Larsen <blarsen@redhat.com>
> Cc: Guinevere Larsen <blarsen@redhat.com>
> Date: Wed,  6 Mar 2024 13:51:35 +0100
> 
>  gdb/NEWS                                      |   7 +
>  gdb/doc/gdb.texinfo                           |  24 +++
>  gdb/frame-unwind.c                            | 161 +++++++++++++++++-
>  gdb/frame-unwind.h                            |  13 ++
>  gdb/testsuite/gdb.base/frame-unwind-disable.c |  21 +++
>  .../gdb.base/frame-unwind-disable.exp         | 114 +++++++++++++
>  6 files changed, 336 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/frame-unwind-disable.c
>  create mode 100644 gdb/testsuite/gdb.base/frame-unwind-disable.exp

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -43,6 +43,13 @@ disable missing-debug-handler LOCUS HANDLER
>  maintenance info linux-lwps
>    List all LWPs under control of the linux-nat target.
>  
> +maintenance frame-unwinder disable [-name]
> +maintenance frame-unwinder enable [-name]
> +  Enable or disable frame unwinders.  This is only meant to be used when
> +  testing unwinders themselves, and you want to ensure that a fallback
> +  algorithm won't obscure a regression. GDB is not expected to behave well
                                         ^^
Two spaces between sentences, please.

> +@kindex maint frame-unwinder disable
> +@kindex maint frame-unwinder enable
> +@item maint frame-unwinder disable [OPTION] UNWINDER
> +@item maint frame-unwinder enable [OPTION] UNWINDER

OPTION and UNWINDER should be @var{option} and @var{unwinder},
respectively.

> +The meaning of the argument @samp{unwinder} depends on the @samp{option}

The correct markup here is @var, not @samp, since these are not
literal symbols, but instead names of parameters that stand for
something else.  Likewise elsewhere in the gdb.texinfo part of the
patch.

> +@table @samp
> +@item @code{-all}
> +ignore @samp{unwinder} and disable/enable all unwinders

This should be a complete sentence: begin with a capital letter and
end with a period.

> +@item @code{-class}
> +@samp{unwinder} is the class on frame unwinders to be disabled or enabled.
                                ^^
This should probably be "of"?

> +The class may include the prefix @code{FRAME_UNWINDER_}, but it is not
> +required.  This is the default option.

What do you mean by the last sentence?  What is "this" that is the
default option?  And what does "default" mean for an option, since an
option is by definition something that doesn't happen by default.

> +Disable one or more frame unwinder(s).\n\
> +Usage: maint frame-unwinder disable [OPTION] UNWINDER\n\
> +\n\
> +The meaning of UNWINDER depends on the OPTION given. These are the possibilities:\n\
                                                      ^^
> +\t-all    - UNWINDER is ignored. All available unwinders will be disabled\n\
                                  ^^

Two spaces between sentences in doc strings as well.

> +The meaning of UNWINDER depends on the OPTION given. These are the possibilities:\n\
> +\t-all    - UNWINDER is ignored. All available unwinders will be enabled\n\

Likewise here.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>