[PATCH 0/3] frame filter fixes, proposed for 7.7

[PATCH 0/3] frame filter fixes, proposed for 7.7

[Tom Tromey]

While working on a frame filter, I found a few bugs in gdb's frame
filter code.  This series fixes the worst of the bugs, basically the
crashers I ran across.

I would like these patches to go on the branch for the 7.7 release.
Frame filters are a new feature in 7.7 and it would be disappointing
to release gdb with them not quite working.  Furthermore I think the
patches are straightforward enough that this should not be of concern.

Tom

[PATCH 1/3] fix crash in frame filters

[Tom Tromey]

apply_frame_filter calls ensure_python_env before computing the
gdbarch to use.  This means that python_gdbarch can be NULL while in
Python code, and if a frame filter depends on this somehow (easy to
do), gdb will crash.

The fix is to compute the gdbarch first.

Built and regtested on x86-64 Fedora 18.
New test case included.

2014-01-22  Tom Tromey  <tromey@redhat.com>

	PR python/16491:
	* python/py-framefilter.c (apply_frame_filter): Call
	ensure_python_env after computing gdbarch.

2014-01-22  Tom Tromey  <tromey@redhat.com>

	PR python/16491:
	* gdb.python/py-framefilter.py (Reverse_Function.function): Read a
	string from an inferior frame.
	* gdb.python/py-framefilter-mi.exp: Update.
---
 gdb/ChangeLog                                  | 6 ++++++
 gdb/python/py-framefilter.c                    | 8 ++++----
 gdb/testsuite/ChangeLog                        | 7 +++++++
 gdb/testsuite/gdb.python/py-framefilter-mi.exp | 4 ++--
 gdb/testsuite/gdb.python/py-framefilter.py     | 5 ++++-
 5 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 76ce4e5..c6a29ef 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1477,18 +1477,18 @@ apply_frame_filter (struct frame_info *frame, int flags,
   if (!gdb_python_initialized)
     return PY_BT_NO_FILTERS;
 
-  cleanups = ensure_python_env (gdbarch, current_language);
-
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
       gdbarch = get_frame_arch (frame);
     }
   if (except.reason < 0)
     {
-      gdbpy_convert_exception (except);
-      goto error;
+      /* Let gdb try to print the stack trace.  */
+      return PY_BT_NO_FILTERS;
     }
 
+  cleanups = ensure_python_env (gdbarch, current_language);
+
   iterable = bootstrap_python_frame_filters (frame, frame_low, frame_high);
 
   if (iterable == NULL)
diff --git a/gdb/testsuite/gdb.python/py-framefilter-mi.exp b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
index 58c8d35..0c5f5d4 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-mi.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
@@ -66,10 +66,10 @@ mi_continue_to_line [gdb_get_line_number {Backtrace end breakpoint} ${srcfile}]
   "step to breakpoint"
 
 mi_gdb_test "-stack-list-frames" \
-    "\\^done,stack=\\\[frame={level=\"0\",addr=\"$hex\",func=\"cnuf_dne\".*},frame={level=\"1\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"2\",addr=\"$hex\",func=\"bcnuf\".*},frame={level=\"3\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"22\",addr=\"$hex\",func=\"1cnuf\".*,children=\\\[frame={level=\"23\",addr=\"$hex\",func=\"func2\".*}\\\]},frame={level=\"24\",addr=\"$hex\",func=\"3cnuf\".*},frame={level=\"27\",addr=\"$hex\",func=\"niam\".*}\\\].*" \
+    "\\^done,stack=\\\[frame={level=\"0\",addr=\"$hex\",func=\"cnuf_dne.*\".*},frame={level=\"1\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"2\",addr=\"$hex\",func=\"bcnuf\".*},frame={level=\"3\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"22\",addr=\"$hex\",func=\"1cnuf\".*,children=\\\[frame={level=\"23\",addr=\"$hex\",func=\"func2\".*}\\\]},frame={level=\"24\",addr=\"$hex\",func=\"3cnuf\".*},frame={level=\"27\",addr=\"$hex\",func=\"niam\".*}\\\].*" \
     "filtered stack listing"
 mi_gdb_test "-stack-list-frames 0 3" \
-    "\\^done,stack=\\\[frame={level=\"0\",addr=\"$hex\",func=\"cnuf_dne\".*},frame={level=\"1\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"2\",addr=\"$hex\",func=\"bcnuf\".*},frame={level=\"3\",addr=\"$hex\",func=\"acnuf\".*}\\\]" \
+    "\\^done,stack=\\\[frame={level=\"0\",addr=\"$hex\",func=\"cnuf_dne.*\".*},frame={level=\"1\",addr=\"$hex\",func=\"acnuf\".*},frame={level=\"2\",addr=\"$hex\",func=\"bcnuf\".*},frame={level=\"3\",addr=\"$hex\",func=\"acnuf\".*}\\\]" \
     "filtered stack list 0 3"
 mi_gdb_test "-stack-list-frames 22 24" \
     "\\^done,stack=\\\[frame={level=\"22\",addr=\"$hex\",func=\"1cnuf\".*,children=\\\[frame={level=\"23\",addr=\"$hex\",func=\"func2\".*}\\\]},frame={level=\"24\",addr=\"$hex\",func=\"3cnuf\".*}\\\]" \
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index 1cf8c22..25b3368 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -30,8 +30,11 @@ class Reverse_Function (FrameDecorator):
         fname = str (self.fobj.function())
         if (fname == None or fname == ""):
             return None
+        if fname == 'end_func':
+            extra = self.fobj.inferior_frame().read_var('str').string()
         else:
-            fname = fname[::-1]
+            extra = ''
+        fname = fname[::-1] + extra
         return fname
 
 class Dummy (FrameDecorator):
-- 
1.8.1.4

Re: [PATCH 1/3] fix crash in frame filters

[Phil Muldoon]

On Wed, 22 Jan 2014, Tom Tromey wrote:

> apply_frame_filter calls ensure_python_env before computing the
> gdbarch to use.  This means that python_gdbarch can be NULL while in
> Python code, and if a frame filter depends on this somehow (easy to
> do), gdb will crash.
> 
> The fix is to compute the gdbarch first.
> 
> Built and regtested on x86-64 Fedora 18.
> New test case included.
> 
> 2014-01-22  Tom Tromey  <tromey@redhat.com>
> 
> 	PR python/16491:
> 	* python/py-framefilter.c (apply_frame_filter): Call
> 	ensure_python_env after computing gdbarch.
> 
> 2014-01-22  Tom Tromey  <tromey@redhat.com>
> 
> 	PR python/16491:
> 	* gdb.python/py-framefilter.py (Reverse_Function.function): Read a
> 	string from an inferior frame.
> 	* gdb.python/py-framefilter-mi.exp: Update.

These are all fine, and thanks for fixing them.  I think they should
proceed into 7.7 too.  Especially this one:

> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index 76ce4e5..c6a29ef 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -1477,18 +1477,18 @@ apply_frame_filter (struct frame_info *frame, int flags,
>    if (!gdb_python_initialized)
>      return PY_BT_NO_FILTERS;
>  
> -  cleanups = ensure_python_env (gdbarch, current_language);
> -
>    TRY_CATCH (except, RETURN_MASK_ALL)
>      {
>        gdbarch = get_frame_arch (frame);
>      }
>    if (except.reason < 0)
>      {
> -      gdbpy_convert_exception (except);
> -      goto error;
> +      /* Let gdb try to print the stack trace.  */
> +      return PY_BT_NO_FILTERS;
>      }
>  
> +  cleanups = ensure_python_env (gdbarch, current_language);
> +

Ugh ugly. Ssing gdbarch when it is NULL before querying the frame for an
architecture is especially clumsy.  :( mea culpa.

Cheers,

Phil

Re: [PATCH 1/3] fix crash in frame filters

[Joel Brobecker]

> These are all fine, and thanks for fixing them.  I think they should
> proceed into 7.7 too.  Especially this one:

Same here, OK for 7.7.

-- 
Joel

[PATCH 3/3] avoid python exception in FrameDecorator.py

[Tom Tromey]

This fixes a bug in FrameDecorator.py.

FrameVars seems to assume that Frame.block can return None if there is
no block.  However, it actually throws an exception.

I saw this bug while developing a frame filter, but unfortunately I
don't know how to reproduce it.  It seems to me that the SAL tests in
_is_limited_frame should exclude the bad cases; and in my attempts to
write a test they do.

Nevertheless I think the fix is reasonably obvious and ought to go in.

2014-01-22  Tom Tromey  <tromey@redhat.com>

	PR python/16485:
	* python/lib/gdb/FrameDecorator.py: (FrameVars.fetch_frame_args):
	Handle exception from frame.block.
	(FrameVars.fetch_frame_locals): Likewise.
---
 gdb/ChangeLog                        |  7 +++++++
 gdb/python/lib/gdb/FrameDecorator.py | 12 ++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index 1b8b4ed..1bbc5ab 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -258,7 +258,10 @@ class FrameVars(object):
         are no frame local variables, return an empty list."""
         lvars = []
 
-        block = self.frame.block()
+        try:
+            block = self.frame.block()
+        except RuntimeError:
+            block = None
 
         while block != None:
             if block.is_global or block.is_static:
@@ -279,7 +282,12 @@ class FrameVars(object):
         there are no frame argument variables, return an empty list."""
 
         args = []
-        block = self.frame.block()
+
+        try:
+            block = self.frame.block()
+        except RuntimeError:
+            block = None
+
         while block != None:
             if block.function != None:
                 break
-- 
1.8.1.4

[PATCH 2/3] fix erroneous error-handling in frame filter code

[Tom Tromey]

This fixes PR python/16487.

The bug here is that the function-name-handling code in py_print_frame
had a small logic error (really a misplaced closing brace).  This
error could lead to a Py_DECREF(NULL), which crashes.

This patch fixes the bug in the obvious way.

Built and regtested on x86-64 Fedora 18.  New test case included.

2014-01-22  Tom Tromey  <tromey@redhat.com>

	PR python/16487:
	* python/py-framefilter.c (py_print_frame): Don't call Py_DECREF
	on a NULL pointer.  Move "goto error" to correct place.

2014-01-22  Tom Tromey  <tromey@redhat.com>

	PR python/16487:
	* gdb.python/py-framefilter.exp: Add test using "Error" filter.
	* gdb.python/py-framefilter.py (ErrorInName, ErrorFilter): New
	classes.
---
 gdb/ChangeLog                               |  6 ++++++
 gdb/python/py-framefilter.c                 |  6 +++---
 gdb/testsuite/ChangeLog                     |  7 +++++++
 gdb/testsuite/gdb.python/py-framefilter.exp | 11 +++++++++++
 gdb/testsuite/gdb.python/py-framefilter.py  | 20 ++++++++++++++++++++
 5 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index c6a29ef..1a9f3e0 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1218,11 +1218,11 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 		  gdbpy_convert_exception (except);
 		  goto error;
 		}
+	      Py_DECREF (py_func);
 	    }
-	  Py_DECREF (py_func);
+	  else
+	    goto error;
 	}
-      else
-	goto error;
     }
 
 
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 106240b..905ad8c 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -177,6 +177,17 @@ gdb_test "bt 1" \
     "#0  end_func \\(foo=21, bar=\"Param\", fb=, bf=\{nothing = \"Foo Bar\", f = 42, s = 19\}\\) at .*py-framefilter.c.*" \
     "bt 1 no addresss"
 
+gdb_test_no_output "set python print-stack message" \
+    "Set python print-stack to message for Error filter"
+gdb_test_no_output  "enable frame-filter global Error" \
+    "enable frame-filter global Error"
+set test "bt 1 with Error filter"
+gdb_test_multiple "bt 1" $test {
+    -re "Python Exception .*whoops:.*$gdb_prompt $" {
+	pass $test
+    }
+}
+
 remote_file host delete ${remote_python_file}
 
 # Test with no debuginfo
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index 25b3368..a085688 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -128,5 +128,25 @@ class FrameElider ():
     def filter (self, frame_iter):
         return ElidingIterator (frame_iter)
 
+# A simple decorator that gives an error when computing the function.
+class ErrorInName(FrameDecorator):
+    def __init__(self, frame):
+        FrameDecorator.__init__(self, frame)
+
+    def function(self):
+        raise RuntimeError('whoops')
+
+# A filter that supplies buggy frames.  Disabled by default.
+class ErrorFilter():
+    def __init__ (self):
+        self.name = "Error"
+        self.priority = 1
+        self.enabled = False
+        gdb.frame_filters [self.name] = self
+
+    def filter(self, frame_iter):
+        return itertools.imap(ErrorInName, frame_iter)
+
 FrameFilter()
 FrameElider()
+ErrorFilter()
-- 
1.8.1.4