[PATCHv2 3/6] gdb/arm: avoid undefined behaviour in arm_frame_is_thumb

[Luis Machado]

On 6/10/22 14:08, Andrew Burgess via Gdb-patches wrote:
> This commit fixes real undefined behaviour in GDB which I spotted when
> working on a later patch in this series.  The later patch in this
> series detects when the result of gdbarch_tdep() is cast to the wrong
> type.
> 
> The issue is revealed by the gdb.multi/multi-arch.exp test.
> 
> In this test we setup two inferiors, an AArch64 process, and an ARM
> process, then at one point we have inferior 1 selected (the AArch64
> inferior), and we place a breakpoint on a symbol present in the other
> inferior (the ARM inferior).
> 
> During the process of creating the breakpoint we call arm_pc_is_thumb,
> the GDBARCH passed into this function is correct, that is, represents
> the ARM process.
> 
> For whatever reason we are unable to figure out if the address in
> question is thumb or not throughout most of arm_pc_is_thumb, and so we
> get to this code at the end of the function:
> 
>    /* If we couldn't find any symbol, but we're talking to a running
>       target, then trust the current value of $cpsr.  This lets
>       "display/i $pc" always show the correct mode (though if there is
>       a symbol table we will not reach here, so it still may not be
>       displayed in the mode it will be executed).  */
>    if (target_has_registers ())
>      return arm_frame_is_thumb (get_current_frame ());
> 
> Which I guess is a last attempt to figure out the thumb status of an
> address.  However, remember, we the AArch64 inferior is current at
> this time, so the current frame is an AArch64 frame.

If we're trying to insert a breakpoint into a 32-bit inferior,
we should really have the 32-bit arm gdbarch at hand, not the AArch64 gdbarch.

I think the bug is somewhere else, in whoever passed the current inferior's gdbarch
as opposed to the gdbarch of the inferior that contains the symbol we've found.

I think this should be an assertion instead, as it is clearly wrong.