Be stricter about frame pointer unwinding on ARM

There were too many false positives. We require reasonable values for SP
and FP now, and we check that the new value for LR is not null before we
set anything.

Change-Id: I91ca205d0f0ad00e1934f2007d7aa998bb597b35
Reviewed-by: Christian Kandeler <[email protected]>

1 files changed, 11 insertions, 9 deletions

diff --git a/backends/arm_unwind.c b/backends/arm_unwind.c
index 3d1e1f25..240eb56f 100644
--- a/backends/arm_unwind.c
+++ b/backends/arm_unwind.c
@@ -60,16 +60,20 @@ EBLHOOK(unwind) (Ebl *ebl __attribute__ ((unused)), Dwarf_Addr pc __attribute__
 {
   Dwarf_Word fp = 0, sp = 0; // have to be initialized because registers are 32bit only
 
-  if (!getfunc(FP_REG, 1, &fp, arg))
-    fp = 0;
+  if (!getfunc(FP_REG, 1, &fp, arg) || fp == 0)
+    return false;
 
-  if (!getfunc(SP_REG, 1, &sp, arg))
-    sp = 0;
+  if (!getfunc(SP_REG, 1, &sp, arg) || sp == 0)
+    return false;
+
+  // Stack frames of > 8M are rather unlikely in real programs.
+  if (fp <= sp || fp - sp >= 1 << 23)
+    return false;
 
   Dwarf_Word newLr, newFp, newSp;
 
-  if (!readfunc(fp + LR_OFFSET, &newLr, arg))
-    newLr = 0;
+  if (!readfunc(fp + LR_OFFSET, &newLr, arg) || newLr == 0)
+    return false;
 
   if (!readfunc(fp + FP_OFFSET, &newFp, arg))
     newFp = 0;
@@ -87,7 +91,5 @@ EBLHOOK(unwind) (Ebl *ebl __attribute__ ((unused)), Dwarf_Addr pc __attribute__
   setfunc(FP_REG, 1, &newFp, arg);
   setfunc(SP_REG, 1, &newSp, arg);
 
-  // If the fp is invalid, we might still have a valid lr.
-  // But if the fp is valid, then the stack should be moving in the right direction.
-  return newLr != 0 && (fp == 0 || newSp > sp);
+  return true;
 }