[RFA 0/3] unwinder registration and the frame cache

[RFA 0/3] unwinder registration and the frame cache

[Tom Tromey]

This series fixes a couple of related bugs: when registering or
unregistering an unwinder, it makes sense to flush the frame cache.
This way, the changes to the unwinder take effect immediately.

I built and regtested this series on x86-64 Fedora 23.

There's one random patch in the middle that is just something I
noticed when working on this.

Tom

[RFA 1/3] Clear frame cache when loading or unloading a JIT unwinder

[Tom Tromey]

It seemed to me that gdb should flush the frame cache when loading or
unloading a JIT unwinder.  This makes testing a JIT unwinder a bit
simpler.

jit-reader-load apparently has no tests in-tree, so I didn't add a new
test here.

2016-06-09  Tom Tromey  <tom@tromey.com>

	* jit.c (jit_reader_load_command, jit_reader_unload_command): Call
	reinit_frame_cache.
---
 gdb/ChangeLog | 5 +++++
 gdb/jit.c     | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 335476b..04609c0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-06-09  Tom Tromey  <tom@tromey.com>
+
+	* jit.c (jit_reader_load_command, jit_reader_unload_command): Call
+	reinit_frame_cache.
+
 2016-06-10  Tom Tromey  <tom@tromey.com>
 
 	* gdbtypes.c (arch_type, arch_integer_type, arch_character_type)
diff --git a/gdb/jit.c b/gdb/jit.c
index 9fd5ae6..9bb2da5 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -218,6 +218,7 @@ jit_reader_load_command (char *args, int from_tty)
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
+  reinit_frame_cache ();
   do_cleanups (prev_cleanup);
 }
 
@@ -234,6 +235,7 @@ jit_reader_unload_command (char *args, int from_tty)
   gdb_dlclose (loaded_jit_reader->handle);
   xfree (loaded_jit_reader);
   loaded_jit_reader = NULL;
+  reinit_frame_cache ();
 }
 
 /* Per-program space structure recording which objfile has the JIT
-- 
2.5.5

[RFA 3/3] PR python/19293 - invalidate frame cache when unwinders change

[Tom Tromey]

PR python/19293 notes that when a Python unwinder is disabled, the
frame cache is not invalidated.  This means that disabling an unwinder
doesn't have any immediate effect -- but in my experience it's often
the case that I want to enable or disable an unwinder in order to see
what happens.

This patch adds a new gdb.invalidate_cached_frames function and
arranges for the relevant bits of library code to call it.  I've only
partially documented this function, considering a warning sufficient
without going into all the reasons ordinary code should not call it.
The name of the new function was taken from a comment in frame.h next
to reinit_frame_cache.

No new test as I think the updates to the existing test are sufficient
to show that the code is working as intended.

Built and regtested on x86-64 Fedora 23.

2016-06-09  Tom Tromey  <tom@tromey.com>

	PR python/19293:
	* python/lib/gdb/command/unwinders.py (do_enable_unwinder): Call
	gdb.invalidate_cached_frames.
	* python/lib/gdb/unwinder.py (register_unwinder): Call
	gdb.invalidate_cached_frames.
	* python/python.c (gdbpy_invalidate_cached_frames): New function.
	(python_GdbMethods): Add entry for invalidate_cached_frames.

2016-06-09  Tom Tromey  <tom@tromey.com>

	PR python/19293:
	* python.texi (Frames In Python): Document
	gdb.invalidate_cached_frames.

2016-06-10  Tom Tromey  <tom@tromey.com>

	PR python/19293:
	* gdb.python/py-unwind-maint.exp: Update tests.
---
 gdb/ChangeLog                                | 10 ++++++++++
 gdb/doc/ChangeLog                            |  6 ++++++
 gdb/doc/python.texi                          |  9 +++++++++
 gdb/python/lib/gdb/command/unwinders.py      |  2 ++
 gdb/python/lib/gdb/unwinder.py               |  1 +
 gdb/python/python.c                          | 13 +++++++++++++
 gdb/testsuite/ChangeLog                      |  5 +++++
 gdb/testsuite/gdb.python/py-unwind-maint.exp | 11 +++++++++--
 8 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a64977e..58e9c66 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2016-06-09  Tom Tromey  <tom@tromey.com>
 
+	PR python/19293:
+	* python/lib/gdb/command/unwinders.py (do_enable_unwinder): Call
+	gdb.invalidate_cached_frames.
+	* python/lib/gdb/unwinder.py (register_unwinder): Call
+	gdb.invalidate_cached_frames.
+	* python/python.c (gdbpy_invalidate_cached_frames): New function.
+	(python_GdbMethods): Add entry for invalidate_cached_frames.
+
+2016-06-09  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (gdbpy_parameter): Now static.
 	* python/python-internal.h (gdbpy_parameter): Don't declare.
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index a01d545..a010562 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-09  Tom Tromey  <tom@tromey.com>
+
+	PR python/19293:
+	* python.texi (Frames In Python): Document
+	gdb.invalidate_cached_frames.
+
 2016-06-09  Toshihito Kikuchi  <k.toshihito@yahoo.de>
 
 	* gdb.texinfo (Examining Memory): Document negative repeat
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 6623d8e..7682940 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3838,6 +3838,15 @@ frames, as expressed by the given @var{reason} code (an integer, see the
 @code{unwind_stop_reason} method further down in this section).
 @end defun
 
+@findex gdb.invalidate_cached_frames
+@defun gdb.invalidate_cached_frames
+@value{GDBN} internally keeps a cache of the frames that have been
+unwound.  This function causes invalidates this cache.
+
+This function should not generally be called by ordinary Python code.
+It is documented for the sake of completeness.
+@end defun
+
 A @code{gdb.Frame} object has the following methods:
 
 @defun Frame.is_valid ()
diff --git a/gdb/python/lib/gdb/command/unwinders.py b/gdb/python/lib/gdb/command/unwinders.py
index a9b9d8a..8fd0136 100644
--- a/gdb/python/lib/gdb/command/unwinders.py
+++ b/gdb/python/lib/gdb/command/unwinders.py
@@ -136,6 +136,8 @@ def do_enable_unwinder(arg, flag):
         if locus_re.match(objfile.filename):
             total += do_enable_unwinder1(objfile.frame_unwinders, name_re,
                                          flag)
+    if total > 0:
+        gdb.invalidate_cached_frames()
     print("%d unwinder%s %s" % (total, "" if total == 1 else "s",
                                 "enabled" if flag else "disabled"))
 
diff --git a/gdb/python/lib/gdb/unwinder.py b/gdb/python/lib/gdb/unwinder.py
index 14b2758..67a37cb 100644
--- a/gdb/python/lib/gdb/unwinder.py
+++ b/gdb/python/lib/gdb/unwinder.py
@@ -92,3 +92,4 @@ def register_unwinder(locus, unwinder, replace=False):
                                    unwinder.name)
         i += 1
     locus.frame_unwinders.insert(0, unwinder)
+    gdb.invalidate_cached_frames()
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9c4ddda..62c9bde 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -884,6 +884,13 @@ gdbpy_find_pc_line (PyObject *self, PyObject *args)
   return result;
 }
 
+static PyObject *
+gdbpy_invalidate_cached_frames (PyObject *self, PyObject *args)
+{
+  reinit_frame_cache ();
+  Py_RETURN_NONE;
+}
+
 /* Read a file as Python code.
    This is the extension_language_script_ops.script_sourcer "method".
    FILE is the file to load.  FILENAME is name of the file FILE.
@@ -2070,6 +2077,12 @@ Return the selected inferior object." },
   { "inferiors", gdbpy_inferiors, METH_NOARGS,
     "inferiors () -> (gdb.Inferior, ...).\n\
 Return a tuple containing all inferiors." },
+
+  { "invalidate_cached_frames", gdbpy_invalidate_cached_frames, METH_NOARGS,
+    "invalidate_cached_frames () -> None.\n\
+Invalidate any cached frame objects in gdb.\n\
+Intended for internal use only." },
+
   {NULL, NULL, 0, NULL}
 };
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7d2b144..bc46282 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-10  Tom Tromey  <tom@tromey.com>
 
+	PR python/19293:
+	* gdb.python/py-unwind-maint.exp: Update tests.
+
+2016-06-10  Tom Tromey  <tom@tromey.com>
+
 	PR rust/20110:
 	* gdb.rust/expr.exp: Add test for integer constant larger than
 	i32.
diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.exp b/gdb/testsuite/gdb.python/py-unwind-maint.exp
index e89d284..3a98cb1 100644
--- a/gdb/testsuite/gdb.python/py-unwind-maint.exp
+++ b/gdb/testsuite/gdb.python/py-unwind-maint.exp
@@ -34,7 +34,11 @@ if ![runto_main ] then {
     return -1
 }
 
-gdb_test "source ${pyfile}" "Python script imported" "import python scripts"
+gdb_test_sequence "source ${pyfile}" "import python scripts" {
+    "Python script imported"
+    "py_unwind_maint_ps_unwinder called"
+    "global_unwinder called"
+}
 
 gdb_test_sequence "info unwinder" "Show all unwinders" {
     "Global:"
@@ -50,7 +54,10 @@ gdb_test_sequence "continue" "Unwinders called" {
     "global_unwinder called"
 }
 
-gdb_test "disable unwinder global .*" "1 unwinder disabled" "Unwinder disabled"
+gdb_test_sequence "disable unwinder global .*" "Unwinder disabled" {
+    "1 unwinder disabled"
+    "py_unwind_maint_ps_unwinder called"
+}
 
 gdb_test_sequence "info unwinder" "Show with global unwinder disabled" {
     "Global:"
-- 
2.5.5

[RFA 2/3] Make gdbpy_parameter static

[Tom Tromey]

While working on the next patch in this series, I noticed that
gdbpy_parameter did not need to be exported.  This makes it "static".

2016-06-09  Tom Tromey  <tom@tromey.com>

	* python/python.c (gdbpy_parameter): Now static.
	* python/python-internal.h (gdbpy_parameter): Don't declare.
---
 gdb/ChangeLog                | 5 +++++
 gdb/python/python-internal.h | 1 -
 gdb/python/python.c          | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 04609c0..a64977e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-09  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (gdbpy_parameter): Now static.
+	* python/python-internal.h (gdbpy_parameter): Don't declare.
+
+2016-06-09  Tom Tromey  <tom@tromey.com>
+
 	* jit.c (jit_reader_load_command, jit_reader_unload_command): Call
 	reinit_frame_cache.
 
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 6a2619c..7eb9aa4 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -375,7 +375,6 @@ PyObject *gdbpy_create_ptid_object (ptid_t ptid);
 PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args);
 PyObject *gdbpy_selected_inferior (PyObject *self, PyObject *args);
 PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
-PyObject *gdbpy_parameter (PyObject *self, PyObject *args);
 PyObject *gdbpy_parameter_value (enum var_types type, void *var);
 char *gdbpy_parse_command_name (const char *name,
 				struct cmd_list_element ***base_list,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1f1fece..9c4ddda 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -553,7 +553,7 @@ gdbpy_parameter_value (enum var_types type, void *var)
 /* A Python function which returns a gdb parameter's value as a Python
    value.  */
 
-PyObject *
+static PyObject *
 gdbpy_parameter (PyObject *self, PyObject *args)
 {
   struct gdb_exception except = exception_none;
-- 
2.5.5

Re: [RFA 1/3] Clear frame cache when loading or unloading a JIT unwinder

[Pedro Alves]

On 06/11/2016 03:42 AM, Tom Tromey wrote:
> It seemed to me that gdb should flush the frame cache when loading or
> unloading a JIT unwinder.  This makes testing a JIT unwinder a bit
> simpler.
> 
> jit-reader-load apparently has no tests in-tree, so I didn't add a new
> test here.

Yeah.  Tests existed, but had never been pushed in the tree...  I fixed
that now:

  https://sourceware.org/ml/gdb-patches/2016-06/msg00308.html

Could you see if it's easy to extend the test to cover this?

Thanks,
Pedro Alves

Re: [RFA 2/3] Make gdbpy_parameter static

[Pedro Alves]

On 06/11/2016 03:42 AM, Tom Tromey wrote:
> While working on the next patch in this series, I noticed that
> gdbpy_parameter did not need to be exported.  This makes it "static".
> 
> 2016-06-09  Tom Tromey  <tom@tromey.com>
> 
> 	* python/python.c (gdbpy_parameter): Now static.
> 	* python/python-internal.h (gdbpy_parameter): Don't declare.

This is obvious, IMO.

Thanks,
Pedro Alves

Re: [RFA 3/3] PR python/19293 - invalidate frame cache when unwinders change

[Pedro Alves]

On 06/11/2016 03:42 AM, Tom Tromey wrote:
> PR python/19293 notes that when a Python unwinder is disabled, the
> frame cache is not invalidated.  This means that disabling an unwinder
> doesn't have any immediate effect -- but in my experience it's often
> the case that I want to enable or disable an unwinder in order to see
> what happens.
> 
> This patch adds a new gdb.invalidate_cached_frames function and
> arranges for the relevant bits of library code to call it.  I've only
> partially documented this function, considering a warning sufficient
> without going into all the reasons ordinary code should not call it.
> The name of the new function was taken from a comment in frame.h next
> to reinit_frame_cache.

Seems reasonable to me.  Couple nits below.

> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3838,6 +3838,15 @@ frames, as expressed by the given @var{reason} code (an integer, see the
>  @code{unwind_stop_reason} method further down in this section).
>  @end defun
>  
> +@findex gdb.invalidate_cached_frames
> +@defun gdb.invalidate_cached_frames
> +@value{GDBN} internally keeps a cache of the frames that have been
> +unwound.  This function causes invalidates this cache.

s/causes// ?

> +
> +This function should not generally be called by ordinary Python code.
> +It is documented for the sake of completeness.
> +@end defun


> diff --git a/gdb/python/lib/gdb/unwinder.py b/gdb/python/lib/gdb/unwinder.py
> index 14b2758..67a37cb 100644
> --- a/gdb/python/lib/gdb/unwinder.py
> +++ b/gdb/python/lib/gdb/unwinder.py
> @@ -92,3 +92,4 @@ def register_unwinder(locus, unwinder, replace=False):
>                                     unwinder.name)
>          i += 1
>      locus.frame_unwinders.insert(0, unwinder)
> +    gdb.invalidate_cached_frames()
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 9c4ddda..62c9bde 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -884,6 +884,13 @@ gdbpy_find_pc_line (PyObject *self, PyObject *args)
>    return result;
>  }
>  
> +static PyObject *
> +gdbpy_invalidate_cached_frames (PyObject *self, PyObject *args)

Intro comment?

Thanks,
Pedro Alves

Re: [RFA 1/3] Clear frame cache when loading or unloading a JIT unwinder

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Tom> jit-reader-load apparently has no tests in-tree, so I didn't add a new
Tom> test here.

Pedro> Yeah.  Tests existed, but had never been pushed in the tree...  I fixed
Pedro> that now:
Pedro>   https://sourceware.org/ml/gdb-patches/2016-06/msg00308.html
Pedro> Could you see if it's easy to extend the test to cover this?

I looked and I didn't see a straightforward way.

The basic problem is that flushing the frame cache doesn't affect the
results in the jit-reader.exp test.  In this test, the test case can be
unwound without assistance; JIT reader in question supplies some
symbols.  Clearing the frame cache doesn't affect this.

FWIW I don't care all that much about this patch.  It seemed like an
oversight to me, but I'm more concerned with getting the Python case
right, since in the end that's the sort of unwinder I wrote.

Tom

Re: [RFA 2/3] Make gdbpy_parameter static

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> 2016-06-09  Tom Tromey  <tom@tromey.com>
>> 
>> * python/python.c (gdbpy_parameter): Now static.
>> * python/python-internal.h (gdbpy_parameter): Don't declare.

Pedro> This is obvious, IMO.

Thanks, I'm pushing this one in.
It's independent of the other patches here.

Tom

Re: [RFA 3/3] PR python/19293 - invalidate frame cache when unwinders change

[Tom Tromey]

Pedro> Seems reasonable to me.  Couple nits below.

Here's the updated patch.
This still needs doc review.

Tom

commit dd5cd89e2345325da2361a3a72a11420de01e802
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Jun 9 15:20:09 2016 -0600

    PR python/19293 - invalidate frame cache when unwinders change
    
    PR python/19293 notes that when a Python unwinder is disabled, the
    frame cache is not invalidated.  This means that disabling an unwinder
    doesn't have any immediate effect -- but in my experience it's often
    the case that I want to enable or disable an unwinder in order to see
    what happens.
    
    This patch adds a new gdb.invalidate_cached_frames function and
    arranges for the relevant bits of library code to call it.  I've only
    partially documented this function, considering a warning sufficient
    without going into all the reasons ordinary code should not call it.
    The name of the new function was taken from a comment in frame.h next
    to reinit_frame_cache.
    
    No new test as I think the updates to the existing test are sufficient
    to show that the code is working as intended.
    
    Built and regtested on x86-64 Fedora 23.
    
    2016-06-23  Tom Tromey  <tom@tromey.com>
    
    	PR python/19293:
    	* python/lib/gdb/command/unwinders.py (do_enable_unwinder): Call
    	gdb.invalidate_cached_frames.
    	* python/lib/gdb/unwinder.py (register_unwinder): Call
    	gdb.invalidate_cached_frames.
    	* python/python.c (gdbpy_invalidate_cached_frames): New function.
    	(python_GdbMethods): Add entry for invalidate_cached_frames.
    
    2016-06-23  Tom Tromey  <tom@tromey.com>
    
    	PR python/19293:
    	* python.texi (Frames In Python): Document
    	gdb.invalidate_cached_frames.
    
    2016-06-23  Tom Tromey  <tom@tromey.com>
    
    	PR python/19293:
    	* gdb.python/py-unwind-maint.exp: Update tests.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1ce5757..ee356bf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2016-06-23  Tom Tromey  <tom@tromey.com>
+
+	PR python/19293:
+	* python/lib/gdb/command/unwinders.py (do_enable_unwinder): Call
+	gdb.invalidate_cached_frames.
+	* python/lib/gdb/unwinder.py (register_unwinder): Call
+	gdb.invalidate_cached_frames.
+	* python/python.c (gdbpy_invalidate_cached_frames): New function.
+	(python_GdbMethods): Add entry for invalidate_cached_frames.
+
 2016-06-09  Tom Tromey  <tom@tromey.com>
 
 	* jit.c (jit_reader_load_command, jit_reader_unload_command): Call
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 3186ff2..3d051d8 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-23  Tom Tromey  <tom@tromey.com>
+
+	PR python/19293:
+	* python.texi (Frames In Python): Document
+	gdb.invalidate_cached_frames.
+
 2016-06-21  Pedro Alves  <palves@redhat.com>
 
 	* gdb.texinfo (Interpreters): Update intepreter-exec section,
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 6623d8e..f218ad6 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3838,6 +3838,15 @@ frames, as expressed by the given @var{reason} code (an integer, see the
 @code{unwind_stop_reason} method further down in this section).
 @end defun
 
+@findex gdb.invalidate_cached_frames
+@defun gdb.invalidate_cached_frames
+@value{GDBN} internally keeps a cache of the frames that have been
+unwound.  This function invalidates this cache.
+
+This function should not generally be called by ordinary Python code.
+It is documented for the sake of completeness.
+@end defun
+
 A @code{gdb.Frame} object has the following methods:
 
 @defun Frame.is_valid ()
diff --git a/gdb/python/lib/gdb/command/unwinders.py b/gdb/python/lib/gdb/command/unwinders.py
index a9b9d8a..8fd0136 100644
--- a/gdb/python/lib/gdb/command/unwinders.py
+++ b/gdb/python/lib/gdb/command/unwinders.py
@@ -136,6 +136,8 @@ def do_enable_unwinder(arg, flag):
         if locus_re.match(objfile.filename):
             total += do_enable_unwinder1(objfile.frame_unwinders, name_re,
                                          flag)
+    if total > 0:
+        gdb.invalidate_cached_frames()
     print("%d unwinder%s %s" % (total, "" if total == 1 else "s",
                                 "enabled" if flag else "disabled"))
 
diff --git a/gdb/python/lib/gdb/unwinder.py b/gdb/python/lib/gdb/unwinder.py
index 14b2758..67a37cb 100644
--- a/gdb/python/lib/gdb/unwinder.py
+++ b/gdb/python/lib/gdb/unwinder.py
@@ -92,3 +92,4 @@ def register_unwinder(locus, unwinder, replace=False):
                                    unwinder.name)
         i += 1
     locus.frame_unwinders.insert(0, unwinder)
+    gdb.invalidate_cached_frames()
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 3a272a9..621e201 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -885,6 +885,15 @@ gdbpy_find_pc_line (PyObject *self, PyObject *args)
   return result;
 }
 
+/* Implementation of gdb.invalidate_cached_frames.  */
+
+static PyObject *
+gdbpy_invalidate_cached_frames (PyObject *self, PyObject *args)
+{
+  reinit_frame_cache ();
+  Py_RETURN_NONE;
+}
+
 /* Read a file as Python code.
    This is the extension_language_script_ops.script_sourcer "method".
    FILE is the file to load.  FILENAME is name of the file FILE.
@@ -2071,6 +2080,12 @@ Return the selected inferior object." },
   { "inferiors", gdbpy_inferiors, METH_NOARGS,
     "inferiors () -> (gdb.Inferior, ...).\n\
 Return a tuple containing all inferiors." },
+
+  { "invalidate_cached_frames", gdbpy_invalidate_cached_frames, METH_NOARGS,
+    "invalidate_cached_frames () -> None.\n\
+Invalidate any cached frame objects in gdb.\n\
+Intended for internal use only." },
+
   {NULL, NULL, 0, NULL}
 };
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 8b5274d..ad11415 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-23  Tom Tromey  <tom@tromey.com>
 
+	PR python/19293:
+	* gdb.python/py-unwind-maint.exp: Update tests.
+
+2016-06-23  Tom Tromey  <tom@tromey.com>
+
 	PR gdb/16483:
 	* gdb.python/py-framefilter.exp: Add "info frame-filter" test
 	before any filters are loaded.
diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.exp b/gdb/testsuite/gdb.python/py-unwind-maint.exp
index e89d284..3a98cb1 100644
--- a/gdb/testsuite/gdb.python/py-unwind-maint.exp
+++ b/gdb/testsuite/gdb.python/py-unwind-maint.exp
@@ -34,7 +34,11 @@ if ![runto_main ] then {
     return -1
 }
 
-gdb_test "source ${pyfile}" "Python script imported" "import python scripts"
+gdb_test_sequence "source ${pyfile}" "import python scripts" {
+    "Python script imported"
+    "py_unwind_maint_ps_unwinder called"
+    "global_unwinder called"
+}
 
 gdb_test_sequence "info unwinder" "Show all unwinders" {
     "Global:"
@@ -50,7 +54,10 @@ gdb_test_sequence "continue" "Unwinders called" {
     "global_unwinder called"
 }
 
-gdb_test "disable unwinder global .*" "1 unwinder disabled" "Unwinder disabled"
+gdb_test_sequence "disable unwinder global .*" "Unwinder disabled" {
+    "1 unwinder disabled"
+    "py_unwind_maint_ps_unwinder called"
+}
 
 gdb_test_sequence "info unwinder" "Show with global unwinder disabled" {
     "Global:"

[PATCH] Extend JIT-reader test and fix GDB problems that exposes (was: Re: [RFA 1/3] Clear frame cache when loading or unloading a JIT unwinder)

[Pedro Alves]

On 06/24/2016 03:39 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Tom> jit-reader-load apparently has no tests in-tree, so I didn't add a new
> Tom> test here.
> 
> Pedro> Yeah.  Tests existed, but had never been pushed in the tree...  I fixed
> Pedro> that now:
> Pedro>   https://sourceware.org/ml/gdb-patches/2016-06/msg00308.html
> Pedro> Could you see if it's easy to extend the test to cover this?
> 
> I looked and I didn't see a straightforward way.
> 
> The basic problem is that flushing the frame cache doesn't affect the
> results in the jit-reader.exp test.  In this test, the test case can be
> unwound without assistance; JIT reader in question supplies some
> symbols.  Clearing the frame cache doesn't affect this.

The JIT reader in the testcase does more than that, actually.  It really
jits a function (mmaps and pokes in some op codes) and runs to it.  It
then provides the symbol for that function, _but_, it also supplies a
corresponding JIT-reader unwinder.  So flushing the frame cache _should_ be
making a difference in the test.  But, alas, the test is broken, incomplete,
and after fixing that, it exposes further GDB problems...

All fixed by the patch below, which includes your change too.  I was aiming
at keeping mine as a separate patch, but with the unregister/register
code at jit-reader-unload/jit-reader-load time change, that no longer
makes much sense.  See details below.

From 9dc4f957370881309eecbbb8a8e5d639e93a1da1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 28 Jun 2016 16:49:46 +0100
Subject: [PATCH] Extend JIT-reader test and fix GDB problems that exposes

The jit-reader.exp test isn't really exercising the jit-reader's
unwinder API at all.  This commit address that, and then fixes GDB
problems exposed.

- The custom JIT reader provided for the jit-reader.exp testcase
  always rejects the jitted function's frame...

  This is because the custom JIT reader in the testcase never ever
  sets state->code_begin/end, so the bounds check in
  gdb.base/jitreader.c:unwind_frame:

   if (this_ip >= state->code_end || this_ip < state->code_begin)
     return GDB_FAIL;

  tends to fail, unless you're "lucky" (because it references
  uninitialized data).

  The result is that GDB is always actually using a built-in unwinder
  for the jitted function.

- The provided unwinder doesn't do anything that GDB's built-in
  unwinder can't do.

  IOW, we can't really tell whether the JIT reader's unwinder is
  working or not.

  I fixed that by making the jitted function mangle its own stack
  pointer with a xor, and then teaching the jit unwinder to demangle
  it back (another xor).  So now "backtrace" with GDB's built-in
  unwinder fails while with the jit unwinder, it succeeds.

- GDB crashes after unloading the JIT reader, and flushing frames...

  I made the testcase use the "flushregs" command after unloading the
  JIT reader, to force the JIT frames to be flushed.  However, that
  crashes GDB...

  When reinit_frame_cache tears down a frame's cache, it calls its
  unwinder's dealloc_cache method, which for JIT frames ends up in
  jit.c:jit_dealloc_cache.  This function calls each of the frame's
  gdb_reg_value's "free" pointer:

   for (i = 0; i < gdbarch_num_regs (frame_arch); i++)
     if (priv_data->registers[i] && priv_data->registers[i]->free)
       priv_data->registers[i]->free (priv_data->registers[i]);

  and the problem is these gdb_reg_value instances have been returned
  by the JIT reader that has been already unloaded, and their "free"
  function pointers likely point to functions in the DSO that has
  already been unloaded...

  A fix for that could be to call reinit_frame_cache in
  jit_reader_unload_command _before_ unloading the jit reader DSO so
  that the jit reader is given a chance to clean up the gdb_reg_values
  before it is unloaded.  However, the fix for the point below makes
  this unnecessary, because it stops jit.c from keeping around
  gdb_reg_values in the first place.

- However, it still makes sense to clear the frame cache when loading
  or unloading a JIT unwinder.

  This makes testing a JIT unwinder a bit simpler.

- Not only the frame cache actually -- gdb is not unloading the
  jit-registered objfiles when the JIT reader is unloaded, and not
  loading the already-registered descriptors when a JIT reader is
  loaded.

  The new test exercises unloading the jit reader, loading it back
  again, and then making sure the JIT reader's unwinder works again.
  Without the unload/re-load of already-read descriptors, the newly
  loaded JIT would have no idea where the new function is, because
  it's stored at symbol read time.

- I added a couple "info frame" calls to the test, and that
  crashes GDB...

  The problem is that jit_frame_prev_register assumes it'll only be
  called for raw registers, so when it gets a pseudo register number,
  the "priv->registers[reg]" access is really an out-of-bounds access.

  To fix that, I made jit_frame_prev_register use
  gdbarch_pseudo_register_read_value for reading the pseudo-registers.
  However, that works with a regcache and we don't have one.  To fix
  that, I made the JIT unwinder store a regcache in its cache instead
  of an array of gdb_reg_value pointers.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Tom Tromey  <tom@tromey.com>

	* jit.c (jit_reader_load_command): Call reinit_frame_cache and
	jit_inferior_created_hook.
	(jit_reader_unload_command): Call reinit_frame_cache and
	jit_inferior_exit_hook.
	* jit.c (struct jit_unwind_private) <registers>: Delete field.
	<regcache>: New field.
	(jit_unwind_reg_set_impl): Set the register's value in the
	regcache.  Free the passed-in gdb_reg_value.
	(jit_dealloc_cache): Adjust to free the regcache.
	(jit_frame_sniffer): Allocate a regcache instead of an array of
	gdb_reg_value pointers.
	(jit_frame_this_id): Adjust.
	(jit_frame_prev_register): Read raw registers off of the regcache
	instead of from the gdb_reg_value pointer array.  Use
	gdbarch_pseudo_register_read_value to read pseudo registers.
	* regcache.c (regcache_raw_set_cached_value): New function,
	factored out from ...
	(regcache_raw_write): ... here.
	* regcache.h (regcache_raw_set_cached_value): Declare.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/jit-reader.exp (info_registers_current_frame): New
	procedure.
	(jit_reader_test): Test the jit reader's unwinder.
	* gdb.base/jithost.c (jit_function_00_code): New global.
	(main): Use memcpy to fill in the mmapped code, instead of poking
	bytes manually here.
	* gdb.base/jitreader.c (enum register_mapping) <AMD64_RBP>: New
	value.
	(read_debug_info): Save the function's range.
	(read_sp): New function.
	(unwind_frame): Use it.  Also unwind RBP.
	(get_frame_id): Use read_sp.
	(gdb_init_reader): Use calloc instead of malloc.
	* lib/gdb.exp (get_hexadecimal_valueof): Add optional 'test'
	parameter.  Use gdb_test_multiple.
---
 gdb/jit.c                             |  51 ++++++----
 gdb/regcache.c                        |  15 ++-
 gdb/regcache.h                        |   8 ++
 gdb/testsuite/gdb.base/jit-reader.exp | 174 +++++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/jithost.c      |  24 ++++-
 gdb/testsuite/gdb.base/jitreader.c    |  55 +++++++++--
 gdb/testsuite/lib/gdb.exp             |  21 ++--
 7 files changed, 306 insertions(+), 42 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 9fd5ae6..2b6cf77 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -51,6 +51,7 @@ static const char *const jit_descriptor_name = "__jit_debug_descriptor";
 static const struct program_space_data *jit_program_space_data = NULL;
 
 static void jit_inferior_init (struct gdbarch *gdbarch);
+static void jit_inferior_exit_hook (struct inferior *inf);
 
 /* An unwinder is registered for every gdbarch.  This key is used to
    remember if the unwinder has been registered for a particular
@@ -218,6 +219,8 @@ jit_reader_load_command (char *args, int from_tty)
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
+  reinit_frame_cache ();
+  jit_inferior_created_hook ();
   do_cleanups (prev_cleanup);
 }
 
@@ -229,6 +232,8 @@ jit_reader_unload_command (char *args, int from_tty)
   if (!loaded_jit_reader)
     error (_("No JIT reader loaded."));
 
+  reinit_frame_cache ();
+  jit_inferior_exit_hook (current_inferior ());
   loaded_jit_reader->functions->destroy (loaded_jit_reader->functions);
 
   gdb_dlclose (loaded_jit_reader->handle);
@@ -1090,7 +1095,7 @@ struct jit_unwind_private
 {
   /* Cached register values.  See jit_frame_sniffer to see how this
      works.  */
-  struct gdb_reg_value **registers;
+  struct regcache *regcache;
 
   /* The frame being unwound.  */
   struct frame_info *this_frame;
@@ -1115,11 +1120,12 @@ jit_unwind_reg_set_impl (struct gdb_unwind_callbacks *cb, int dwarf_regnum,
         fprintf_unfiltered (gdb_stdlog,
                             _("Could not recognize DWARF regnum %d"),
                             dwarf_regnum);
+      value->free (value);
       return;
     }
 
-  gdb_assert (priv->registers);
-  priv->registers[gdb_reg] = value;
+  regcache_raw_set_cached_value (priv->regcache, gdb_reg, value->value);
+  value->free (value);
 }
 
 static void
@@ -1162,14 +1168,10 @@ jit_dealloc_cache (struct frame_info *this_frame, void *cache)
   struct gdbarch *frame_arch;
   int i;
 
-  gdb_assert (priv_data->registers);
+  gdb_assert (priv_data->regcache != NULL);
   frame_arch = get_frame_arch (priv_data->this_frame);
 
-  for (i = 0; i < gdbarch_num_regs (frame_arch); i++)
-    if (priv_data->registers[i] && priv_data->registers[i]->free)
-      priv_data->registers[i]->free (priv_data->registers[i]);
-
-  xfree (priv_data->registers);
+  regcache_xfree (priv_data->regcache);
   xfree (priv_data);
 }
 
@@ -1188,6 +1190,8 @@ jit_frame_sniffer (const struct frame_unwind *self,
   struct jit_unwind_private *priv_data;
   struct gdb_unwind_callbacks callbacks;
   struct gdb_reader_funcs *funcs;
+  struct address_space *aspace;
+  struct gdbarch *gdbarch;
 
   callbacks.reg_get = jit_unwind_reg_get_impl;
   callbacks.reg_set = jit_unwind_reg_set_impl;
@@ -1200,11 +1204,12 @@ jit_frame_sniffer (const struct frame_unwind *self,
 
   gdb_assert (!*cache);
 
+  aspace = get_frame_address_space (this_frame);
+  gdbarch = get_frame_arch (this_frame);
+
   *cache = XCNEW (struct jit_unwind_private);
   priv_data = (struct jit_unwind_private *) *cache;
-  priv_data->registers =
-    XCNEWVEC (struct gdb_reg_value *,	      
-	      gdbarch_num_regs (get_frame_arch (this_frame)));
+  priv_data->regcache = regcache_xmalloc (gdbarch, aspace);
   priv_data->this_frame = this_frame;
 
   callbacks.priv_data = priv_data;
@@ -1240,7 +1245,7 @@ jit_frame_this_id (struct frame_info *this_frame, void **cache,
   struct gdb_reader_funcs *funcs;
   struct gdb_unwind_callbacks callbacks;
 
-  priv.registers = NULL;
+  priv.regcache = NULL;
   priv.this_frame = this_frame;
 
   /* We don't expect the frame_id function to set any registers, so we
@@ -1264,17 +1269,25 @@ static struct value *
 jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg)
 {
   struct jit_unwind_private *priv = (struct jit_unwind_private *) *cache;
-  struct gdb_reg_value *value;
+  struct gdbarch *gdbarch;
 
   if (priv == NULL)
     return frame_unwind_got_optimized (this_frame, reg);
 
-  gdb_assert (priv->registers);
-  value = priv->registers[reg];
-  if (value && value->defined)
-    return frame_unwind_got_bytes (this_frame, reg, value->value);
+  gdbarch = get_regcache_arch (priv->regcache);
+  if (reg < gdbarch_num_regs (gdbarch))
+    {
+      gdb_byte *buf = (gdb_byte *) alloca (register_size (gdbarch, reg));
+      enum register_status status;
+
+      status = regcache_raw_read (priv->regcache, reg, buf);
+      if (status == REG_VALID)
+	return frame_unwind_got_bytes (this_frame, reg, buf);
+      else
+	return frame_unwind_got_optimized (this_frame, reg);
+    }
   else
-    return frame_unwind_got_optimized (this_frame, reg);
+    return gdbarch_pseudo_register_read_value (gdbarch, priv->regcache, reg);
 }
 
 /* Relay everything back to the unwinder registered by the JIT debug
diff --git a/gdb/regcache.c b/gdb/regcache.c
index f0ba0cf..a5c90a6 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -890,6 +890,17 @@ regcache_cooked_write_unsigned (struct regcache *regcache, int regnum,
   regcache_cooked_write (regcache, regnum, buf);
 }
 
+/* See regcache.h.  */
+
+void
+regcache_raw_set_cached_value (struct regcache *regcache, int regnum,
+			       const gdb_byte *buf)
+{
+  memcpy (register_buffer (regcache, regnum), buf,
+	  regcache->descr->sizeof_register[regnum]);
+  regcache->register_status[regnum] = REG_VALID;
+}
+
 void
 regcache_raw_write (struct regcache *regcache, int regnum,
 		    const gdb_byte *buf)
@@ -917,9 +928,7 @@ regcache_raw_write (struct regcache *regcache, int regnum,
   inferior_ptid = regcache->ptid;
 
   target_prepare_to_store (regcache);
-  memcpy (register_buffer (regcache, regnum), buf,
-	  regcache->descr->sizeof_register[regnum]);
-  regcache->register_status[regnum] = REG_VALID;
+  regcache_raw_set_cached_value (regcache, regnum, buf);
 
   /* Register a cleanup function for invalidating the register after it is
      written, in case of a failure.  */
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 1a4ff42..1bb0ce0 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -66,6 +66,14 @@ extern void regcache_raw_write_signed (struct regcache *regcache,
 extern void regcache_raw_write_unsigned (struct regcache *regcache,
 					 int regnum, ULONGEST val);
 
+/* Set a raw register's value in the regcache's buffer.  Unlike
+   regcache_raw_write, this is not write-through.  The intention is
+   allowing to change the buffer contents of a read-only regcache
+   allocated with regcache_xmalloc.  */
+
+extern void regcache_raw_set_cached_value
+  (struct regcache *regcache, int regnum, const gdb_byte *buf);
+
 /* Partial transfer of raw registers.  These perform read, modify,
    write style operations.  The read variant returns the status of the
    register.  */
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index b5e297a..642c257 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -57,10 +57,50 @@ if { [gdb_compile_shlib "${srcdir}/${subdir}/${jit_reader_src}" "${jit_reader_bi
     return -1
 }
 
+# Test "info registers" in the current frame, expecting RSP's value to
+# be SP.
+
+proc info_registers_current_frame {sp} {
+    global hex decimal
+
+    set any "\[^\r\n\]*"
+
+    gdb_test "info registers" \
+	[multi_line \
+	     "rax            $hex	$decimal" \
+	     "rbx            $hex	$decimal" \
+	     "rcx            $hex	$decimal" \
+	     "rdx            $hex	$decimal" \
+	     "rsi            $hex	$decimal" \
+	     "rdi            $hex	$decimal" \
+	     "rbp            $hex	$hex" \
+	     "rsp            $sp	$sp" \
+	     "r8             $hex	$decimal" \
+	     "r9             $hex	$decimal" \
+	     "r10            $hex	$decimal" \
+	     "r11            $hex	$decimal" \
+	     "r12            $hex	$decimal" \
+	     "r13            $hex	$decimal" \
+	     "r14            $hex	$decimal" \
+	     "r15            $hex	$decimal" \
+	     "rip            $hex	$hex$any" \
+	     "eflags         $hex	\\\[$any\\\]" \
+	     "cs             $hex	$decimal" \
+	     "ss             $hex	$decimal" \
+	     "ds             $hex	$decimal" \
+	     "es             $hex	$decimal" \
+	     "fs             $hex	$decimal" \
+	     "gs             $hex	$decimal" \
+	    ]
+}
+
 proc jit_reader_test {} {
     global jit_host_bin
     global jit_reader_bin
     global verbose
+    global hex decimal
+
+    set any "\[^\r\n\]*"
 
     clean_restart $jit_host_bin
     gdb_load_shlib $jit_reader_bin
@@ -73,7 +113,139 @@ proc jit_reader_test {} {
     gdb_run_cmd
     gdb_test "" "Program received signal SIGTRAP, .*" "expect SIGTRAP"
 
-    gdb_test "bt" "jit_function_00.*"
+    # Test the JIT reader unwinder.
+    with_test_prefix "with jit-reader" {
+
+	with_test_prefix "before mangling" {
+	    gdb_test "bt" \
+		[multi_line \
+		     "#0 ${any} in jit_function_00 ${any}" \
+		     "#1 ${any} in main ${any}" \
+		    ] \
+		"bt works"
+
+	    set sp_before_mangling \
+		[get_hexadecimal_valueof "\$sp" 0 "get sp"]
+
+	    gdb_test "up" "#1  $any in main $any\r\n$any  function $any" \
+		"move up to caller"
+
+	    set caller_sp \
+		[get_hexadecimal_valueof "\$sp" 0 "get caller sp"]
+	}
+
+	# Step over the instruction that mangles the stack pointer.
+	# While that confuses GDB's built-in unwinder, the JIT
+	# reader's unwinder understands the mangling and should thus
+	# be able to unwind at that location.
+	with_test_prefix "after mangling" {
+	    gdb_test "si" "in jit_function_00 .*" "step over stack mangling"
+
+	    set sp_after_mangling \
+		[get_hexadecimal_valueof "\$sp" 0 "get sp"]
+
+	    gdb_assert {$sp_before_mangling != $sp_after_mangling} \
+		"sp is mangled"
+
+	    # Check that the jit unwinder manages to backtrace through
+	    # the mangled stack pointer.
+	    gdb_test "bt" \
+		[multi_line \
+		     "#0 ${any} in jit_function_00 ${any}" \
+		     "#1 ${any} in main ${any}" \
+		    ] \
+		"bt works"
+
+	    with_test_prefix "current frame" {
+		info_registers_current_frame $sp_after_mangling
+
+		gdb_test "info frame" \
+		    "Stack level 0, frame at $sp_before_mangling.*in jit_function_00.*"
+	    }
+
+	    with_test_prefix "caller frame" {
+		gdb_test "up" "#1  $any in main $any\r\n$any  function $any" \
+		    "up to caller"
+
+		# Since the JIT unwinder only provides RIP/RSP/RBP,
+		# all other registers should show as "<not saved>".
+		gdb_test "info registers" \
+		    [multi_line \
+			 "rax            <not saved>" \
+			 "rbx            <not saved>" \
+			 "rcx            <not saved>" \
+			 "rdx            <not saved>" \
+			 "rsi            <not saved>" \
+			 "rdi            <not saved>" \
+			 "rbp            $hex	$hex" \
+			 "rsp            $caller_sp	$caller_sp" \
+			 "r8             <not saved>" \
+			 "r9             <not saved>" \
+			 "r10            <not saved>" \
+			 "r11            <not saved>" \
+			 "r12            <not saved>" \
+			 "r13            <not saved>" \
+			 "r14            <not saved>" \
+			 "r15            <not saved>" \
+			 "rip            $hex	$hex $any" \
+			 "eflags         <not saved>" \
+			 "cs             <not saved>" \
+			 "ss             <not saved>" \
+			 "ds             <not saved>" \
+			 "es             <not saved>" \
+			 "fs             <not saved>" \
+			 "gs             <not saved>" \
+			]
+
+		# Make sure that "info frame" doesn't crash.
+		gdb_test "info frame" "Stack level 1, .*in main.*"
+
+		# ... and that neither does printing a pseudo
+		# register.
+		gdb_test "print /x \$ebp" " = $hex" "print pseudo register"
+
+		# There's no way for the JIT reader API to support
+		# modifyiable values.
+		gdb_test "print \$rbp = -1" \
+		    "Attempt to assign to an unmodifiable value\." \
+		    "cannot assign to register"
+	    }
+	}
+    }
+
+    # Now unload the jit reader, and ensure that backtracing really
+    # doesn't work without it.
+    with_test_prefix "without jit-reader" {
+	gdb_test_no_output "jit-reader-unload ${jit_reader_bin}" \
+	    "jit-reader-unload"
+
+	# Check that we're no longer using the JIT unwinder, and that
+	# the built-in unwinder cannot backtrace through the mangled
+	# stack pointer.
+	gdb_test "bt" \
+	    [multi_line \
+		 "Backtrace stopped: Cannot access memory at address $sp_after_mangling" \
+		] \
+	    "bt shows error"
+
+	gdb_test "info frame" "Cannot access memory at address.*" \
+	    "info frame shows error"
+	info_registers_current_frame $sp_after_mangling
+	gdb_test "up" "Initial frame selected; you cannot go up\\." \
+	    "cannot go up"
+    }
+
+    with_test_prefix "with jit-reader again" {
+	gdb_test_no_output "jit-reader-load ${jit_reader_bin}" "jit-reader-load"
+
+	# Check that the jit unwinder manages to backtrace through
+	# the mangled stack pointer.
+	gdb_test "bt" \
+	    [multi_line \
+		 "#0 ${any} in jit_function_00 ${any}" \
+		 "#1 ${any} in main ${any}" \
+		]
+    }
 }
 
 jit_reader_test
diff --git a/gdb/testsuite/gdb.base/jithost.c b/gdb/testsuite/gdb.base/jithost.c
index 09ab377..afefc98 100644
--- a/gdb/testsuite/gdb.base/jithost.c
+++ b/gdb/testsuite/gdb.base/jithost.c
@@ -33,18 +33,32 @@ struct jit_code_entry only_entry;
 
 typedef void (jit_function_t) ();
 
-int main (int argc, char **argv)
+/* The code of the jit_function_00 function that is copied into an
+   mmapped buffer in the inferior at run time.
+
+   The second instruction mangles the stack pointer, meaning that when
+   stopped at the third instruction, GDB needs assistance from the JIT
+   unwinder in order to be able to unwind successfully.  */
+const unsigned char jit_function_00_code[] = {
+  0xcc,				/* int3 */
+  0x48, 0x83, 0xf4, 0xff,	/* xor $0xffffffffffffffff, %rsp */
+  0x48, 0x83, 0xf4, 0xff,	/* xor $0xffffffffffffffff, %rsp */
+  0xc3				/* ret */
+};
+
+int
+main (int argc, char **argv)
 {
+  struct jithost_abi *symfile;
   char *code = mmap (NULL, getpagesize (), PROT_WRITE | PROT_EXEC,
 		     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   jit_function_t *function = (jit_function_t *) code;
 
-  code[0] = 0xcc; /* SIGTRAP  */
-  code[1] = 0xc3; /* RET  */
+  memcpy (code, jit_function_00_code, sizeof (jit_function_00_code));
 
-  struct jithost_abi *symfile = malloc (sizeof (struct jithost_abi));
+  symfile = malloc (sizeof (struct jithost_abi));
   symfile->begin = code;
-  symfile->end = code + 2;
+  symfile->end = code + sizeof (jit_function_00_code);
 
   only_entry.symfile_addr = symfile;
   only_entry.symfile_size = sizeof (struct jithost_abi);
diff --git a/gdb/testsuite/gdb.base/jitreader.c b/gdb/testsuite/gdb.base/jitreader.c
index 850fb46..c18e40d 100644
--- a/gdb/testsuite/gdb.base/jitreader.c
+++ b/gdb/testsuite/gdb.base/jitreader.c
@@ -28,6 +28,7 @@ GDB_DECLARE_GPL_COMPATIBLE_READER;
 enum register_mapping
 {
   AMD64_RA = 16,
+  AMD64_RBP = 6,
   AMD64_RSP = 7,
 };
 
@@ -47,6 +48,11 @@ read_debug_info (struct gdb_reader_funcs *self,
   struct gdb_symtab *symtab = cbs->symtab_open (cbs, object, "");
   GDB_CORE_ADDR begin = (GDB_CORE_ADDR) symfile->begin;
   GDB_CORE_ADDR end = (GDB_CORE_ADDR) symfile->end;
+  struct reader_state *state = (struct reader_state *) self->priv_data;
+
+  /* Record the function's range, for the unwinder.  */
+  state->code_begin = begin;
+  state->code_end = end;
 
   cbs->block_open (cbs, symtab, NULL, begin, end, "jit_function_00");
 
@@ -91,11 +97,36 @@ read_register (struct gdb_unwind_callbacks *callbacks, int dw_reg,
   return 1;
 }
 
+/* Read the stack pointer into *VALUE.  IP is the address the inferior
+   is currently stopped at.  Takes care of demangling the stack
+   pointer if necessary.  */
+
+static int
+read_sp (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs,
+	 uintptr_t ip, uintptr_t *value)
+{
+  struct reader_state *state = (struct reader_state *) self->priv_data;
+  uintptr_t sp;
+
+  if (!read_register (cbs, AMD64_RSP, &sp))
+    return GDB_FAIL;
+
+  /* If stopped at the instruction after the "xor $-1, %rsp", demangle
+     the stack pointer back.  */
+  if (ip == state->code_begin + 5)
+    sp ^= (uintptr_t) -1;
+
+  *value = sp;
+  return GDB_SUCCESS;
+}
+
 static enum gdb_status
 unwind_frame (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs)
 {
   const int word_size = sizeof (uintptr_t);
-  uintptr_t this_sp, this_ip, prev_ip, prev_sp;
+  uintptr_t prev_sp, this_sp;
+  uintptr_t prev_ip, this_ip;
+  uintptr_t prev_bp, this_bp;
   struct reader_state *state = (struct reader_state *) self->priv_data;
 
   if (!read_register (cbs, AMD64_RA, &this_ip))
@@ -104,15 +135,25 @@ unwind_frame (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs)
   if (this_ip >= state->code_end || this_ip < state->code_begin)
     return GDB_FAIL;
 
-  if (!read_register (cbs, AMD64_RSP, &this_sp))
+  /* Unwind RBP in order to make the unwinder that tries to unwind
+     from the just-unwound frame happy.  */
+  if (!read_register (cbs, AMD64_RBP, &this_bp))
     return GDB_FAIL;
+  /* RBP is unmodified.  */
+  prev_bp = this_bp;
 
-  if (cbs->target_read (this_sp, &prev_ip, word_size) == GDB_FAIL)
+  /* Fetch the demangled stack pointer.  */
+  if (!read_sp (self, cbs, this_ip, &this_sp))
     return GDB_FAIL;
 
+  /* The return address is saved on the stack.  */
+  if (cbs->target_read (this_sp, &prev_ip, word_size) == GDB_FAIL)
+    return GDB_FAIL;
   prev_sp = this_sp + word_size;
+
   write_register (cbs, AMD64_RA, prev_ip);
   write_register (cbs, AMD64_RSP, prev_sp);
+  write_register (cbs, AMD64_RBP, prev_bp);
   return GDB_SUCCESS;
 }
 
@@ -121,9 +162,11 @@ get_frame_id (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *cbs)
 {
   struct reader_state *state = (struct reader_state *) self->priv_data;
   struct gdb_frame_id frame_id;
-
+  uintptr_t ip;
   uintptr_t sp;
-  read_register (cbs, AMD64_RSP, &sp);
+
+  read_register (cbs, AMD64_RA, &ip);
+  read_sp (self, cbs, ip, &sp);
 
   frame_id.code_address = (GDB_CORE_ADDR) state->code_begin;
   frame_id.stack_address = (GDB_CORE_ADDR) sp;
@@ -141,7 +184,7 @@ destroy_reader (struct gdb_reader_funcs *self)
 struct gdb_reader_funcs *
 gdb_init_reader (void)
 {
-  struct reader_state *state = malloc (sizeof (struct reader_state));
+  struct reader_state *state = calloc (1, sizeof (struct reader_state));
   struct gdb_reader_funcs *reader_funcs =
     malloc (sizeof (struct gdb_reader_funcs));
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b52fdd9..b7b8fad 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5435,19 +5435,24 @@ proc get_integer_valueof { exp default } {
     return ${val}
 }
 
-proc get_hexadecimal_valueof { exp default } {
+# Retrieve the value of EXP in the inferior, as an hexadecimal value
+# (using "print /x").  DEFAULT is used as fallback if print fails.
+# TEST is the test message to use.  If can be ommitted, in which case
+# a test message is built from EXP.
+
+proc get_hexadecimal_valueof { exp default {test ""} } {
     global gdb_prompt
-    send_gdb "print /x ${exp}\n"
-    set test "get hexadecimal valueof \"${exp}\""
-    gdb_expect {
+
+    if {$test == ""} {
+	set test "get hexadecimal valueof \"${exp}\""
+    }
+
+    set val ${default}
+    gdb_test_multiple "print /x ${exp}" $test {
 	-re "\\$\[0-9\]* = (0x\[0-9a-zA-Z\]+).*$gdb_prompt $" {
 	    set val $expect_out(1,string)
 	    pass "$test"
 	}
-	timeout {
-	    set val ${default}
-	    fail "$test (timeout)"
-	}
     }
     return ${val}
 }
-- 
2.5.5

Re: [PATCH] Extend JIT-reader test and fix GDB problems that exposes

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> But, alas, the test is broken, incomplete, and after fixing that,
Pedro> it exposes further GDB problems...

Wow, thank you for doing all of that.

Tom

Re: [PATCH] Extend JIT-reader test and fix GDB problems that exposes

[Pedro Alves]

On 06/28/2016 11:14 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> But, alas, the test is broken, incomplete, and after fixing that,
> Pedro> it exposes further GDB problems...
> 
> Wow, thank you for doing all of that.

My pleasure.  We've heard from a couple users of this interface
recently so it's useful effort.

I've pushed it in now.

Thanks,
Pedro Alves