Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: gdb-patches@sourceware.org
Cc: Kevin Pouget <kevin.pouget@gmail.com>
Subject: Re: [RFC][Python] gdbpy_frame_stop_reason_string bug
Date: Wed, 12 Oct 2011 14:52:00 -0000	[thread overview]
Message-ID: <201110121552.29096.pedro@codesourcery.com> (raw)
In-Reply-To: <CAPftXULjbKonB=-YneTVg+jeZiUDUruqtjrTighL8dgXTWoo_g@mail.gmail.com>

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

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

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

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

-- 
Pedro Alves

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

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


  parent reply	other threads:[~2011-10-12 14:52 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-12 14:02 Kevin Pouget
2011-10-12 14:33 ` Phil Muldoon
2011-10-12 14:52 ` Pedro Alves [this message]
2011-10-12 15:18   ` Kevin Pouget
2011-10-12 15:28     ` Phil Muldoon
2011-10-12 15:59       ` Pedro Alves
2011-10-12 17:07         ` Tom Tromey
2011-10-13 11:25           ` Kevin Pouget
2011-10-13 11:27             ` Kevin Pouget
2011-10-13 15:19               ` Pedro Alves
2011-10-14  8:18                 ` Kevin Pouget
2011-10-14 14:22                   ` Tom Tromey
2011-10-14 14:41                     ` Kevin Pouget
2011-10-14 15:00                       ` Pedro Alves
2011-10-17 10:31                         ` Kevin Pouget
2011-10-19 21:22                           ` Tom Tromey
2011-10-24 16:53                             ` Kevin Pouget
2011-10-25  0:59                               ` Eli Zaretskii
2011-10-25  8:31                                 ` Kevin Pouget
2011-10-25 12:49                                   ` Eli Zaretskii
2011-10-25 14:27                                     ` Kevin Pouget
2011-10-27 10:02                                       ` Kevin Pouget
2011-10-27 10:16                                         ` Eli Zaretskii
2011-10-27 12:11                                           ` Kevin Pouget
2011-10-12 15:31     ` Pedro Alves
2011-10-12 17:06     ` Tom Tromey
2011-10-12 17:00   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201110121552.29096.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=kevin.pouget@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox