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

Simon Marchi
2024-10-03T18:47:16Z

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
> +
> +/* 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