[5/8,AArch64] Teach stub unwinder to terminate gracefully

Message ID 1436273518-5959-6-git-send-email-pierre.langlois@arm.com
State New, archived
Headers

Commit Message

Pierre Langlois July 7, 2015, 12:51 p.m. UTC
  The stub unwinder is used on AArch64 if the target's memory is not
readable at the current PC.  If we purposely kill the inferior before
examining the trace then we get the following issue:

~~~
...
(gdb) trace f
Tracepoint 3 at 0x7fb7fc28c0
(gdb) tstart
(gdb) continue
...
(gdb) tstop
(gdb) tsave /tmp/trace
(gdb) kill
...
(gdb) target tfile /tmp/trace
...
(gdb) tfind
Register 31 is not available.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Found trace frame 0, tracepoint 3
#-1 0x0000007fb7fc28c0 in f () ...
^^^
~~~

This patch teaches the stub unwinder to report to the core frame code
with UNWIND_UNAVAILABLE when either the stack pointer of the return
address are unavailable to read from the target.

The following test cases now pass when enabling tracepoints:

PASS: gdb.trace/pending.exp: trace works: tfind test frame 0
PASS: gdb.trace/pending.exp: trace resolved_in_trace: tfind test frame 0
PASS: gdb.trace/pending.exp: trace installed_in_trace: tfind test frame 0
PASS: gdb.trace/mi-tracepoint-changed.exp: pending resolved: -trace-find frame-number 0

gdb/ChangeLog:

	* aarch64-tdep.c (aarch64_make_stub_cache): Set available_p and
	swallow NOT_AVAILABLE_ERROR.
	(aarch64_stub_this_id): Call frame_id_build_unavailable_stack if
	available_p is not set.
	(aarch64_stub_frame_unwind_stop_reason): New function.
	(aarch64_stub_unwind): Install it.
---
 gdb/aarch64-tdep.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)
  

Comments

Yao Qi July 8, 2015, 4:34 p.m. UTC | #1
Pierre Langlois <pierre.langlois@arm.com> writes:

> The stub unwinder is used on AArch64 if the target's memory is not
> readable at the current PC.  If we purposely kill the inferior before

The stub unwinder is used if the target memory is not readable, for
example, PC is 0x0.  Many GDB ports use stub unwinder to handle this
case.  This is not aarch64 specific.  Please update your commit log to
reflect this.

> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 87a6d61..57976b7 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1109,13 +1109,36 @@ aarch64_make_stub_cache (struct frame_info *this_frame, void **this_cache)
>    cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
>    *this_cache = cache;
>  
> -  cache->prev_sp
> -    = get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
> -  cache->prev_pc = get_frame_pc (this_frame);
> +  TRY
> +    {
> +      cache->prev_sp
> +	= get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);

I feel this is a better way to indent the code,

         cache->prev_sp	= get_frame_register_unsigned (this_frame,
                                                      AARCH64_SP_REGNUM);

> +      cache->prev_pc = get_frame_pc (this_frame);
> +      cache->available_p = 1;
> +    }
> +  CATCH (ex, RETURN_MASK_ERROR)
> +    {
> +      if (ex.error != NOT_AVAILABLE_ERROR)
> +	throw_exception (ex);
> +    }
> +  END_CATCH
>  
>    return cache;
>  }
>  
> +static enum unwind_stop_reason
> +aarch64_stub_frame_unwind_stop_reason (struct frame_info *this_frame,
> +				       void **this_cache)
> +{

We need comments to this function.

> +  struct aarch64_prologue_cache *cache
> +    = aarch64_make_stub_cache (this_frame, this_cache);

I realise that prologue cache is used for stub unwinder.  If stub
unwinder doesn't use all the fields of prologue cache, probably, we can
create a stub cache.  However, it can be a follow-up patch.

This patch is good to me with the change.
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 87a6d61..57976b7 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1109,13 +1109,36 @@  aarch64_make_stub_cache (struct frame_info *this_frame, void **this_cache)
   cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
   *this_cache = cache;
 
-  cache->prev_sp
-    = get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
-  cache->prev_pc = get_frame_pc (this_frame);
+  TRY
+    {
+      cache->prev_sp
+	= get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
+      cache->prev_pc = get_frame_pc (this_frame);
+      cache->available_p = 1;
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
 
   return cache;
 }
 
+static enum unwind_stop_reason
+aarch64_stub_frame_unwind_stop_reason (struct frame_info *this_frame,
+				       void **this_cache)
+{
+  struct aarch64_prologue_cache *cache
+    = aarch64_make_stub_cache (this_frame, this_cache);
+
+  if (!cache->available_p)
+    return UNWIND_UNAVAILABLE;
+
+  return UNWIND_NO_REASON;
+}
+
 /* Our frame ID for a stub frame is the current SP and LR.  */
 
 static void
@@ -1125,7 +1148,10 @@  aarch64_stub_this_id (struct frame_info *this_frame,
   struct aarch64_prologue_cache *cache
     = aarch64_make_stub_cache (this_frame, this_cache);
 
-  *this_id = frame_id_build (cache->prev_sp, cache->prev_pc);
+  if (cache->available_p)
+    *this_id = frame_id_build (cache->prev_sp, cache->prev_pc);
+  else
+    *this_id = frame_id_build_unavailable_stack (cache->prev_pc);
 }
 
 /* Implement the "sniffer" frame_unwind method.  */
@@ -1152,7 +1178,7 @@  aarch64_stub_unwind_sniffer (const struct frame_unwind *self,
 struct frame_unwind aarch64_stub_unwind =
 {
   NORMAL_FRAME,
-  default_frame_unwind_stop_reason,
+  aarch64_stub_frame_unwind_stop_reason,
   aarch64_stub_this_id,
   aarch64_prologue_prev_register,
   NULL,