Re: [PATCH v3 2/5] gdb: add "unwinder class" to frame unwinders

[Guinevere Larsen <blarsen@redhat.com>]

On 5/8/24 15:20, Eli Zaretskii wrote:
>> From: Guinevere Larsen <blarsen@redhat.com>
>> Cc: Guinevere Larsen <blarsen@redhat.com>
>> Date: Wed,  8 May 2024 11:54:52 -0300
>>
>> A future patch will add a way to disable certain unwinders based on
>> different characteristics. This patch aims to make it more convenient
>> to disable related unwinders in bulk, such as architecture specific
>> ones, by indentifying all unwinders by which part of the code adds it.
>> The classes, and explanations, are as follows:
>>
>> * GDB: An internal unwinder, added by GDB core, such as the unwinder
>>    for dummy frames;
>> * EXTENSION: Unwinders added by extension languages;
>> * DEBUGINFO: Unwinders installed by the debug info reader;
>> * ARCH: Unwinders installed by the architecture specific code.
> Thanks.  I wonder whether some of the above should in the manual as
> well.
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 050f6f96e62..679295f0507 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -32,6 +32,10 @@ maintenance info line-table
>>     if the line is considered the start of the epilgoue, and thus a point at
>>     which the frame can be considered destroyed.
>>   
>> +mainenance info frame-unwinders
>> +  Add a CLASS column to the output.  This class is a somewhat arbitrary
>> +  grouping of unwinders, based on which area of GDB adds the unwinder.
> This part is okay.
>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 61f91ef4ad6..3fc0fafa3cd 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -42094,7 +42094,7 @@ architecture, then enabling this flag does not cause them to be used.
>>   
>>   @kindex maint info frame-unwinders
>>   @item maint info frame-unwinders
>> -List the frame unwinders currently in effect, starting with the highest priority.
>> +List the frame unwinders currently in effect, starting with the highest priority.  This also lists the unwinder class, which is mostly defined by which area  of @value{GDBN} uses it.
> This is also okay, but please split the long line into a few, so that
> each line is shorter than 79 characters.  And also please consider
> adding the list of currently known classes to the above description.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>
Thanks for the quick review! I added the following diff to fix the comments:

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a3226d98ff6..862dd152ab2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42094,7 +42094,16 @@ architecture, then enabling this flag does not 
cause them to be used.

  @kindex maint info frame-unwinders
  @item maint info frame-unwinders
-List the frame unwinders currently in effect, starting with the highest 
priority.  This also lists the unwinder class, which is mostly defined 
by which area  of @value{GDBN} uses it.
+List the frame unwinders currently in effect, starting with the highest
+priority.  This also lists the unwinder class, which is mostly defined by
+which area  of @value{GDBN} uses it.  The currently available classes are:
+
+@table @samp
+@item GDB: An internal unwinder, added by GDB core;
+@item EXTENSION: Unwinders added by extension languages;
+@item DEBUGINFO: Unwinders installed by debug information readers;
+@item ARCH: Unwinders isntalled by the architecture specific code.
+@end table

  @kindex maint frame-unwinder disable
  @kindex maint frame-unwinder enable

-- 
Cheers,
Guinevere Larsen
She/Her/Hers