[RFC][Python] gdbpy_frame_stop_reason_string bug

[RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

Hi,

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

> (gdb) start
> ...
> (gdb) pp gdb.newest_frame().unwind_stop_reason()
> 0

so I'm not convinced that it's the best solution.

Would it be better to return (in frame_stop_reason_string) a reason
for each value of "enum unwind_stop_reason reason", but breaking the
original intend of the `internal_error' call?

thanks for your comments,

Kevin

[-- Attachment #2: gdbpy_frame_stop_reason_string.patch --]
[-- Type: text/x-patch, Size: 1396 bytes --]

diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..1c1a18b 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2369,8 +2369,7 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
     case UNWIND_NO_REASON:
     case UNWIND_FIRST_ERROR:
     default:
-      internal_error (__FILE__, __LINE__,
-		      "Invalid frame stop reason");
+      error (_("Invalid frame stop reason"));
     }
 }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..2b95112 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -538,19 +538,20 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
 {
   int reason;
   const char *str;
+  PyObject *py_str_reason = NULL;
+  volatile struct gdb_exception except;
 
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      PyErr_SetString (PyExc_ValueError, 
-		       _("Invalid frame stop reason."));
-      return NULL;
+      str = frame_stop_reason_string (reason);
+      py_str_reason = PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
     }
+  GDB_PY_HANDLE_EXCEPTION (except);
 
-  str = frame_stop_reason_string (reason);
-  return PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
+  return py_str_reason;
 }
 
 /* Implements the equality comparison for Frame objects.

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Phil Muldoon]

Kevin Pouget <kevin.pouget@gmail.com> writes:

> Hi,
>
> 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.

Why not use the supplied gdb constants in this case?  "2" does not map
to any enum.

IE gdb.FRAME_UNWIND_NO_REASON

These 1:1 mappings of the internal GDB enums.  

FWIW I think we should error check this function for each enum.  The < 0
&& >UNWIND_NO_SAVED_PC check seems too liberal.

Cheers,

Phil

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves]

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."));

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

On Wed, Oct 12, 2011 at 4:52 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> 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

yes, makes perfectly sense, thanks

the numbers come from there:
http://sourceware.org/gdb/onlinedocs/gdb/Frames-In-Python.html#index-unwind_005fstop_005freason-on-Frame-1870

... I don't know if changing these numbers would be considered as a
backward incompatibility ... ?

From: Phil Muldoon <pmuldoon@redhat.com>
> Why not use the supplied gdb constants in this case?  "2" does not map
> to any enum.
> IE gdb.FRAME_UNWIND_NO_REASON

it looks like these enums are not documented, are they? I can't grep
'FRAME_UNWIND_NO_REASON' in gdb.texinfo

('2' was a bad example, but the reason why I first used '0' was
because it was returned by Frame.unwind_stop_reason(), as depicted in
the first mail)
by the way, python print
gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_FIRST_ERROR) crashes the
same way, there is certainly a few more lines to fix on the Python
side

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Phil Muldoon]

Kevin Pouget <kevin.pouget@gmail.com> writes:


> it looks like these enums are not documented, are they? I can't grep
> 'FRAME_UNWIND_NO_REASON' in gdb.texinfo

Yeah, that at least is a bug for sure.

> ('2' was a bad example, but the reason why I first used '0' was
> because it was returned by Frame.unwind_stop_reason(), as depicted in
> the first mail)
> by the way, python print
> gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_FIRST_ERROR) crashes the
> same way, there is certainly a few more lines to fix on the Python
> side

I can look at this, or you can hack on it.  I don't mind.

Cheers,

Phil

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves]

On Wednesday 12 October 2011 16:28:24, Phil Muldoon wrote:
> Kevin Pouget <kevin.pouget@gmail.com> writes:
> 
> 
> > it looks like these enums are not documented, are they? I can't grep
> > 'FRAME_UNWIND_NO_REASON' in gdb.texinfo
> 
> Yeah, that at least is a bug for sure.
> 
> > ('2' was a bad example, but the reason why I first used '0' was
> > because it was returned by Frame.unwind_stop_reason(), as depicted in
> > the first mail)
> > by the way, python print
> > gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_FIRST_ERROR) crashes the
> > same way, there is certainly a few more lines to fix on the Python
> > side
> 
> I can look at this, or you can hack on it.  I don't mind.

I found this:

  /* Note: These would probably be best exposed as class attributes of
     Frame, but I don't know how to do it except by messing with the
     type's dictionary.  That seems too messy.  */
  PyModule_AddIntConstant (gdb_module, "NORMAL_FRAME", NORMAL_FRAME);
  PyModule_AddIntConstant (gdb_module, "DUMMY_FRAME", DUMMY_FRAME);
  PyModule_AddIntConstant (gdb_module, "INLINE_FRAME", INLINE_FRAME);
  PyModule_AddIntConstant (gdb_module, "TAILCALL_FRAME", TAILCALL_FRAME);
  PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
  PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
  PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
  PyModule_AddIntConstant (gdb_module,
			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);

We should definitely reimplement these enums in a table in a
.def file.  That's 2 places already that map the enums to
something else.  This one is missing UNWIND_UNAVAILABLE.
And what do you think of making UNWIND_FIRST_ERROR
an alias like in my patch?  Do you think that's likely
to break anything?

-- 
Pedro Alves

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> We should definitely reimplement these enums in a table in a
Pedro> .def file.  That's 2 places already that map the enums to
Pedro> something else.  This one is missing UNWIND_UNAVAILABLE.

Makes sense to me.

Pedro> And what do you think of making UNWIND_FIRST_ERROR
Pedro> an alias like in my patch?  Do you think that's likely
Pedro> to break anything?

I think it would be fine.

Tom

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

On Wed, Oct 12, 2011 at 7:06 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>
> Pedro> We should definitely reimplement these enums in a table in a
> Pedro> .def file.  That's 2 places already that map the enums to
> Pedro> something else.  This one is missing UNWIND_UNAVAILABLE.
>
> Makes sense to me.
>
> Pedro> And what do you think of making UNWIND_FIRST_ERROR
> Pedro> an alias like in my patch?  Do you think that's likely
> Pedro> to break anything?
>
> I think it would be fine.


Hello,

here is a patch which implements the enums in a .def file, as
suggested earlier. Please let me know what you think about it (tested
with no regression on X86_64)

UNWIND_FIRST_ERROR is now an "enum alias", instead of being defined by
the preprocessor (the only way I managed to implement it), as well as
UNWIND_FIRST and UNWIND_LAST, used for bound-checking in py-frame.c

I also moved the original enum comments to the .def file with no modification


Thanks,

Kevin

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Thu, Oct 13, 2011 at 1:24 PM, Kevin Pouget <kevin.pouget@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 7:06 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>>
>> Pedro> We should definitely reimplement these enums in a table in a
>> Pedro> .def file.  That's 2 places already that map the enums to
>> Pedro> something else.  This one is missing UNWIND_UNAVAILABLE.
>>
>> Makes sense to me.
>>
>> Pedro> And what do you think of making UNWIND_FIRST_ERROR
>> Pedro> an alias like in my patch?  Do you think that's likely
>> Pedro> to break anything?
>>
>> I think it would be fine.
>
>
> Hello,
>
> here is a patch which implements the enums in a .def file, as
> suggested earlier. Please let me know what you think about it (tested
> with no regression on X86_64)
>
> UNWIND_FIRST_ERROR is now an "enum alias", instead of being defined by
> the preprocessor (the only way I managed to implement it), as well as
> UNWIND_FIRST and UNWIND_LAST, used for bound-checking in py-frame.c
>
> I also moved the original enum comments to the .def file with no modification
>
>
> Thanks,
>
> Kevin

(with the file attached this time, sorry for the repost)

[-- Attachment #2: unwind_stop_reasons.def.patch --]
[-- Type: text/x-patch, Size: 5110 bytes --]

diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..70c372f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index f5866bd..317714c 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..64388dd 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -585,6 +585,8 @@ frapy_richcompare (PyObject *self, PyObject *other, int op)
 void
 gdbpy_initialize_frames (void)
 {
+  int unwind_cpt = 0;
+
   frame_object_type.tp_new = PyType_GenericNew;
   if (PyType_Ready (&frame_object_type) < 0)
     return;
@@ -599,18 +601,11 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, unwind_cpt++);
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves]

The patch appears to be missing unwind_stop_reasons.def.

> +#define SET(name, description) \
> +  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, unwind_cpt++);
> +#include "gdb/unwind_stop_reasons.def"

What's that `unwind_cpt' for?  Wouldn't:

#define SET(name, description) \
  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);

work?

-- 
Pedro Alves

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On Thu, Oct 13, 2011 at 5:19 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> The patch appears to be missing unwind_stop_reasons.def.
>
>> +#define SET(name, description) \
>> +  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, unwind_cpt++);
>> +#include "gdb/unwind_stop_reasons.def"
>
> What's that `unwind_cpt' for?  Wouldn't:
>
> #define SET(name, description) \
>  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
>
> work?

yes indeed, I didn't catch this one

I've updated the patch and include the missing file, thanks

Kevin

gdb/
2011-10-14  Kevin Pouget  <kevin.pouget@st.com>

	* frame.c (frame_stop_reason_string): Rewrite using
	unwind_stop_reasons.def.
	* frame.h (enum unwind_stop_reason): Likewise.
	* python/py-frame.c (gdbpy_initialize_frames): Likewise.
	(gdbpy_frame_stop_reason_string): Use new enum unwind_stop_reason
	constants for bound-checking.

include/gdb/
2011-10-14  Kevin Pouget  <kevin.pouget@st.com>

	* unwind_stop_reasons.def: New file.

[-- Attachment #2: 0001-Move-unwind-reasons-to-an-external-.def-file.patch --]
[-- Type: text/x-patch, Size: 8539 bytes --]

From 84c81ae388947e5863831cd7d0461800268f51b6 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Fri, 14 Oct 2011 10:03:52 +0200
Subject: [PATCH] Move unwind reasons to an external .def file

---
 gdb/frame.c                         |   20 ++-------
 gdb/frame.h                         |   51 ++++------------------
 gdb/python/py-frame.c               |   19 +++------
 include/gdb/unwind_stop_reasons.def |   79 +++++++++++++++++++++++++++++++++++
 4 files changed, 99 insertions(+), 70 deletions(-)
 create mode 100644 include/gdb/unwind_stop_reasons.def

diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..70c372f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index f5866bd..317714c 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..2860aa7 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -599,18 +599,11 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#include "gdb/unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);
diff --git a/include/gdb/unwind_stop_reasons.def b/include/gdb/unwind_stop_reasons.def
new file mode 100644
index 0000000..f628aae
--- /dev/null
+++ b/include/gdb/unwind_stop_reasons.def
@@ -0,0 +1,79 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Reasons why frames could not be further unwound
+   SET (name, description)
+  
+   After this reason name, all reasons should be considered errors;
+   i.e.: abnormal stack termination.
+   FIRST_ERROR (name)  
+   
+   First and Last reason defined
+   FIRST_ENTRY (name)
+   LAST_ENTRY (name)  */
+
+#ifdef SET
+/* No particular reason; either we haven't tried unwinding yet, 
+   or we didn't fail.  */
+SET (UNWIND_NO_REASON, "No reason")
+
+/* The previous frame's analyzer returns an invalid result
+   from this_id.
+
+   FIXME drow/2006-08-16: This is how GDB used to indicate end of
+   stack.  We should migrate to a model where frames always have a
+   valid ID, and this becomes not just an error but an internal
+   error.  But that's a project for another day.  */
+SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
+
+/* This frame is the outermost.  */
+SET (UNWIND_OUTERMOST, "outermost")
+
+/* Can't unwind further, because that would require knowing the
+   values of registers or memory that haven't been collected.  */
+SET (UNWIND_UNAVAILABLE, "Not enough registers or memory available to unwind further")
+
+/* This frame ID looks like it ought to belong to a NEXT frame,
+   but we got it for a PREV frame.  Normally, this is a sign of
+   unwinder failure.  It could also indicate stack corruption.  */
+SET (UNWIND_INNER_ID, "previous frame inner to this frame (corrupt stack?)")
+
+/* This frame has the same ID as the previous one.  That means
+   that unwinding further would almost certainly give us another
+   frame with exactly the same ID, so break the chain.  Normally,
+   this is a sign of unwinder failure.  It could also indicate
+   stack corruption.  */
+SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
+
+/* The frame unwinder didn't find any saved PC, but we needed
+   one to unwind further.  */
+SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
+
+#endif /* SET */
+
+
+#ifdef FIRST_ERROR
+FIRST_ERROR (UNWIND_UNAVAILABLE)
+#endif
+
+#ifdef FIRST_ENTRY
+FIRST_ENTRY (UNWIND_NO_REASON)
+#endif
+
+#ifdef LAST_ENTRY
+LAST_ENTRY (UNWIND_NO_SAVED_PC)
+#endif
-- 
1.7.6.4

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

Kevin> include/gdb/
Kevin> 2011-10-14  Kevin Pouget  <kevin.pouget@st.com>
Kevin> 	* unwind_stop_reasons.def: New file.

Don't put it in include/gdb.  That is too obscure.  Just put this file
directly in the gdb directory along with everything else.

Tom

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On Fri, Oct 14, 2011 at 4:21 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> Kevin> include/gdb/
> Kevin> 2011-10-14  Kevin Pouget  <kevin.pouget@st.com>
> Kevin>  * unwind_stop_reasons.def: New file.
>
> Don't put it in include/gdb.  That is too obscure.  Just put this file
> directly in the gdb directory along with everything else.
>
> Tom

ok, I did it based on signals.def, but it's indeed not relevant here.

I've updated the patch

Thanks,

Kevin

2011-10-14  Kevin Pouget  <kevin.pouget@st.com>

	* frame.c (frame_stop_reason_string): Rewrite using
	unwind_stop_reasons.def.
	* frame.h (enum unwind_stop_reason): Likewise.
	* python/py-frame.c (gdbpy_initialize_frames): Likewise.
	(gdbpy_frame_stop_reason_string): Use new enum unwind_stop_reason
	constants for bound-checking.
	* unwind_stop_reasons.def: New file.

[-- Attachment #2: 0001-Move-unwind-reasons-to-an-external-.def-file.patch --]
[-- Type: text/x-patch, Size: 8482 bytes --]

From eae545861d1b1cbfe28bbc729c72cdd99758e85b Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Fri, 14 Oct 2011 10:03:52 +0200
Subject: [PATCH] Move unwind reasons to an external .def file

---
 gdb/frame.c                 |   20 ++---------
 gdb/frame.h                 |   51 +++++----------------------
 gdb/python/py-frame.c       |   19 +++-------
 gdb/unwind_stop_reasons.def |   79 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 99 insertions(+), 70 deletions(-)
 create mode 100644 gdb/unwind_stop_reasons.def

diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..e5e442a 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index f5866bd..514393b 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..854faa0 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -599,18 +599,11 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#include "../unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
new file mode 100644
index 0000000..f628aae
--- /dev/null
+++ b/gdb/unwind_stop_reasons.def
@@ -0,0 +1,79 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Reasons why frames could not be further unwound
+   SET (name, description)
+  
+   After this reason name, all reasons should be considered errors;
+   i.e.: abnormal stack termination.
+   FIRST_ERROR (name)  
+   
+   First and Last reason defined
+   FIRST_ENTRY (name)
+   LAST_ENTRY (name)  */
+
+#ifdef SET
+/* No particular reason; either we haven't tried unwinding yet, 
+   or we didn't fail.  */
+SET (UNWIND_NO_REASON, "No reason")
+
+/* The previous frame's analyzer returns an invalid result
+   from this_id.
+
+   FIXME drow/2006-08-16: This is how GDB used to indicate end of
+   stack.  We should migrate to a model where frames always have a
+   valid ID, and this becomes not just an error but an internal
+   error.  But that's a project for another day.  */
+SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
+
+/* This frame is the outermost.  */
+SET (UNWIND_OUTERMOST, "outermost")
+
+/* Can't unwind further, because that would require knowing the
+   values of registers or memory that haven't been collected.  */
+SET (UNWIND_UNAVAILABLE, "Not enough registers or memory available to unwind further")
+
+/* This frame ID looks like it ought to belong to a NEXT frame,
+   but we got it for a PREV frame.  Normally, this is a sign of
+   unwinder failure.  It could also indicate stack corruption.  */
+SET (UNWIND_INNER_ID, "previous frame inner to this frame (corrupt stack?)")
+
+/* This frame has the same ID as the previous one.  That means
+   that unwinding further would almost certainly give us another
+   frame with exactly the same ID, so break the chain.  Normally,
+   this is a sign of unwinder failure.  It could also indicate
+   stack corruption.  */
+SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
+
+/* The frame unwinder didn't find any saved PC, but we needed
+   one to unwind further.  */
+SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
+
+#endif /* SET */
+
+
+#ifdef FIRST_ERROR
+FIRST_ERROR (UNWIND_UNAVAILABLE)
+#endif
+
+#ifdef FIRST_ENTRY
+FIRST_ENTRY (UNWIND_NO_REASON)
+#endif
+
+#ifdef LAST_ENTRY
+LAST_ENTRY (UNWIND_NO_SAVED_PC)
+#endif
-- 
1.7.6.4

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves]

On Friday 14 October 2011 15:40:49, Kevin Pouget wrote:

> 2011-10-14  Kevin Pouget  <kevin.pouget@st.com>
> 
> 	* frame.c (frame_stop_reason_string): Rewrite using
> 	unwind_stop_reasons.def.
> 	* frame.h (enum unwind_stop_reason): Likewise.
> 	* python/py-frame.c (gdbpy_initialize_frames): Likewise.
> 	(gdbpy_frame_stop_reason_string): Use new enum unwind_stop_reason
> 	constants for bound-checking.
> 	* unwind_stop_reasons.def: New file.
> 

You're missing this change that was in my patch:

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));
     }

because before, UNWIND_FIRST_ERROR was not an alias, now it is.

I notice only these two descriptions are capitalized:

> +SET (UNWIND_NO_REASON, "No reason")

> +SET (UNWIND_UNAVAILABLE, "Not enough registers or memory available to unwind further")

(the latter my fault).  Can you lowercase them too for consistency?

Also, I believe as is, we lose gdb.FRAME_UNWIND_FIRST_ERROR?

-- 
Pedro Alves

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

On Fri, Oct 14, 2011 at 5:00 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 14 October 2011 15:40:49, Kevin Pouget wrote:
>
>> 2011-10-14  Kevin Pouget  <kevin.pouget@st.com>
>>
>>       * frame.c (frame_stop_reason_string): Rewrite using
>>       unwind_stop_reasons.def.
>>       * frame.h (enum unwind_stop_reason): Likewise.
>>       * python/py-frame.c (gdbpy_initialize_frames): Likewise.
>>       (gdbpy_frame_stop_reason_string): Use new enum unwind_stop_reason
>>       constants for bound-checking.
>>       * unwind_stop_reasons.def: New file.
>>
>
> You're missing this change that was in my patch:
>
> 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));
>     }
>
> because before, UNWIND_FIRST_ERROR was not an alias, now it is.

fixed

> I notice only these two descriptions are capitalized:
>
>> +SET (UNWIND_NO_REASON, "No reason")
>
>> +SET (UNWIND_UNAVAILABLE, "Not enough registers or memory available to unwind further")
>
> (the latter my fault).  Can you lowercase them too for consistency?

fixed

> Also, I believe as is, we lose gdb.FRAME_UNWIND_FIRST_ERROR?

you're right, I've added it, as well as a description in the documentation:

> @item gdb.FRAME_UNWIND_FIRST_ERROR
> All the conditions after this alias are considered errors;
> abnormal stack termination. Current value is gdb.FRAME_UNWIND_UNAVAILABLE.

I'm not 100% convinced about the last part, but I feel like it would
be important to know at read time (instead of run time) what is
considered as an error and what isn't. Let me know if you want me to
remove/rephrase it.


Thanks,

Kevin

[-- Attachment #2: 0001-Move-unwind-reasons-to-an-external-.def-file.patch --]
[-- Type: text/x-patch, Size: 9626 bytes --]

From f37e4adb86342a694ae32b8a8737dc1860d85f53 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Mon, 17 Oct 2011 09:42:44 +0200
Subject: [PATCH] Move unwind reasons to an external .def file

---
 gdb/doc/gdb.texinfo         |    4 ++
 gdb/frame.c                 |   20 ++---------
 gdb/frame.h                 |   51 +++++----------------------
 gdb/python/py-frame.c       |   21 ++++-------
 gdb/stack.c                 |    2 +-
 gdb/unwind_stop_reasons.def |   79 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 106 insertions(+), 71 deletions(-)
 create mode 100644 gdb/unwind_stop_reasons.def

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0aa90eb..e6e060c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23445,6 +23445,10 @@ stack corruption.
 @item gdb.FRAME_UNWIND_NO_SAVED_PC
 The frame unwinder did not find any saved PC, but we needed
 one to unwind further.
+
+@item gdb.FRAME_UNWIND_FIRST_ERROR
+All the conditions after this alias are considered errors;
+abnormal stack termination. Current value is gdb.FRAME_UNWIND_UNAVAILABLE.
 @end table
 
 @end defun
diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..e5e442a 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index f5866bd..514393b 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..4983c82 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -599,18 +599,13 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#define FIRST_ERROR(name) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#include "../unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);
diff --git a/gdb/stack.c b/gdb/stack.c
index 953d3bd..003725a 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1625,7 +1625,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int from_tty)
       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));
     }
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
new file mode 100644
index 0000000..3eca2b2
--- /dev/null
+++ b/gdb/unwind_stop_reasons.def
@@ -0,0 +1,79 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Reasons why frames could not be further unwound
+   SET (name, description)
+  
+   After this reason name, all reasons should be considered errors;
+   i.e.: abnormal stack termination.
+   FIRST_ERROR (name)  
+   
+   First and Last reason defined
+   FIRST_ENTRY (name)
+   LAST_ENTRY (name)  */
+
+#ifdef SET
+/* No particular reason; either we haven't tried unwinding yet, 
+   or we didn't fail.  */
+SET (UNWIND_NO_REASON, "no reason")
+
+/* The previous frame's analyzer returns an invalid result
+   from this_id.
+
+   FIXME drow/2006-08-16: This is how GDB used to indicate end of
+   stack.  We should migrate to a model where frames always have a
+   valid ID, and this becomes not just an error but an internal
+   error.  But that's a project for another day.  */
+SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
+
+/* This frame is the outermost.  */
+SET (UNWIND_OUTERMOST, "outermost")
+
+/* Can't unwind further, because that would require knowing the
+   values of registers or memory that haven't been collected.  */
+SET (UNWIND_UNAVAILABLE, "not enough registers or memory available to unwind further")
+
+/* This frame ID looks like it ought to belong to a NEXT frame,
+   but we got it for a PREV frame.  Normally, this is a sign of
+   unwinder failure.  It could also indicate stack corruption.  */
+SET (UNWIND_INNER_ID, "previous frame inner to this frame (corrupt stack?)")
+
+/* This frame has the same ID as the previous one.  That means
+   that unwinding further would almost certainly give us another
+   frame with exactly the same ID, so break the chain.  Normally,
+   this is a sign of unwinder failure.  It could also indicate
+   stack corruption.  */
+SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
+
+/* The frame unwinder didn't find any saved PC, but we needed
+   one to unwind further.  */
+SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
+
+#endif /* SET */
+
+
+#ifdef FIRST_ERROR
+FIRST_ERROR (UNWIND_UNAVAILABLE)
+#endif
+
+#ifdef FIRST_ENTRY
+FIRST_ENTRY (UNWIND_NO_REASON)
+#endif
+
+#ifdef LAST_ENTRY
+LAST_ENTRY (UNWIND_NO_SAVED_PC)
+#endif
-- 
1.7.6.4

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Tom Tromey]

Kevin> I'm not 100% convinced about the last part, but I feel like it would
Kevin> be important to know at read time (instead of run time) what is
Kevin> considered as an error and what isn't. Let me know if you want me to
Kevin> remove/rephrase it.

This needs a doc review.

The code bits are fine with one nit fixed.

Kevin> +#include "../unwind_stop_reasons.def"

I don't think you need ".." here.

Thanks for doing this.

Tom

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Wed, Oct 19, 2011 at 10:59 PM, Tom Tromey <tromey@redhat.com> wrote:
> Kevin> I'm not 100% convinced about the last part, but I feel like it would
> Kevin> be important to know at read time (instead of run time) what is
> Kevin> considered as an error and what isn't. Let me know if you want me to
> Kevin> remove/rephrase it.
>
> This needs a doc review.
>
> The code bits are fine with one nit fixed.
>
> Kevin> +#include "../unwind_stop_reasons.def"
>
> I don't think you need ".." here.
>
> Thanks for doing this.
>
> Tom

ping, you might have missed this thread Eli

Thanks,

Kevin

[-- Attachment #2: 0001-Move-unwind-reasons-to-an-external-.def-file.patch --]
[-- Type: text/x-patch, Size: 9627 bytes --]

From f37e4adb86342a694ae32b8a8737dc1860d85f53 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Mon, 17 Oct 2011 09:42:44 +0200
Subject: [PATCH] Move unwind reasons to an external .def file

---
 gdb/doc/gdb.texinfo         |    4 ++
 gdb/frame.c                 |   20 ++---------
 gdb/frame.h                 |   51 +++++----------------------
 gdb/python/py-frame.c       |   21 ++++-------
 gdb/stack.c                 |    2 +-
 gdb/unwind_stop_reasons.def |   79 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 106 insertions(+), 71 deletions(-)
 create mode 100644 gdb/unwind_stop_reasons.def

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0aa90eb..e6e060c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23445,6 +23445,10 @@ stack corruption.
 @item gdb.FRAME_UNWIND_NO_SAVED_PC
 The frame unwinder did not find any saved PC, but we needed
 one to unwind further.
+
+@item gdb.FRAME_UNWIND_FIRST_ERROR
+All the conditions after this alias are considered errors;
+abnormal stack termination.  Current value is gdb.FRAME_UNWIND_UNAVAILABLE.
 @end table
 
 @end defun
diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..e5e442a 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index f5866bd..514393b 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 75aa44e..4983c82 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -599,18 +599,13 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#define FIRST_ERROR(name) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#include "../unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);
diff --git a/gdb/stack.c b/gdb/stack.c
index 953d3bd..003725a 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1625,7 +1625,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int from_tty)
       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));
     }
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
new file mode 100644
index 0000000..3eca2b2
--- /dev/null
+++ b/gdb/unwind_stop_reasons.def
@@ -0,0 +1,79 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Reasons why frames could not be further unwound
+   SET (name, description)
+  
+   After this reason name, all reasons should be considered errors;
+   i.e.: abnormal stack termination.
+   FIRST_ERROR (name)  
+   
+   First and Last reason defined
+   FIRST_ENTRY (name)
+   LAST_ENTRY (name)  */
+
+#ifdef SET
+/* No particular reason; either we haven't tried unwinding yet, 
+   or we didn't fail.  */
+SET (UNWIND_NO_REASON, "no reason")
+
+/* The previous frame's analyzer returns an invalid result
+   from this_id.
+
+   FIXME drow/2006-08-16: This is how GDB used to indicate end of
+   stack.  We should migrate to a model where frames always have a
+   valid ID, and this becomes not just an error but an internal
+   error.  But that's a project for another day.  */
+SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
+
+/* This frame is the outermost.  */
+SET (UNWIND_OUTERMOST, "outermost")
+
+/* Can't unwind further, because that would require knowing the
+   values of registers or memory that haven't been collected.  */
+SET (UNWIND_UNAVAILABLE, "not enough registers or memory available to unwind further")
+
+/* This frame ID looks like it ought to belong to a NEXT frame,
+   but we got it for a PREV frame.  Normally, this is a sign of
+   unwinder failure.  It could also indicate stack corruption.  */
+SET (UNWIND_INNER_ID, "previous frame inner to this frame (corrupt stack?)")
+
+/* This frame has the same ID as the previous one.  That means
+   that unwinding further would almost certainly give us another
+   frame with exactly the same ID, so break the chain.  Normally,
+   this is a sign of unwinder failure.  It could also indicate
+   stack corruption.  */
+SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
+
+/* The frame unwinder didn't find any saved PC, but we needed
+   one to unwind further.  */
+SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
+
+#endif /* SET */
+
+
+#ifdef FIRST_ERROR
+FIRST_ERROR (UNWIND_UNAVAILABLE)
+#endif
+
+#ifdef FIRST_ENTRY
+FIRST_ENTRY (UNWIND_NO_REASON)
+#endif
+
+#ifdef LAST_ENTRY
+LAST_ENTRY (UNWIND_NO_SAVED_PC)
+#endif
-- 
1.7.6.4

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Mon, 24 Oct 2011 18:27:01 +0200
> Cc: Pedro Alves <pedro@codesourcery.com>, pmuldoon@redhat.com, gdb-patches@sourceware.org, 
> 	Tom Tromey <tromey@redhat.com>
> 
> ping, you might have missed this thread Eli

Yes, sorry.

> +@item gdb.FRAME_UNWIND_FIRST_ERROR
> +All the conditions after this alias are considered errors;

Hmm...  This table is preceded by this text:

  @defun Frame.unwind_stop_reason ()
  Return an integer representing the reason why it's not possible to find
  more frames toward the outermost frame.  Use
  @code{gdb.frame_stop_reason_string} to convert the value returned by this
  function to a string. The value can be one of:

So "conditions after this alias" seems inappropriate in the list that
follows, because we are not describing conditions or aliases.  Can you
rephrase this to be consistent with the rest of the list.

And why is it important that the value is an alias for another?

> +abnormal stack termination.  Current value is gdb.FRAME_UNWIND_UNAVAILABLE.
                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be in @code{}.

Okay with these changes.

Thanks.

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

On Mon, Oct 24, 2011 at 7:13 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kevin Pouget <kevin.pouget@gmail.com>
>> Date: Mon, 24 Oct 2011 18:27:01 +0200
>> Cc: Pedro Alves <pedro@codesourcery.com>, pmuldoon@redhat.com, gdb-patches@sourceware.org,
>>       Tom Tromey <tromey@redhat.com>
>>
>> ping, you might have missed this thread Eli
>
> Yes, sorry.
>
>> +@item gdb.FRAME_UNWIND_FIRST_ERROR
>> +All the conditions after this alias are considered errors;
>
> Hmm...  This table is preceded by this text:
>
>  @defun Frame.unwind_stop_reason ()
>  Return an integer representing the reason why it's not possible to find
>  more frames toward the outermost frame.  Use
>  @code{gdb.frame_stop_reason_string} to convert the value returned by this
>  function to a string. The value can be one of:
>
> So "conditions after this alias" seems inappropriate in the list that
> follows, because we are not describing conditions or aliases.  Can you
> rephrase this to be consistent with the rest of the list.

I'm not sure about you see wrong with "alias". We could replace it
with "reference" (like the & operator in C++, but also used in Python
and Java), but it sounds more or less the same to me.

What do you think about:
"Stop reasons greater or equal to this value/alias/reference"

> And why is it important that the value is an alias for another?

it's important because it's not a distinct value as the other ones, so
> frame_stop_reason_string(UNWIND_FIRST_ERROR) == frame_stop_reason_string(UNWIND_UNAVAILABLE)
is True, which might be counter-intuitive if you don't know that
UNWIND_FIRST_ERROR is an alias/reference


>> +abnormal stack termination.  Current value is gdb.FRAME_UNWIND_UNAVAILABLE.                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This should be in @code{}.

I'll change it

> Okay with these changes.

Thanks for your time,

Kevin

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Tue, 25 Oct 2011 09:20:19 +0200
> Cc: pedro@codesourcery.com, pmuldoon@redhat.com, gdb-patches@sourceware.org, 
> 	tromey@redhat.com
> 
> >> +@item gdb.FRAME_UNWIND_FIRST_ERROR
> >> +All the conditions after this alias are considered errors;
> >
> > Hmm...  This table is preceded by this text:
> >
> >  @defun Frame.unwind_stop_reason ()
> >  Return an integer representing the reason why it's not possible to find
> >  more frames toward the outermost frame.  Use
> >  @code{gdb.frame_stop_reason_string} to convert the value returned by this
> >  function to a string. The value can be one of:
> >
> > So "conditions after this alias" seems inappropriate in the list that
> > follows, because we are not describing conditions or aliases.  Can you
> > rephrase this to be consistent with the rest of the list.
> 
> I'm not sure about you see wrong with "alias". We could replace it
> with "reference" (like the & operator in C++, but also used in Python
> and Java), but it sounds more or less the same to me.
> 
> What do you think about:
> "Stop reasons greater or equal to this value/alias/reference"

Now that I understand the intent, I would suggest

  Any stop reason greater or equal to this value indicates some kind
  of error.  This special value facilitates writing code that tests
  for errors in unwinding in a way that will work correctly even if
  the list of the other values is modified in future @value{GDBN}
  versions.  Using it, you could write:
  @smallexample
  ... insert here a snippet of code using this value ...
  @end smallexample

> > And why is it important that the value is an alias for another?
> 
> it's important because it's not a distinct value as the other ones, so
> > frame_stop_reason_string(UNWIND_FIRST_ERROR) == frame_stop_reason_string(UNWIND_UNAVAILABLE)
> is True, which might be counter-intuitive if you don't know that
> UNWIND_FIRST_ERROR is an alias/reference

I still don't see the importance, sorry.  Moreover, having this text
means that we will need to update the manual each time the list of
unwind reasons is modified, which in a way works against this very
feature.

Thanks.

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

On Tue, Oct 25, 2011 at 2:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kevin Pouget <kevin.pouget@gmail.com>
>> Date: Tue, 25 Oct 2011 09:20:19 +0200
>> Cc: pedro@codesourcery.com, pmuldoon@redhat.com, gdb-patches@sourceware.org,
>>       tromey@redhat.com
>>
>> >> +@item gdb.FRAME_UNWIND_FIRST_ERROR
>> >> +All the conditions after this alias are considered errors;
>> >
>> > Hmm...  This table is preceded by this text:
>> >
>> >  @defun Frame.unwind_stop_reason ()
>> >  Return an integer representing the reason why it's not possible to find
>> >  more frames toward the outermost frame.  Use
>> >  @code{gdb.frame_stop_reason_string} to convert the value returned by this
>> >  function to a string. The value can be one of:
>> >
>> > So "conditions after this alias" seems inappropriate in the list that
>> > follows, because we are not describing conditions or aliases.  Can you
>> > rephrase this to be consistent with the rest of the list.
>>
>> I'm not sure about you see wrong with "alias". We could replace it
>> with "reference" (like the & operator in C++, but also used in Python
>> and Java), but it sounds more or less the same to me.
>>
>> What do you think about:
>> "Stop reasons greater or equal to this value/alias/reference"
>
> Now that I understand the intent, I would suggest
>
>  Any stop reason greater or equal to this value indicates some kind
>  of error.  This special value facilitates writing code that tests
>  for errors in unwinding in a way that will work correctly even if
>  the list of the other values is modified in future @value{GDBN}
>  versions.  Using it, you could write:

yes, if you feel like it has to be explicited, it's better this way

@smallexample
reason = gdb.selected_frame().unwind_stop_reason ()
reason_str =  gdb.frame_stop_reason_string (reason)
if reason >=  gdb.FRAME_UNWIND_FIRST_ERROR:
    print "An error occured during the unwinding process: %s" % reason_str

@end smallexample
>
>> > And why is it important that the value is an alias for another?
>>
>> it's important because it's not a distinct value as the other ones, so
>> > frame_stop_reason_string(UNWIND_FIRST_ERROR) == frame_stop_reason_string(UNWIND_UNAVAILABLE)
>> is True, which might be counter-intuitive if you don't know that
>> UNWIND_FIRST_ERROR is an alias/reference
>
> I still don't see the importance, sorry.  Moreover, having this text
> means that we will need to update the manual each time the list of
> unwind reasons is modified, **which in a way works against this very
> feature.**

You're half right with this last sentence, but this Python value is
not here to ease *our* (GDB developers) lives, but rather for Python
developers.
this value is not meant to change very often (not to say almost
never), so I think it's not a big burden to change the documentation
as well when it happens.
The very reason why I wrote it is that I think it's useful for the
reader: when you look over this list, you can easily see what is a
normal reason for unwinders to stop, and what isn't (I think that not
so many people will actually have the chance to use it, but out of
curiosity, some people will want to read and understand it).


But that put aside, you're not convinced yet and I'm running short of
arguments ... Pedro, Tom, Phil, how do you see that ?

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

[-- Attachment #1: Type: text/plain, Size: 4490 bytes --]

On Tue, Oct 25, 2011 at 2:48 PM, Kevin Pouget <kevin.pouget@gmail.com> wrote:
> On Tue, Oct 25, 2011 at 2:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Kevin Pouget <kevin.pouget@gmail.com>
>>> Date: Tue, 25 Oct 2011 09:20:19 +0200
>>> Cc: pedro@codesourcery.com, pmuldoon@redhat.com, gdb-patches@sourceware.org,
>>>       tromey@redhat.com
>>>
>>> >> +@item gdb.FRAME_UNWIND_FIRST_ERROR
>>> >> +All the conditions after this alias are considered errors;
>>> >
>>> > Hmm...  This table is preceded by this text:
>>> >
>>> >  @defun Frame.unwind_stop_reason ()
>>> >  Return an integer representing the reason why it's not possible to find
>>> >  more frames toward the outermost frame.  Use
>>> >  @code{gdb.frame_stop_reason_string} to convert the value returned by this
>>> >  function to a string. The value can be one of:
>>> >
>>> > So "conditions after this alias" seems inappropriate in the list that
>>> > follows, because we are not describing conditions or aliases.  Can you
>>> > rephrase this to be consistent with the rest of the list.
>>>
>>> I'm not sure about you see wrong with "alias". We could replace it
>>> with "reference" (like the & operator in C++, but also used in Python
>>> and Java), but it sounds more or less the same to me.
>>>
>>> What do you think about:
>>> "Stop reasons greater or equal to this value/alias/reference"
>>
>> Now that I understand the intent, I would suggest
>>
>>  Any stop reason greater or equal to this value indicates some kind
>>  of error.  This special value facilitates writing code that tests
>>  for errors in unwinding in a way that will work correctly even if
>>  the list of the other values is modified in future @value{GDBN}
>>  versions.  Using it, you could write:
>
> yes, if you feel like it has to be explicited, it's better this way
>
> @smallexample
> reason = gdb.selected_frame().unwind_stop_reason ()
> reason_str =  gdb.frame_stop_reason_string (reason)
> if reason >=  gdb.FRAME_UNWIND_FIRST_ERROR:
>    print "An error occured during the unwinding process: %s" % reason_str
>
> @end smallexample
>>
>>> > And why is it important that the value is an alias for another?
>>>
>>> it's important because it's not a distinct value as the other ones, so
>>> > frame_stop_reason_string(UNWIND_FIRST_ERROR) == frame_stop_reason_string(UNWIND_UNAVAILABLE)
>>> is True, which might be counter-intuitive if you don't know that
>>> UNWIND_FIRST_ERROR is an alias/reference
>>
>> I still don't see the importance, sorry.  Moreover, having this text
>> means that we will need to update the manual each time the list of
>> unwind reasons is modified, **which in a way works against this very
>> feature.**
>
> You're half right with this last sentence, but this Python value is
> not here to ease *our* (GDB developers) lives, but rather for Python
> developers.
> this value is not meant to change very often (not to say almost
> never), so I think it's not a big burden to change the documentation
> as well when it happens.
> The very reason why I wrote it is that I think it's useful for the
> reader: when you look over this list, you can easily see what is a
> normal reason for unwinders to stop, and what isn't (I think that not
> so many people will actually have the chance to use it, but out of
> curiosity, some people will want to read and understand it).
>
>
> But that put aside, you're not convinced yet and I'm running short of
> arguments ... Pedro, Tom, Phil, how do you see that ?

Hello,

there was a line longer than 80char in unwind_reason.def, I've split it

I've also added the definition for FRAME_UNWIND_FIRST_ERROR that you
suggested Eli
(I've truncated the example to

> reason = gdb.selected_frame().unwind_stop_reason ()
> reason_str =  gdb.frame_stop_reason_string (reason)
> if reason >=  gdb.FRAME_UNWIND_FIRST_ERROR:
>     print "An error occured: %s" % reason_str

because the full line was too long for the PDF.

is it your last word for not mentioning the current value of the alias?


Thanks,

Kevin

2011-10-14  Kevin Pouget  <kevin.pouget@st.com>

	* frame.c (frame_stop_reason_string): Rewrite using
	unwind_stop_reasons.def.
	* frame.h (enum unwind_stop_reason): Likewise.
	* python/py-frame.c (gdbpy_initialize_frames): Likewise.
	(gdbpy_frame_stop_reason_string): Use new enum unwind_stop_reason
	constants for bound-checking.
	* unwind_stop_reasons.def: New file.

[-- Attachment #2: 0001-Move-unwind-reasons-to-an-external-.def-file.patch --]
[-- Type: text/x-patch, Size: 10038 bytes --]

From d310b0e18b3b6aaf029b16bfcfa9ba9e056e4348 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Mon, 17 Oct 2011 09:42:44 +0200
Subject: [PATCH] Move unwind reasons to an external .def file

---
 gdb/doc/gdb.texinfo         |   13 +++++++
 gdb/frame.c                 |   20 ++--------
 gdb/frame.h                 |   51 +++++----------------------
 gdb/python/py-frame.c       |   21 ++++-------
 gdb/stack.c                 |    2 +-
 gdb/unwind_stop_reasons.def |   80 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 71 deletions(-)
 create mode 100644 gdb/unwind_stop_reasons.def

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 69e2bda..f99fd4a4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23445,6 +23445,19 @@ stack corruption.
 @item gdb.FRAME_UNWIND_NO_SAVED_PC
 The frame unwinder did not find any saved PC, but we needed
 one to unwind further.
+
+@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
+for errors in unwinding in a way that will work correctly even if
+the list of the other values is modified in future @value{GDBN}
+versions.  Using it, you could write:
+@smallexample
+reason = gdb.selected_frame().unwind_stop_reason ()
+reason_str =  gdb.frame_stop_reason_string (reason)
+if reason >=  gdb.FRAME_UNWIND_FIRST_ERROR:
+    print "An error occured: %s" % reason_str
+@end smallexample
 @end table
 
 @end defun
diff --git a/gdb/frame.c b/gdb/frame.c
index 5824020..e5e442a 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2351,23 +2351,11 @@ frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
-    case UNWIND_NULL_ID:
-      return _("unwinder did not report frame ID");
+#define SET(name, description) \
+    case name: return _(description);
+#include "unwind_stop_reasons.def"
+#undef SET
 
-    case UNWIND_UNAVAILABLE:
-      return _("Not enough registers or memory available to unwind further");
-
-    case UNWIND_INNER_ID:
-      return _("previous frame inner to this frame (corrupt stack?)");
-
-    case UNWIND_SAME_ID:
-      return _("previous frame identical to this frame (corrupt stack?)");
-
-    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");
diff --git a/gdb/frame.h b/gdb/frame.h
index a261a91..55972de 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -446,47 +446,16 @@ extern struct address_space *get_frame_address_space (struct frame_info *);
 
 enum unwind_stop_reason
   {
-    /* No particular reason; either we haven't tried unwinding yet,
-       or we didn't fail.  */
-    UNWIND_NO_REASON,
-
-    /* The previous frame's analyzer returns an invalid result
-       from this_id.
-
-       FIXME drow/2006-08-16: This is how GDB used to indicate end of
-       stack.  We should migrate to a model where frames always have a
-       valid ID, and this becomes not just an error but an internal
-       error.  But that's a project for another day.  */
-    UNWIND_NULL_ID,
-
-    /* This frame is the outermost.  */
-    UNWIND_OUTERMOST,
-
-    /* All the conditions after this point are considered errors;
-       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,
-
-    /* Can't unwind further, because that would require knowing the
-       values of registers or memory that haven't been collected.  */
-    UNWIND_UNAVAILABLE,
-
-    /* This frame ID looks like it ought to belong to a NEXT frame,
-       but we got it for a PREV frame.  Normally, this is a sign of
-       unwinder failure.  It could also indicate stack corruption.  */
-    UNWIND_INNER_ID,
-
-    /* This frame has the same ID as the previous one.  That means
-       that unwinding further would almost certainly give us another
-       frame with exactly the same ID, so break the chain.  Normally,
-       this is a sign of unwinder failure.  It could also indicate
-       stack corruption.  */
-    UNWIND_SAME_ID,
-
-    /* The frame unwinder didn't find any saved PC, but we needed
-       one to unwind further.  */
-    UNWIND_NO_SAVED_PC,
+#define SET(name, description) name,
+#define FIRST_ENTRY(name) UNWIND_FIRST = name,
+#define LAST_ENTRY(name) UNWIND_LAST = name,
+#define FIRST_ERROR(name) UNWIND_FIRST_ERROR = name,
+
+#include "unwind_stop_reasons.def"
+#undef SET
+#undef FIRST_ENTRY
+#undef LAST_ENTRY
+#undef FIRST_ERROR
   };
 
 /* Return the reason why we can't unwind past this frame.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 398ce84..5ed4347 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -542,7 +542,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &reason))
     return NULL;
 
-  if (reason < 0 || reason > UNWIND_NO_SAVED_PC)
+  if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     {
       PyErr_SetString (PyExc_ValueError, 
 		       _("Invalid frame stop reason."));
@@ -599,18 +599,13 @@ gdbpy_initialize_frames (void)
   PyModule_AddIntConstant (gdb_module, "SIGTRAMP_FRAME", SIGTRAMP_FRAME);
   PyModule_AddIntConstant (gdb_module, "ARCH_FRAME", ARCH_FRAME);
   PyModule_AddIntConstant (gdb_module, "SENTINEL_FRAME", SENTINEL_FRAME);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_REASON", UNWIND_NO_REASON);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NULL_ID", UNWIND_NULL_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_FIRST_ERROR", UNWIND_FIRST_ERROR);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_INNER_ID", UNWIND_INNER_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_SAME_ID", UNWIND_SAME_ID);
-  PyModule_AddIntConstant (gdb_module,
-			   "FRAME_UNWIND_NO_SAVED_PC", UNWIND_NO_SAVED_PC);
+
+#define SET(name, description) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#define FIRST_ERROR(name) \
+  PyModule_AddIntConstant (gdb_module, "FRAME_"#name, name);
+#include "unwind_stop_reasons.def"
+#undef SET
 
   Py_INCREF (&frame_object_type);
   PyModule_AddObject (gdb_module, "Frame", (PyObject *) &frame_object_type);
diff --git a/gdb/stack.c b/gdb/stack.c
index 953d3bd..003725a 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1625,7 +1625,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int from_tty)
       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));
     }
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
new file mode 100644
index 0000000..9a34feb
--- /dev/null
+++ b/gdb/unwind_stop_reasons.def
@@ -0,0 +1,80 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Reasons why frames could not be further unwound
+   SET (name, description)
+  
+   After this reason name, all reasons should be considered errors;
+   i.e.: abnormal stack termination.
+   FIRST_ERROR (name)  
+   
+   First and Last reason defined
+   FIRST_ENTRY (name)
+   LAST_ENTRY (name)  */
+
+#ifdef SET
+/* No particular reason; either we haven't tried unwinding yet, 
+   or we didn't fail.  */
+SET (UNWIND_NO_REASON, "no reason")
+
+/* The previous frame's analyzer returns an invalid result
+   from this_id.
+
+   FIXME drow/2006-08-16: This is how GDB used to indicate end of
+   stack.  We should migrate to a model where frames always have a
+   valid ID, and this becomes not just an error but an internal
+   error.  But that's a project for another day.  */
+SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
+
+/* This frame is the outermost.  */
+SET (UNWIND_OUTERMOST, "outermost")
+
+/* Can't unwind further, because that would require knowing the
+   values of registers or memory that haven't been collected.  */
+SET (UNWIND_UNAVAILABLE, \
+     "not enough registers or memory available to unwind further")
+
+/* This frame ID looks like it ought to belong to a NEXT frame,
+   but we got it for a PREV frame.  Normally, this is a sign of
+   unwinder failure.  It could also indicate stack corruption.  */
+SET (UNWIND_INNER_ID, "previous frame inner to this frame (corrupt stack?)")
+
+/* This frame has the same ID as the previous one.  That means
+   that unwinding further would almost certainly give us another
+   frame with exactly the same ID, so break the chain.  Normally,
+   this is a sign of unwinder failure.  It could also indicate
+   stack corruption.  */
+SET (UNWIND_SAME_ID, "previous frame identical to this frame (corrupt stack?)")
+
+/* The frame unwinder didn't find any saved PC, but we needed
+   one to unwind further.  */
+SET (UNWIND_NO_SAVED_PC, "frame did not save the PC")
+
+#endif /* SET */
+
+
+#ifdef FIRST_ERROR
+FIRST_ERROR (UNWIND_UNAVAILABLE)
+#endif
+
+#ifdef FIRST_ENTRY
+FIRST_ENTRY (UNWIND_NO_REASON)
+#endif
+
+#ifdef LAST_ENTRY
+LAST_ENTRY (UNWIND_NO_SAVED_PC)
+#endif
-- 
1.7.6.4

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Thu, 27 Oct 2011 11:40:50 +0200
> Cc: pedro@codesourcery.com, pmuldoon@redhat.com, gdb-patches@sourceware.org, 
> 	tromey@redhat.com
> 
> is it your last word for not mentioning the current value of the alias?

I prefer not to mention that, yes.  But if you feel strongly about it,
I won't fight you.

Thanks.

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Kevin Pouget]

On Wed, Oct 19, 2011 at 10:59 PM, Tom Tromey <tromey@redhat.com> wrote:
> ...
> The code bits are fine with one nit fixed.
>
> Kevin> +#include "../unwind_stop_reasons.def"
>
> I don't think you need ".." here.
>
> Thanks for doing this.

On Thu, Oct 27, 2011 at 12:02 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kevin Pouget <kevin.pouget@gmail.com>
>> Date: Thu, 27 Oct 2011 11:40:50 +0200
>> Cc: pedro@codesourcery.com, pmuldoon@redhat.com, gdb-patches@sourceware.org,
>>       tromey@redhat.com
>>
>> is it your last word for not mentioning the current value of the alias?
>
> I prefer not to mention that, yes.  But if you feel strongly about it,
> I won't fight you.

Not a strong feeling, so I committed the patch without it:
http://sourceware.org/ml/gdb-cvs/2011-10/msg00195.html

Thanks,

Kevin

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Pedro Alves]

On Wednesday 12 October 2011 16:17:32, Kevin Pouget wrote:
> On Wed, Oct 12, 2011 at 4:52 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > 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.

> yes, makes perfectly sense, thanks
> 
> the numbers come from there:
> http://sourceware.org/gdb/onlinedocs/gdb/Frames-In-Python.html#index-unwind_005fstop_005freason-on-Frame-1870

Yeah, that much I had found, but I got stuck looking for the enum
versions of those.

> ... I don't know if changing these numbers would be considered as a
> backward incompatibility ... ?

Sigh, I hope not.

> From: Phil Muldoon <pmuldoon@redhat.com>
> > Why not use the supplied gdb constants in this case?  "2" does not map
> > to any enum.
> > IE gdb.FRAME_UNWIND_NO_REASON
> 
> it looks like these enums are not documented, are they? I can't grep
> 'FRAME_UNWIND_NO_REASON' in gdb.texinfo

Yeah.

> 
> ('2' was a bad example, but the reason why I first used '0' was
> because it was returned by Frame.unwind_stop_reason(), as depicted in
> the first mail)
> by the way, python print
> gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_FIRST_ERROR) crashes the
> same way, there is certainly a few more lines to fix on the Python
> side

The patch I posted supposedly fixes that, my making
FRAME_UNWIND_FIRST_ERROR an internal alias for a real
value (..._UNAVAILABLE), so that there are no "holes" in
the enum values.

-- 
Pedro Alves

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

>> Why not use the supplied gdb constants in this case?  "2" does not map
>> to any enum.
>> IE gdb.FRAME_UNWIND_NO_REASON

Kevin> it looks like these enums are not documented, are they? I can't grep
Kevin> 'FRAME_UNWIND_NO_REASON' in gdb.texinfo

Yeah, we should document those.
I filed: http://sourceware.org/bugzilla/show_bug.cgi?id=13285

Tom

Re: [RFC][Python] gdbpy_frame_stop_reason_string bug

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

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

Users must use the named constants provided.
We should not guarantee that their values will remain the same.

Your patch looks reasonable to me.

Tom