Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 12 October 2011 15:02:20, Kevin Pouget wrote:

> I wanted to discuss the best way to solve this bug before going any
> further in the development:
> 
> > (gdb) py print gdb.frame_stop_reason_string(2)
> > /home/kevin/travail/git/gdb/gdb/frame.c:2372: internal-error: Invalid frame stop reason
> > A problem internal to GDB has been detected,further debugging may prove unreliable.
> 
> 
> I prepared the attached patch, which requires to change
> 'internal_error' to a simple 'error' (I assume that it can't break
> anything because it ends up calling `exit()', but I didn't check yet),
> 
> but "Frame.unwind_stop_reason ()" easily returns 'invalid frame stop
> reason', for instance

2 == UNWIND_OUTERMOST.  Why would that be invalid?
frame_stop_reason_string isn't handling this, nor UNWIND_NO_REASON.
Is there a reason for that?  I think something like the below
patch would be much better.  This _is_ an internal error / bug after
all.  (We could leave UNWIND_FIRST_ERROR as part of the enum with
`UNWIND_UNAVAILABLE = UNWIND_FIRST_ERROR', I don't have that
a strong preference.)  Better yet could be to define the
values/strings in the same place in a .def file.

Where do magical the numbers come from?  I hope we've not
blessed them as stable.

-- 
Pedro Alves

---
 gdb/frame.c           |    8 ++++++--
 gdb/frame.h           |    4 +++-
 gdb/python/py-frame.c |    2 +-
 gdb/stack.c           |    2 +-
 4 files changed, 11 insertions(+), 5 deletions(-)

Index: src/gdb/frame.c
===================================================================
--- src.orig/gdb/frame.c	2011-10-11 12:43:17.000000000 +0100
+++ src/gdb/frame.c	2011-10-12 15:40:21.593658382 +0100
@@ -2351,9 +2351,15 @@ frame_stop_reason_string (enum unwind_st
 {
   switch (reason)
     {
+    case UNWIND_NO_REASON:
+      return _("no reason");
+
     case UNWIND_NULL_ID:
       return _("unwinder did not report frame ID");
 
+    case UNWIND_OUTERMOST:
+      return _("outermost");
+
     case UNWIND_UNAVAILABLE:
       return _("Not enough registers or memory available to unwind further");
 
@@ -2366,8 +2372,6 @@ frame_stop_reason_string (enum unwind_st
     case UNWIND_NO_SAVED_PC:
       return _("frame did not save the PC");
 
-    case UNWIND_NO_REASON:
-    case UNWIND_FIRST_ERROR:
     default:
       internal_error (__FILE__, __LINE__,
 		      "Invalid frame stop reason");
Index: src/gdb/frame.h
===================================================================
--- src.orig/gdb/frame.h	2011-10-11 12:43:17.000000000 +0100
+++ src/gdb/frame.h	2011-10-12 15:42:14.303658362 +0100
@@ -466,7 +466,7 @@ enum unwind_stop_reason
        abnormal stack termination.  If a backtrace stops for one
        of these reasons, we'll let the user know.  This marker
        is not a valid stop reason.  */
-    UNWIND_FIRST_ERROR,
+#define UNWIND_FIRST_ERROR UNWIND_UNAVAILABLE
 
     /* Can't unwind further, because that would require knowing the
        values of registers or memory that haven't been collected.  */
@@ -487,6 +487,8 @@ enum unwind_stop_reason
     /* The frame unwinder didn't find any saved PC, but we needed
        one to unwind further.  */
     UNWIND_NO_SAVED_PC,
+
+#define UNWIND_STOP_REASON_LAST UNWIND_NO_SAVED_PC
   };
 
 /* Return the reason why we can't unwind past this frame.  */
Index: src/gdb/stack.c
===================================================================
--- src.orig/gdb/stack.c	2011-10-11 12:43:20.000000000 +0100
+++ src/gdb/stack.c	2011-10-12 15:38:23.083658404 +0100
@@ -1625,7 +1625,7 @@ backtrace_command_1 (char *count_exp, in
       enum unwind_stop_reason reason;
 
       reason = get_frame_unwind_stop_reason (trailing);
-      if (reason > UNWIND_FIRST_ERROR)
+      if (reason >= UNWIND_FIRST_ERROR)
 	printf_filtered (_("Backtrace stopped: %s\n"),
 			 frame_stop_reason_string (reason));
     }
Index: src/gdb/python/py-frame.c
===================================================================
--- src.orig/gdb/python/py-frame.c	2011-10-12 15:50:30.000000000 +0100
+++ src/gdb/python/py-frame.c	2011-10-12 15:50:36.583658269 +0100
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < 0 || reason > UNWIND_STOP_REASON_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));