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

[Simon Marchi <simark@simark.ca>]

On 10/1/24 2:42 PM, Guinevere Larsen wrote:
> From: Guinevere Larsen <blarsen@redhat.com>
> 
> 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.

indentifying -> identifying

> diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
> index b69ae8596a2..afc1258c6a9 100644
> --- a/gdb/frame-unwind.c
> +++ b/gdb/frame-unwind.c
> @@ -30,6 +30,17 @@
>  #include "dwarf2/frame-tailcall.h"
>  #include "cli/cli-cmds.h"
>  #include "inferior.h"
> +#include <map>
> +
> +/* Conversion list between the enum for frame_unwind_class and
> +   string.  */
> +static std::map<enum frame_unwind_class, const char *> unwind_class_conversion =
> +{
> +  {FRAME_UNWIND_GDB, "GDB"},
> +  {FRAME_UNWIND_ARCH, "ARCH"},
> +  {FRAME_UNWIND_EXTENSION, "EXTENSION"},
> +  {FRAME_UNWIND_DEBUGINFO, "DEBUGINFO"},
> +};

It's not reaaally a big deal, but using an std::map for this is kind of
heavyweight.  It could probably be a simple array, still indexed by the
enum value.  Personally, I used functions with switches to handle cases
this (see target_waitkind_str), because compilers can warn you if you
forget to handle an enumerator (which can easily happen when a new one
is added).

Otherwise, a more medium-term but clean solution would be to integrate
one of these libs in GDB:

https://github.com/Neargye/magic_enum
https://github.com/quicknir/wise_enum

I use the latter in another project, and it works nicely.

Otherwise, LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon