diff mbox

[v3,4/4] Add a TRY_CATCH to get_prev_frame to better handle errors during unwind.

Message ID 538672C2.2080601@broadcom.com
State Committed
Headers show

Commit Message

Andrew Burgess May 28, 2014, 11:35 p.m. UTC
On 28/05/2014 7:31 PM, Pedro Alves wrote:
> On 04/30/2014 11:55 AM, Andrew Burgess wrote:
>> Here a new TRY_CATCH is added to the core of get_prev_frame, all uncaught
>> errors are turned into UNWIND_MISC_ERROR, and where possible the error
>> message associated with the error is stored as a frame specific stop reason
>> string.  The reason string is held of the frame OBSTACK so it lives as long
>> as the frame does.
>>
>> There's a new function for getting the frame_stop_reason_string, this
>> replaces the now (thanks to patch #3) old frame_stop_reason_string
>> function, I know that reusing the name could confuse, but this function was
>> not widely used, so I hope that'll not be an issue.
> 
> Sounds like we'll want to expose this to Python too.  If you're not
> planning on doing that, could you file a PR once this goes in?

I'm happy to add the new python (and even guile) functions, I figured
I'd post a follow up patch once these were merged ... if that's OK.

> 
>>
>> I don't know if you'll all be happy with me catching _all_ errors in
>> get_prev_frame.
> 
> Yeah, that makes me somewhat nervous.  E.g., this catches and
> swallows TARGET_CLOSE_ERROR.
> 
>> Given that the issues I'm seeing "in the wild" and which
>> I've put into the tests are for accessing invalid memory through a bad
>> stack pointer, I could reduce the catching to only catch MEMORY_ERRORs,
>> then UNWIND_MISC_ERROR would become UNWIND_MEMORY_ERROR.  
> 
> I think I'd prefer that.  This swallows TARGET_CLOSE_ERROR, for example.

I've changed the patch to only catch MEMORY_ERROR now.

> 
> Could you please include both MI and CLI before/after examples
> in the description/commit log, please?  You had something
> close to that in the cover letter, but that doesn't make it to
> the repository.

I've included my proposed commit message below too along with
the patch.
> 
> 
>> I could even add
>> the memory error detection on top of the new MISC_ERROR case if preferred,
>> though I'd need to think of a new test...
> 
> I didn't understand this.

Sorry, I just meant I could add FRAME_MEMORY_ERROR and FRAME_MISC_ERROR, but
as you point out, some errors really /don't/ make sense to catch.  But given
your good point about TARGET_CLOSE_ERROR I don't think FRAME_MISC_ERROR is a
good idea any more, we now just have FRAME_MEMORY_ERROR as the new stop
reason.
 
>> 	* unwind_stop_reason.def: Add UNWIND_MISC_ERROR.
> 
> Values added here are exposed to Python/scheme automatically.
> But, we need to document them manually in the manual.  E.g.
> for python, in gdb/python.texi in the table beneath the
> description of Frame.unwind_stop_reason ().

Done.

Thanks for the review, and is this OK to push?

Thanks,
Andrew


Commit message:

Currently a MEMORY_ERROR raised during unwinding a frame will cause the
unwind to stop with an error message, for example:

  (gdb) bt
  #0  breakpt () at amd64-invalid-stack-middle.c:27
  #1  0x00000000004008f0 in func5 () at amd64-invalid-stack-middle.c:32
  #2  0x0000000000400900 in func4 () at amd64-invalid-stack-middle.c:38
  #3  0x0000000000400910 in func3 () at amd64-invalid-stack-middle.c:44
  #4  0x0000000000400928 in func2 () at amd64-invalid-stack-middle.c:50
  Cannot access memory at address 0x2aaaaaab0000

However, frame #4 is marked as being the end of the stack unwind, so a
subsequent request for the backtrace looses the error message, such as:

  (gdb) bt
  #0  breakpt () at amd64-invalid-stack-middle.c:27
  #1  0x00000000004008f0 in func5 () at amd64-invalid-stack-middle.c:32
  #2  0x0000000000400900 in func4 () at amd64-invalid-stack-middle.c:38
  #3  0x0000000000400910 in func3 () at amd64-invalid-stack-middle.c:44
  #4  0x0000000000400928 in func2 () at amd64-invalid-stack-middle.c:50

When fetching the backtrace, or requesting the stack depth using the MI
interface the situation is even worse, the first time a request is made
we encounter the memory error and so the MI returns an error instead of
the correct result, for example:

  (gdb) -stack-info-depth
  ^error,msg="Cannot access memory at address 0x2aaaaaab0000"

Or,

  (gdb) -stack-list-frames
  ^error,msg="Cannot access memory at address 0x2aaaaaab0000"

However, once one of these commands has been used gdb has, internally,
walked the stack and figured that out that frame #4 is the bottom of the
stack, so the second time an MI command is tried you'll get the "expected"
result:

  (gdb) -stack-info-depth
  ^done,depth="5"

Or,

  (gdb) -stack-list-frames
  ^done,stack=[frame={level="0", .. snip lots .. }]

After this patch the MEMORY_ERROR encountered during the frame unwind is
attached to frame #4 as the stop reason, and is displayed in the CLI each
time the backtrace is requested.  In the MI, catching the error means that
the "expected" result is returned the first time the MI command is issued.

---

gdb/ChangeLog:

	* frame.c (struct frame_info): Add stop_string field.
	(get_prev_frame_always_1): Renamed from get_prev_frame_always.
	(get_prev_frame_always): Old content moved into
	get_prev_frame_always_1.  Call get_prev_frame_always_1 inside
	TRY_CATCH, handle MEMORY_ERROR exceptions.
	(frame_stop_reason_string): New function definition.
	* frame.h (frame_stop_reason_string): New function declaration.
	* stack.c (frame_info): Switch to frame_stop_reason_string.
	(backtrace_command_1): Switch to frame_stop_reason_string.
	* unwind_stop_reason.def: Add UNWIND_MEMORY_ERROR.
	(LAST_ENTRY): Changed to UNWIND_MEMORY_ERROR.
	* guile/lib/gdb.scm: Add FRAME_UNWIND_MEMORY_ERROR to export list.

gdb/doc/ChangeLog:

	* guile.texi (Frames In Guile): Mention FRAME_UNWIND_MEMORY_ERROR.
	* python.texi (Frames In Python): Mention
	gdb.FRAME_UNWIND_MEMORY_ERROR.

gdb/testsuite/ChangeLog:

	* gdb.arch/amd64-invalid-stack-middle.exp: Update expected results.
	* gdb.arch/amd64-invalid-stack-top.exp: Likewise.
---
 gdb/doc/guile.texi                                 |  3 +
 gdb/doc/python.texi                                |  3 +
 gdb/frame.c                                        | 74 ++++++++++++++++++++--
 gdb/frame.h                                        |  6 ++
 gdb/guile/lib/gdb.scm                              |  1 +
 gdb/stack.c                                        |  4 +-
 .../gdb.arch/amd64-invalid-stack-middle.exp        | 48 +++-----------
 gdb/testsuite/gdb.arch/amd64-invalid-stack-top.exp | 49 +++-----------
 gdb/unwind_stop_reasons.def                        |  5 +-
 9 files changed, 104 insertions(+), 89 deletions(-)

Comments

Pedro Alves May 29, 2014, 9:41 a.m. UTC | #1
On 05/29/2014 12:35 AM, Andrew Burgess wrote:
> On 28/05/2014 7:31 PM, Pedro Alves wrote:
>> On 04/30/2014 11:55 AM, Andrew Burgess wrote:
>>> Here a new TRY_CATCH is added to the core of get_prev_frame, all uncaught
>>> errors are turned into UNWIND_MISC_ERROR, and where possible the error
>>> message associated with the error is stored as a frame specific stop reason
>>> string.  The reason string is held of the frame OBSTACK so it lives as long
>>> as the frame does.
>>>
>>> There's a new function for getting the frame_stop_reason_string, this
>>> replaces the now (thanks to patch #3) old frame_stop_reason_string
>>> function, I know that reusing the name could confuse, but this function was
>>> not widely used, so I hope that'll not be an issue.
>>
>> Sounds like we'll want to expose this to Python too.  If you're not
>> planning on doing that, could you file a PR once this goes in?
> 
> I'm happy to add the new python (and even guile) functions, I figured
> I'd post a follow up patch once these were merged ... if that's OK.

That's great, thanks.

> Commit message:
> 
> Currently a MEMORY_ERROR raised during unwinding a frame will cause the
> unwind to stop with an error message, for example:
> 
>   (gdb) bt
>   #0  breakpt () at amd64-invalid-stack-middle.c:27
>   #1  0x00000000004008f0 in func5 () at amd64-invalid-stack-middle.c:32
>   #2  0x0000000000400900 in func4 () at amd64-invalid-stack-middle.c:38
>   #3  0x0000000000400910 in func3 () at amd64-invalid-stack-middle.c:44
>   #4  0x0000000000400928 in func2 () at amd64-invalid-stack-middle.c:50
>   Cannot access memory at address 0x2aaaaaab0000
> 
> However, frame #4 is marked as being the end of the stack unwind, so a
> subsequent request for the backtrace looses the error message, such as:
> 
>   (gdb) bt
>   #0  breakpt () at amd64-invalid-stack-middle.c:27
>   #1  0x00000000004008f0 in func5 () at amd64-invalid-stack-middle.c:32
>   #2  0x0000000000400900 in func4 () at amd64-invalid-stack-middle.c:38
>   #3  0x0000000000400910 in func3 () at amd64-invalid-stack-middle.c:44
>   #4  0x0000000000400928 in func2 () at amd64-invalid-stack-middle.c:50
> 
> When fetching the backtrace, or requesting the stack depth using the MI
> interface the situation is even worse, the first time a request is made
> we encounter the memory error and so the MI returns an error instead of
> the correct result, for example:
> 
>   (gdb) -stack-info-depth
>   ^error,msg="Cannot access memory at address 0x2aaaaaab0000"
> 
> Or,
> 
>   (gdb) -stack-list-frames
>   ^error,msg="Cannot access memory at address 0x2aaaaaab0000"
> 
> However, once one of these commands has been used gdb has, internally,
> walked the stack and figured that out that frame #4 is the bottom of the
> stack, so the second time an MI command is tried you'll get the "expected"
> result:
> 
>   (gdb) -stack-info-depth
>   ^done,depth="5"
> 
> Or,
> 
>   (gdb) -stack-list-frames
>   ^done,stack=[frame={level="0", .. snip lots .. }]

Now here's an excellent description / commit message.  Thanks!

> 
> After this patch the MEMORY_ERROR encountered during the frame unwind is
> attached to frame #4 as the stop reason, and is displayed in the CLI each
> time the backtrace is requested.

Please show examples of this in the commit log too.

> In the MI, catching the error means that
> the "expected" result is returned the first time the MI command is issued.

> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3199,6 +3199,9 @@ stack corruption.
>  The frame unwinder did not find any saved PC, but we needed
>  one to unwind further.
>  
> +@item gdb.FRAME_UNWIND_MEMORY_ERROR
> +The frame unwinder caused an error while trying to access memory.

> +
> +	      /* The error needs to live as long as the frame does.  */
> +	      stop_string =
> +		FRAME_OBSTACK_CALLOC (strlen (ex.message) + 1, char);

#1 - Per GNU coding conventions, operators go on the next line, and '='
is an operator (assignment), so:

	      stop_string
		= FRAME_OBSTACK_CALLOC (strlen (ex.message) + 1, char);

#2 - sizeof (char) is 1 by definition, so no need for calloc, actually.
And then that might fit on a single line:

	      stop_string = FRAME_OBSTACK_ZALLOC (strlen (ex.message) + 1);

> +	      strcpy (stop_string, ex.message);
> +	      this_frame->stop_string = stop_string;

But even better we could do this instead:

	      size_t size;

              size = strlen (ex.message) + 1;
              this_frame->stop_string = FRAME_OBSTACK_ZALLOC (size);
              memcpy (this_frame->stop_string, ex.message, size);

> +	    }
> +	  prev_frame = NULL;
> +	}
> +      else
> +	throw_exception (ex);
> +    }
> +
> +  return prev_frame;
> +}
> +
>  /* Construct a new "struct frame_info" and link it previous to
>     this_frame.  */
>  
> @@ -2576,6 +2618,24 @@ unwind_frame_stop_reason_string (enum unwind_stop_reason reason)
>      }
>  }
>  

> +/* Return a possibly frame specific string explaining why the unwind
> +   stopped here.  Should only be called for frames that don't have a
> +   previous frame.  If there's no specific reason stored for a frame then
> +   a generic reason string will be returned.  */
> +const char *frame_stop_reason_string (struct frame_info *);

Let's include an example to make it clearer:

/* Return a possibly frame specific string explaining why the unwind
   stopped here.  E.g., if unwinding tripped on a memory error, this
   will return the error description string, which includes the address
   that we failed to access.  If there's no specific reason stored for
   a frame then a generic reason string will be returned.

   Should only be called for frames that don't have a previous frame.  */

(I imagine that this "Should" will need to be addressed when we expose
this to Python somehow.  We wouldn't want failure to comply to that
to cause an internal error.)


> +/* Return a possibly frame specific string explaining why the unwind
> +   stopped at frame FI.  Must only be called if there is no previous
> +   frame.  */

Let's just remove this comment, thus avoiding maintaining the same
description in two places.  Note it was already divergent from the
comment in the header.

> +
> +const char *
> +frame_stop_reason_string (struct frame_info *fi)
> +{
> +  gdb_assert (fi->prev_p);
> +  gdb_assert (fi->prev == NULL);
> +
> +  /* Return the specific string if we have one.  */
> +  if (fi->stop_string)
> +    return fi->stop_string;

  if (fi->stop_string != NULL)
    return fi->stop_string;

Otherwise looks good.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
index 3e03c7c..bc2a2ce 100644
--- a/gdb/doc/guile.texi
+++ b/gdb/doc/guile.texi
@@ -1827,6 +1827,9 @@  stack corruption.
 The frame unwinder did not find any saved PC, but we needed
 one to unwind further.
 
+@item FRAME_UNWIND_MEMORY_ERROR
+The frame unwinder caused an error while trying to access memory.
+
 @item FRAME_UNWIND_FIRST_ERROR
 Any stop reason greater or equal to this value indicates some kind
 of error.  This special value facilitates writing code that tests
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index ce8ec78..006b873 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3199,6 +3199,9 @@  stack corruption.
 The frame unwinder did not find any saved PC, but we needed
 one to unwind further.
 
+@item gdb.FRAME_UNWIND_MEMORY_ERROR
+The frame unwinder caused an error while trying to access memory.
+
 @item gdb.FRAME_UNWIND_FIRST_ERROR
 Any stop reason greater or equal to this value indicates some kind
 of error.  This special value facilitates writing code that tests
diff --git a/gdb/frame.c b/gdb/frame.c
index ee2b711..517ef08 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -145,6 +145,10 @@  struct frame_info
   /* The reason why we could not set PREV, or UNWIND_NO_REASON if we
      could.  Only valid when PREV_P is set.  */
   enum unwind_stop_reason stop_reason;
+
+  /* A frame specific string describing the STOP_REASON in more detail.
+     Only valid when PREV_P is set, but even then may still be NULL.  */
+  const char *stop_string;
 };
 
 /* A frame stash used to speed up frame lookups.  Create a hash table
@@ -1798,14 +1802,12 @@  get_prev_frame_if_no_cycle (struct frame_info *this_frame)
   return prev_frame;
 }
 
-/* Return a "struct frame_info" corresponding to the frame that called
-   THIS_FRAME.  Returns NULL if there is no such frame.
-
-   Unlike get_prev_frame, this function always tries to unwind the
-   frame.  */
+/* Helper function for get_prev_frame_always, this is called inside a
+   TRY_CATCH block.  Return the frame that called THIS_FRAME or NULL if
+   there is no such frame.  This may throw an exception.  */
 
-struct frame_info *
-get_prev_frame_always (struct frame_info *this_frame)
+static struct frame_info *
+get_prev_frame_always_1 (struct frame_info *this_frame)
 {
   struct gdbarch *gdbarch;
 
@@ -1955,6 +1957,46 @@  get_prev_frame_always (struct frame_info *this_frame)
   return get_prev_frame_if_no_cycle (this_frame);
 }
 
+/* Return a "struct frame_info" corresponding to the frame that called
+   THIS_FRAME.  Returns NULL if there is no such frame.
+
+   Unlike get_prev_frame, this function always tries to unwind the
+   frame.  */
+
+struct frame_info *
+get_prev_frame_always (struct frame_info *this_frame)
+{
+  volatile struct gdb_exception ex;
+  struct frame_info *prev_frame = NULL;
+
+  TRY_CATCH (ex, RETURN_MASK_ERROR)
+    {
+      prev_frame = get_prev_frame_always_1 (this_frame);
+    }
+  if (ex.reason < 0)
+    {
+      if (ex.error == MEMORY_ERROR)
+	{
+	  this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+	  if (ex.message != NULL)
+	    {
+	      char *stop_string;
+
+	      /* The error needs to live as long as the frame does.  */
+	      stop_string =
+		FRAME_OBSTACK_CALLOC (strlen (ex.message) + 1, char);
+	      strcpy (stop_string, ex.message);
+	      this_frame->stop_string = stop_string;
+	    }
+	  prev_frame = NULL;
+	}
+      else
+	throw_exception (ex);
+    }
+
+  return prev_frame;
+}
+
 /* Construct a new "struct frame_info" and link it previous to
    this_frame.  */
 
@@ -2576,6 +2618,24 @@  unwind_frame_stop_reason_string (enum unwind_stop_reason reason)
     }
 }
 
+/* Return a possibly frame specific string explaining why the unwind
+   stopped at frame FI.  Must only be called if there is no previous
+   frame.  */
+
+const char *
+frame_stop_reason_string (struct frame_info *fi)
+{
+  gdb_assert (fi->prev_p);
+  gdb_assert (fi->prev == NULL);
+
+  /* Return the specific string if we have one.  */
+  if (fi->stop_string)
+    return fi->stop_string;
+
+  /* Return the generic string if we have nothing better.  */
+  return unwind_frame_stop_reason_string (fi->stop_reason);
+}
+
 /* Return the enum symbol name of REASON as a string, to use in debug
    output.  */
 
diff --git a/gdb/frame.h b/gdb/frame.h
index 5acb2a2..1a094d9 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -507,6 +507,12 @@  enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
 
 const char *unwind_frame_stop_reason_string (enum unwind_stop_reason);
 
+/* Return a possibly frame specific string explaining why the unwind
+   stopped here.  Should only be called for frames that don't have a
+   previous frame.  If there's no specific reason stored for a frame then
+   a generic reason string will be returned.  */
+const char *frame_stop_reason_string (struct frame_info *);
+
 /* Unwind the stack frame so that the value of REGNUM, in the previous
    (up, older) frame is returned.  If VALUEP is NULL, don't
    fetch/compute the value.  Instead just return the location of the
diff --git a/gdb/guile/lib/gdb.scm b/gdb/guile/lib/gdb.scm
index ec739c7..abc4a67 100644
--- a/gdb/guile/lib/gdb.scm
+++ b/gdb/guile/lib/gdb.scm
@@ -169,6 +169,7 @@ 
  FRAME_UNWIND_INNER_ID
  FRAME_UNWIND_SAME_ID
  FRAME_UNWIND_NO_SAVED_PC
+ FRAME_UNWIND_MEMORY_ERROR
 
  frame?
  frame-valid?
diff --git a/gdb/stack.c b/gdb/stack.c
index a113a03..4f16238 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1529,7 +1529,7 @@  frame_info (char *addr_exp, int from_tty)
       reason = get_frame_unwind_stop_reason (fi);
       if (reason != UNWIND_NO_REASON)
 	printf_filtered (_(" Outermost frame: %s\n"),
-			 unwind_frame_stop_reason_string (reason));
+			 frame_stop_reason_string (fi));
     }
   else if (get_frame_type (fi) == TAILCALL_FRAME)
     puts_filtered (" tail call frame");
@@ -1848,7 +1848,7 @@  backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 	  reason = get_frame_unwind_stop_reason (trailing);
 	  if (reason >= UNWIND_FIRST_ERROR)
 	    printf_filtered (_("Backtrace stopped: %s\n"),
-			     unwind_frame_stop_reason_string (reason));
+			     frame_stop_reason_string (trailing));
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index 8ad5b18..b53ad41 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -43,27 +43,11 @@  if ![runto breakpt] {
     return -1
 }
 
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nCannot access memory at address 0x\[0-9a-f\]+" \
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
 	 "first backtrace, with error message"
 
-send_gdb "bt\n"
-gdb_expect {
-    -re "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nCannot access memory at address 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
-	# Currently gdb will not display the error message associated with
-	# the truncated backtrace after the first backtrace has been
-	# completed.  Ideally, we would do this.  If this case is ever hit
-	# then we have started to display the backtrace in all cases and
-	# the xpass should becomd a pass, and the previous pass case below
-	# should be removed, or changed to a fail.
-	xpass "second backtrace, with error message"
-    }
-    -re "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\n$gdb_prompt $" {
-	pass "second backtrace, without error message"
-    }
-    timeout {
-	fail "second backtrace (timeout)"
-    }
-}
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
+	 "second backtrace, with error message"
 
 clean_restart ${binfile}
 
@@ -71,16 +55,9 @@  if ![runto breakpt] {
     return -1
 }
 
-set test_name "check mi -stack-info-depth command, first time"
-send_gdb "interpreter-exec mi \"-stack-info-depth\"\n"
-gdb_expect {
-    -re "\\^done,depth=\"4\"\r\n$gdb_prompt $" {
-	pass $test_name
-    }
-    -re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
-	xfail $test_name
-    }
-}
+gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
+    "\\^done,depth=\"4\"" \
+    "check mi -stack-info-depth command, first time"
 
 gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
     "\\^done,depth=\"4\"" \
@@ -92,16 +69,9 @@  if ![runto breakpt] {
     return -1
 }
 
-set test_name "check mi -stack-list-frames command, first time"
-send_gdb "interpreter-exec mi \"-stack-list-frames\"\n"
-gdb_expect {
-    -re "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"breakpt\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"func5\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"func4\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"3\",addr=\"$hex\",func=\"func3\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\}\\\]\r\n$gdb_prompt $" {
-	pass $test_name
-    }
-    -re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
-	xfail $test_name
-    }
-}
+gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
+    "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"breakpt\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"func5\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"func4\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"3\",addr=\"$hex\",func=\"func3\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\}\\\]" \
+    "check mi -stack-list-frames command, first time"
 
 gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
     "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"breakpt\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"func5\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"func4\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\},frame=\{level=\"3\",addr=\"$hex\",func=\"func3\",file=\"\[^\"\]+\",fullname=\"\[^\"\]+\",line=\"${decimal}\"\}\\\]" \
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-top.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-top.exp
index 0225326..17c79d7 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-top.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-top.exp
@@ -45,27 +45,11 @@  if ![runto breakpt] {
 # Use 'bt no-filters' here as the python filters will raise their own
 # error during initialisation, the no-filters case is simpler.
 
-gdb_test "bt no-filters" "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nCannot access memory at address 0x\[0-9a-f\]+" \
+gdb_test "bt no-filters" "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
 	 "first backtrace, with error message"
 
-send_gdb "bt no-filters\n"
-gdb_expect {
-    -re "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nCannot access memory at address 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
-	# Currently gdb will not display the error message associated with
-	# the truncated backtrace after the first backtrace has been
-	# completed.  Ideally, we would do this.  If this case is ever hit
-	# then we have started to display the backtrace in all cases and
-	# the xpass should becomd a pass, and the previous pass case below
-	# should be removed, or changed to a fail.
-	xpass "second backtrace, with error message"
-    }
-    -re "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\n$gdb_prompt $" {
-	pass "second backtrace, without error message"
-    }
-    timeout {
-	fail "second backtrace (timeout)"
-    }
-}
+gdb_test "bt no-filters" "^bt no-filters\r\n#0 +$hex in func2 \\(\\)\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
+	 "second backtrace, with error message"
 
 clean_restart ${binfile}
 
@@ -73,16 +57,9 @@  if ![runto breakpt] {
     return -1
 }
 
-set test_name "check mi -stack-info-depth command, first time"
-send_gdb "interpreter-exec mi \"-stack-info-depth\"\n"
-gdb_expect {
-    -re "\\^done,depth=\"1\"\r\n$gdb_prompt $" {
-	pass $test_name
-    }
-    -re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
-	xfail $test_name
-    }
-}
+gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
+    "\\^done,depth=\"1\"" \
+    "check mi -stack-info-depth command, first time"
 
 gdb_test "interpreter-exec mi \"-stack-info-depth\"" \
     "\\^done,depth=\"1\"" \
@@ -94,17 +71,9 @@  if ![runto breakpt] {
     return -1
 }
 
-set test_name "check mi -stack-list-frames command, first time"
-send_gdb "interpreter-exec mi \"-stack-list-frames\"\n"
-gdb_expect {
-    -re "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"func2\"\}\\\]\r\n$gdb_prompt $" {
-	pass $test_name
-    }
-    -re "\\^error,msg=\"Cannot access memory at address $hex\"\r\n$gdb_prompt $" {
-	xfail $test_name
-    }
-}
-
+gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
+    "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"func2\"\}\\\]" \
+    "check mi -stack-list-frames command, first time"
 
 gdb_test "interpreter-exec mi \"-stack-list-frames\"" \
     "\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"func2\"\}\\\]" \
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
index d7da7ea..8cef537 100644
--- a/gdb/unwind_stop_reasons.def
+++ b/gdb/unwind_stop_reasons.def
@@ -60,6 +60,9 @@  SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
    one to unwind further.  */
 SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
 
+/* There was an error accessing memory while unwinding this frame.  */
+SET (UNWIND_MEMORY_ERROR, "memory error while unwinding")
+
 #endif /* SET */
 
 
@@ -72,5 +75,5 @@  FIRST_ENTRY (UNWIND_NO_REASON)
 #endif
 
 #ifdef LAST_ENTRY
-LAST_ENTRY (UNWIND_NO_SAVED_PC)
+LAST_ENTRY (UNWIND_MEMORY_ERROR)
 #endif