[binutils-gdb] [AArch64] Teach prologue unwinder to terminate gracefully

[Pierre Langlois <planglois at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7dfa3edc033c443036d9f2a3e01120f7fb54f498

commit 7dfa3edc033c443036d9f2a3e01120f7fb54f498
Author: Pierre Langlois <pierre.langlois@arm.com>
Date:   Thu Jul 9 16:35:11 2015 +0100

    [AArch64] Teach prologue unwinder to terminate gracefully
    
    Without debugging information, we have the following issue when
    examining a trace buffer:
    
    ~~~
    ...
    (gdb) trace f
    Tracepoint 3 at 0x7fb7fc28c0
    (gdb) tstart
    (gdb) continue
    ...
    (gdb) tstop
    (gdb) tfind start
    Register 31 is not available.
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    Found trace frame 0, tracepoint 3
    #-1 0x0000007fb7fc28c0 in f () ...
    ^^^
    ~~~
    
    The reason for this is that the target's stack pointer is unavailable
    when examining the trace buffer.  What we are seeing is due to the
    'tfind' command creating a sentinel frame and unwinding it.  If an
    exception is thrown, we are left with the sentinel frame being displayed
    at level #-1.  The exception is thrown when the prologue unwinder tries
    to read the stack pointer to construct an ID for the frame.
    
    This patch fixes this by making the prologue unwinder catch
    NOT_AVAILABLE_ERROR exceptions when either registers or memory is
    unreadable and report back to the frame core code with
    UNWIND_UNAVAILABLE.
    
    gdb/ChangeLog:
    
    	* aarch64-tdep.c (aarch64_prologue_cache) <available_p>: New
    	field.
    	(aarch64_make_prologue_cache_1): New function, factored out from
    	aarch64_make_prologue_cache.  Do not allocate cache.  Set
    	available_p.
    	(aarch64_make_prologue_cache): Reimplement wrapping
    	aarch64_make_prologue_cache_1, and swallowing
    	NOT_AVAILABLE_ERROR.
    	(aarch64_prologue_frame_unwind_stop_reason): New function.
    	Return UNWIND_UNAVAILABLE if available_p is not set.
    	(aarch64_prologue_unwind): Install it.
    	(aarch64_prologue_this_id): Move prev_pc and prev_sp limit
    	checks into aarch64_prologue_frame_unwind_stop_reason.  Call
    	frame_id_build_unavailable_stack if available_p is not set.

Diff:
---
 gdb/ChangeLog      | 17 ++++++++++
 gdb/aarch64-tdep.c | 96 +++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 26 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0cc42a3..f715fcd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,22 @@
 2015-07-09  Pierre Langlois  <pierre.langlois@arm.com>
 
+	* aarch64-tdep.c (aarch64_prologue_cache) <available_p>: New
+	field.
+	(aarch64_make_prologue_cache_1): New function, factored out from
+	aarch64_make_prologue_cache.  Do not allocate cache.  Set
+	available_p.
+	(aarch64_make_prologue_cache): Reimplement wrapping
+	aarch64_make_prologue_cache_1, and swallowing
+	NOT_AVAILABLE_ERROR.
+	(aarch64_prologue_frame_unwind_stop_reason): New function.
+	Return UNWIND_UNAVAILABLE if available_p is not set.
+	(aarch64_prologue_unwind): Install it.
+	(aarch64_prologue_this_id): Move prev_pc and prev_sp limit
+	checks into aarch64_prologue_frame_unwind_stop_reason.  Call
+	frame_id_build_unavailable_stack if available_p is not set.
+
+2015-07-09  Pierre Langlois  <pierre.langlois@arm.com>
+
 	* aarch64-tdep.c (aarch64_prologue_cache) <func, prev_pc>: New
 	fields.
 	(aarch64_scan_prologue): Set prev_pc.
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index fe1b71f..37d3eb3 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -162,6 +162,9 @@ struct aarch64_prologue_cache
      to identify this frame.  */
   CORE_ADDR prev_sp;
 
+  /* Is the target available to read from?  */
+  int available_p;
+
   /* The frame base for this frame is just prev_sp - frame size.
      FRAMESIZE is the distance from the frame pointer to the
      initial stack pointer.  */
@@ -941,33 +944,25 @@ aarch64_scan_prologue (struct frame_info *this_frame,
     }
 }
 
-/* Allocate and fill in *THIS_CACHE with information about the prologue of
-   *THIS_FRAME.  Do not do this is if *THIS_CACHE was already allocated.
-   Return a pointer to the current aarch64_prologue_cache in
-   *THIS_CACHE.  */
+/* Fill in *CACHE with information about the prologue of *THIS_FRAME.  This
+   function may throw an exception if the inferior's registers or memory is
+   not available.  */
 
-static struct aarch64_prologue_cache *
-aarch64_make_prologue_cache (struct frame_info *this_frame, void **this_cache)
+static void
+aarch64_make_prologue_cache_1 (struct frame_info *this_frame,
+			       struct aarch64_prologue_cache *cache)
 {
-  struct aarch64_prologue_cache *cache;
   CORE_ADDR unwound_fp;
   int reg;
 
-  if (*this_cache != NULL)
-    return *this_cache;
-
-  cache = FRAME_OBSTACK_ZALLOC (struct aarch64_prologue_cache);
-  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
-  *this_cache = cache;
-
   aarch64_scan_prologue (this_frame, cache);
 
   if (cache->framereg == -1)
-    return cache;
+    return;
 
   unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
   if (unwound_fp == 0)
-    return cache;
+    return;
 
   cache->prev_sp = unwound_fp + cache->framesize;
 
@@ -979,9 +974,63 @@ aarch64_make_prologue_cache (struct frame_info *this_frame, void **this_cache)
 
   cache->func = get_frame_func (this_frame);
 
+  cache->available_p = 1;
+}
+
+/* Allocate and fill in *THIS_CACHE with information about the prologue of
+   *THIS_FRAME.  Do not do this is if *THIS_CACHE was already allocated.
+   Return a pointer to the current aarch64_prologue_cache in
+   *THIS_CACHE.  */
+
+static struct aarch64_prologue_cache *
+aarch64_make_prologue_cache (struct frame_info *this_frame, void **this_cache)
+{
+  struct aarch64_prologue_cache *cache;
+
+  if (*this_cache != NULL)
+    return *this_cache;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct aarch64_prologue_cache);
+  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+  *this_cache = cache;
+
+  TRY
+    {
+      aarch64_make_prologue_cache_1 (this_frame, cache);
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
+
   return cache;
 }
 
+/* Implement the "stop_reason" frame_unwind method.  */
+
+static enum unwind_stop_reason
+aarch64_prologue_frame_unwind_stop_reason (struct frame_info *this_frame,
+					   void **this_cache)
+{
+  struct aarch64_prologue_cache *cache
+    = aarch64_make_prologue_cache (this_frame, this_cache);
+
+  if (!cache->available_p)
+    return UNWIND_UNAVAILABLE;
+
+  /* Halt the backtrace at "_start".  */
+  if (cache->prev_pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc)
+    return UNWIND_OUTERMOST;
+
+  /* We've hit a wall, stop.  */
+  if (cache->prev_sp == 0)
+    return UNWIND_OUTERMOST;
+
+  return UNWIND_NO_REASON;
+}
+
 /* Our frame ID for a normal frame is the current function's starting
    PC and the caller's SP when we were called.  */
 
@@ -992,15 +1041,10 @@ aarch64_prologue_this_id (struct frame_info *this_frame,
   struct aarch64_prologue_cache *cache
     = aarch64_make_prologue_cache (this_frame, this_cache);
 
-  /* This is meant to halt the backtrace at "_start".  */
-  if (cache->prev_pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc)
-    return;
-
-  /* If we've hit a wall, stop.  */
-  if (cache->prev_sp == 0)
-    return;
-
-  *this_id = frame_id_build (cache->prev_sp, cache->func);
+  if (!cache->available_p)
+    *this_id = frame_id_build_unavailable_stack (cache->func);
+  else
+    *this_id = frame_id_build (cache->prev_sp, cache->func);
 }
 
 /* Implement the "prev_register" frame_unwind method.  */
@@ -1051,7 +1095,7 @@ aarch64_prologue_prev_register (struct frame_info *this_frame,
 struct frame_unwind aarch64_prologue_unwind =
 {
   NORMAL_FRAME,
-  default_frame_unwind_stop_reason,
+  aarch64_prologue_frame_unwind_stop_reason,
   aarch64_prologue_this_id,
   aarch64_prologue_prev_register,
   NULL,