Re: [PATCH] [gdb/tdep] Don't use i386 unwinder for amd64

[John Baldwin <jhb@FreeBSD.org>]

On 2/10/23 4:56 AM, Tom de Vries via Gdb-patches wrote:
> For i386 we have these unwinders:
> ...
> $ gdb -q -batch -ex "set arch i386" -ex "maint info frame-unwinders"
> The target architecture is set to "i386".
> dummy                   DUMMY_FRAME
> dwarf2 tailcall         TAILCALL_FRAME
> inline                  INLINE_FRAME
> i386 epilogue           NORMAL_FRAME
> dwarf2                  NORMAL_FRAME
> dwarf2 signal           SIGTRAMP_FRAME
> i386 stack tramp        NORMAL_FRAME
> i386 sigtramp           SIGTRAMP_FRAME
> i386 prologue           NORMAL_FRAME
> ...
> and for amd64:
> ...
> $ gdb -q -batch -ex "set arch i386:x86-64" -ex "maint info frame-unwinders"
> The target architecture is set to "i386:x86-64".
> dummy                   DUMMY_FRAME
> dwarf2 tailcall         TAILCALL_FRAME
> inline                  INLINE_FRAME
> python                  NORMAL_FRAME
> amd64 epilogue          NORMAL_FRAME
> i386 epilogue           NORMAL_FRAME
> dwarf2                  NORMAL_FRAME
> dwarf2 signal           SIGTRAMP_FRAME
> amd64 sigtramp          SIGTRAMP_FRAME
> amd64 prologue          NORMAL_FRAME
> i386 stack tramp        NORMAL_FRAME
> i386 sigtramp           SIGTRAMP_FRAME
> i386 prologue           NORMAL_FRAME
> ...
> 
> ISTM me there's no reason for the i386 unwinders to be there for amd64.
> 
> Furthermore, there's a generic need to play around with enabling and disabling
> unwinders, see PR8434.  Currently, that's only available for both the
> dwarf2 unwinders at once using "maint set dwarf unwinders on/off".
> 
> If I manually disable the "amd64 epilogue" unwinder, the "i386 epilogue"
> unwinder becomes active and gives the wrong answer, while I'm actually
> interested in the result of the dwarf2 unwinder.  Of course I can also
> manually disable the "i386 epilogue", but I take the fact that I have to do
> that as evidence that on amd64, the "i386 epilogue" is not only unnecessary,
> but in the way.
> 
> Fix this by only adding the i386 unwinders only if
> "info.bfd_arch_info->bits_per_word == 32".
> 
> Note that for the x32 abi (x86_64/-mx32) the unwinder list is the same as for
> amd64 (x86_64/-m64), without and with this commit.
> 
> Tested on x86_64-linux, -m64 and -m32.  Not tested with -mx32.

I strongly suspect you don't want these unwinders for x32 either.  I don't
know what bits_per_word is for x32.  If it is 32 then you at least aren't
changing anything for x32.  If it is 64 then I think your change is correct.
You might consider checking 'tdep->num_dword_regs == 0' instead if you want
to enable these for i386-only and exclude x32.  (IIRC, x32 is encoded in ELF
as EM_X86_64 but with ELFCLASS32 or the like which would mean x32 would have
bits_per_word of 32 I think).

LGTM as-is or if you decide to test num_dword_regs == 0 instead.

-- 
John Baldwin