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

[Andrew Burgess <aburgess@redhat.com>]

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.

In arm_frame_is_thumb we call arm_psr_thumb_bit with the architecture
of the current frame (which will be an AArch64 architecture).

And in arm_psr_thumb_bit we do this:

  arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch);

which assumes that the current architecture is an ARM architecture.
In our case this is not true, its AArch64, and so we cast the tdep
object to arm_gdbarch_tdep when really it is of type
aarch64_gdbarch_tdep.

After this we access the fields of the tdep object, but this is
undefined behaviour.

To fix this I propose adding new code to arm_frame_is_thumb that
checks the bfd architecture of the current frame.  And frame that is
not bfd_arch_arm can't be a thumb frame.

I could add a gdb_assert to arm_psr_thumb_bit, however, I have not
done this.  A later commit in this series will add an assert to the
gdbarch_tdep() call to ensure that the cast is of the correct type.
This will make any assert (of bfd architecture) added to
arm_psr_thumb_bit redundant, so I've not added any such assert here.

With this commit in place the gdb.multi/multi-arch.exp test continues
to pass after I have applied the later patches that detect incorrect
casting of the gdbarch_tdep objects.
---
 gdb/arm-tdep.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 7f27d4bd6e8..46027afade4 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -546,15 +546,19 @@ arm_is_thumb (struct regcache *regcache)
 int
 arm_frame_is_thumb (struct frame_info *frame)
 {
-  CORE_ADDR cpsr;
-  ULONGEST t_bit = arm_psr_thumb_bit (get_frame_arch (frame));
+  /* If the current frame is not ARM then it can't be thumb.  */
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  if (gdbarch_bfd_arch_info (target_gdbarch ())->arch != bfd_arch_arm)
+    return 0;
 
   /* Every ARM frame unwinder can unwind the T bit of the CPSR, either
      directly (from a signal frame or dummy frame) or by interpreting
      the saved LR (from a prologue or DWARF frame).  So consult it and
      trust the unwinders.  */
-  cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM);
+  CORE_ADDR cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM);
 
+  /* Find and extract the thumb bit.  */
+  ULONGEST t_bit = arm_psr_thumb_bit (gdbarch);
   return (cpsr & t_bit) != 0;
 }
 
-- 
2.25.4