This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] (ARM Cortex-M) FPU and PSP aware exception frame unwinder


On 04/07/2016 05:07 PM, James-Adam Renquinha Henri wrote:
I submitted it as a bug to the GNU ARM Embedded initially, see here for
details: https://bugs.launchpad.net/gcc-arm-embedded/+bug/1566054

Basically, this patch allow gdb to unwind properly an extended stack
frame, that is an exception frame with FPU state stacked. Additionally,
because all Cortex-M variants have 2 stack pointers, the Main Stack
Pointer (MSP) and the Process Stack Pointer (PSP), the code in the patch
also check which stack was used prior to the exception. That way,
backtraces work beautifully.

In my original submission, I mentioned a known issue that I didn't try
to fix *yet*, because that would involve a lot more work, and the impact
is relatively minor: for a given outer frame, some FPU registers may not
be reported correctly. I hope you don't mind too much. I consider the
current patch still useful, because at least backtraces work, and it's
an annoyance not to be able to get them.

I have feeling people will mind. Ideally it should keep the old behavior intact if possible. So if you can fallback to the old code, it should be ok.

What that said, i'm not going through the arch-specific details, just the more general patch.


--
James-Adam Renquinha Henri, Ing. jr
Ingenieur d'application
CIMEQ INC.


patch


diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0412f71..8f342c1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -470,8 +470,10 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val)
  {
    /* On M-profile devices, do not strip the low bit from EXC_RETURN
       (the magic exception return address).  */
+ /* NOTE: 0xf0000000 is the EXC_RETURN pattern, according to B1.5.8 of the
+    ARMv7-M Reference Manual.  */
    if (gdbarch_tdep (gdbarch)->is_m
-      && (val & 0xfffffff0) == 0xfffffff0)
+      && (val & 0xf0000000) == 0xf0000000)
      return val;

    if (arm_apcs_32)
@@ -2907,13 +2909,31 @@ arm_m_exception_cache (struct frame_info *this_frame)
    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
    struct arm_prologue_cache *cache;
    CORE_ADDR unwound_sp;
+  CORE_ADDR this_lr;
    LONGEST xpsr;
+  int main_stack_used;
+  int extended_frame_type;
+  int stack_regnum;

    cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
    cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);

-  unwound_sp = get_frame_register_unsigned (this_frame,
-					    ARM_SP_REGNUM);
+  /* We need LR to know: 1- if the FPU was used, 2- which stack was used.
+     "B1.5.6 Exception entry behavior" in ARMv7-M Architecture Reference
+     Manual Issue D (or the last one) gives the various bits in LR
+     involved in this. NOTE: this LR is different of the stacked one.  */
+  this_lr
+    = get_frame_register_unsigned (this_frame,
+				   user_reg_map_name_to_regnum (gdbarch,
+								"lr",
+								-1));
+  main_stack_used = (this_lr & 0xf) != 0xd;
+  extended_frame_type = (this_lr & (1 << 4)) == 0;
+  stack_regnum = user_reg_map_name_to_regnum (gdbarch,
+					      main_stack_used ? "sp" : "psp",
+					      -1);
+
+  unwound_sp = get_frame_register_unsigned (this_frame, stack_regnum);

    /* The hardware saves eight 32-bit words, comprising xPSR,
       ReturnAddress, LR (R14), R12, R3, R2, R1, R0.  See details in
@@ -2928,10 +2948,47 @@ arm_m_exception_cache (struct frame_info *this_frame)
    cache->saved_regs[15].addr = unwound_sp + 24;
    cache->saved_regs[ARM_PS_REGNUM].addr = unwound_sp + 28;

+  if (extended_frame_type)
+    {
+      int s0_offset;
+      int fpscr_offset;
+
+      s0_offset = user_reg_map_name_to_regnum (gdbarch, "s0", -1);
+      fpscr_offset = user_reg_map_name_to_regnum (gdbarch, "fpscr", -1);
+
+      if (s0_offset == -1 || fpscr_offset == -1)
+	{
+	  /* Ooops. */
+	  warning (_("can't get register offsets in cache; "
+		     "fpu info may be wrong"));
+	}
+      else
+	{
+	  int i;
+	  int fpu_reg_offset;
+
+	  fpu_reg_offset = unwound_sp + 0x20;
+
+	  /* XXX: This doesn't take into account the lazy stacking, see "Lazy
+	     context save of FP state", in B1.5.7.  */

I'm almost sure we don't want to introduce FIXME's in the code. A fallback to the correct behavior would be ideal.

+	  for (i = 0; i < 16; ++i, fpu_reg_offset += 4)
+	    {
+	      cache->saved_regs[s0_offset + i].addr = fpu_reg_offset;
+	    }
+	  cache->saved_regs[fpscr_offset].addr = unwound_sp + 0x60;
+	}
+
+	/* Offset 0x64 is reserved */
+	cache->prev_sp = unwound_sp + 0x68;
+    }
+  else
+    {
+      cache->prev_sp = unwound_sp + 32;
+    }
+

No need to have the { }'s for single line conditionals.

    /* If bit 9 of the saved xPSR is set, then there is a four-byte
       aligner between the top of the 32-byte stack frame and the
       previous context's stack pointer.  */
-  cache->prev_sp = unwound_sp + 32;
    if (safe_read_memory_integer (unwound_sp + 28, 4, byte_order, &xpsr)
        && (xpsr & (1 << 9)) != 0)
      cache->prev_sp += 4;
@@ -2997,11 +3054,19 @@ arm_m_exception_unwind_sniffer (const struct frame_unwind *self,
    /* Exception frames return to one of these magic PCs.  Other values
       are not defined as of v7-M.  See details in "B1.5.8 Exception
       return behavior" in "ARMv7-M Architecture Reference Manual".  */
-  if (this_pc == 0xfffffff1 || this_pc == 0xfffffff9
-      || this_pc == 0xfffffffd)
-    return 1;
+  switch (this_pc)
+    {
+      case 0xffffffe1:
+      case 0xffffffe9:
+      case 0xffffffed:
+      case 0xfffffff1:
+      case 0xfffffff9:
+      case 0xfffffffd:
+        return 1;

-  return 0;
+      default:
+        return 0;
+    }

The above should probably go to a function that checks for the correct patterns. The code should be cleaner that way.

  }

  /* Frame unwinder for M-profile exceptions.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]