[PATCH 4/6] infcall: stop_registers -> register_dummy_frame_dtor

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

with dummy_frame destructors GDB no longer has to use global stop_registers.
dummy_frame's registers can be now stored associated with their specific
dummy_frame.

stop_registers was questioned by Pedro in:
	Re: [PATCH v4 11/11] RFC only: compile: Use also inferior munmap
	https://sourceware.org/ml/gdb-patches/2015-05/msg00107.html

I am not completely sure what is right for bpfinishpy_pre_stop_hook but the
testsuite passes.  I can think more about it if it gets otherwise approved.


Jan


gdb/ChangeLog
2015-05-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* infcall.c (struct call_function_by_hand_dtor)
	(call_function_by_hand_dtor_data_free, call_function_by_hand_dtor)
	(call_function_by_hand_dtor_drop, call_function_by_hand_dtor_cleanup)
	(call_function_by_hand_dtor_get, call_function_by_hand_dtor_setup): New.
	(call_function_by_hand_dummy): Move discard_cleanups of
	inf_status_cleanup before dummy_frame_push.  Call
	call_function_by_hand_dtor_setup and prepare our_dtor_cleanup.
	Use call_function_by_hand_dtor_get instead of stop_registers.
	* infcall.h (struct call_function_by_hand_dtor)
	(call_function_by_hand_dtor_drop, call_function_by_hand_dtor_cleanup)
	(call_function_by_hand_dtor_get, call_function_by_hand_dtor_setup): New
	declarations.
	* infcmd.c: Include infcall.h.
	(get_return_value): Add parameter dtor_data, use it instead of
	stop_registers.
	(print_return_value): Add parameter dtor_data, pass it.
	(struct finish_command_continuation_args): Add field dtor_data.
	(finish_command_continuation): Update print_return_value caller.
	(finish_command_continuation_free_arg): Free also dtor_data.
	(finish_forward): Call call_function_by_hand_dtor_setup.
	* inferior.h (struct call_function_by_hand_dtor): New declaration.
	(get_return_value): Add parameter dtor_data.
	* python/py-finishbreakpoint.c (bpfinishpy_pre_stop_hook): Update
	get_return_value caller.
---
 gdb/infcall.c                    |  118 ++++++++++++++++++++++++++++++++++----
 gdb/infcall.h                    |   10 +++
 gdb/infcmd.c                     |   48 ++++++++++++---
 gdb/inferior.h                   |    6 +-
 gdb/python/py-finishbreakpoint.c |    2 -
 5 files changed, 159 insertions(+), 25 deletions(-)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index cef6b91..9707861 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -469,6 +469,97 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   return call_function_by_hand_dummy (function, nargs, args, NULL, NULL);
 }
 
+/* Data for call_function_by_hand_dtor.  Structure can be freed only
+   after both call_function_by_hand_dtor and
+   call_function_by_hand_dtor_drop have been called for it.  */
+
+struct call_function_by_hand_dtor
+{
+  /* Inferior registers fetched before associated dummy_frame got freed
+     and before any other destructors of associated dummy_frame got called.
+     It is initialized to NULL.  */
+  struct regcache *retbuf;
+
+  /* It is 1 if this call_function_by_hand_dtor_drop has been already
+     called.  */
+  int drop_done;
+};
+
+/* Free struct call_function_by_hand_dtor.  */
+
+static void
+call_function_by_hand_dtor_data_free (struct call_function_by_hand_dtor *data)
+{
+  regcache_xfree (data->retbuf);
+  xfree (data);
+}
+
+/* Destructor for associated dummy_frame.  */
+
+static dummy_frame_dtor_ftype call_function_by_hand_dtor;
+static void
+call_function_by_hand_dtor (void *data_voidp, int registers_valid)
+{
+  struct call_function_by_hand_dtor *data = data_voidp;
+
+  gdb_assert (data->retbuf == NULL);
+
+  if (data->drop_done)
+    call_function_by_hand_dtor_data_free (data);
+  else if (registers_valid)
+    data->retbuf = regcache_dup (get_current_regcache ());
+}
+
+/* Caller is no longer interested in this
+   struct call_function_by_hand_dtor.  After its associated dummy_frame
+   gets freed struct call_function_by_hand_dtor can be also freed.  */
+
+void
+call_function_by_hand_dtor_drop (struct call_function_by_hand_dtor *data)
+{
+  data->drop_done = 1;
+
+  if (!find_dummy_frame_dtor (call_function_by_hand_dtor, data))
+    call_function_by_hand_dtor_data_free (data);
+}
+
+/* Stub for call_function_by_hand_dtor_drop compatible with make_cleanup.  */
+
+void
+call_function_by_hand_dtor_cleanup (void *data_voidp)
+{
+  struct call_function_by_hand_dtor *data = data_voidp;
+
+  call_function_by_hand_dtor_drop (data);
+}
+
+/* Fetch RETBUF field of possibly opaque DTOR_DATA.
+   RETBUF must not be NULL.  */
+
+struct regcache *
+call_function_by_hand_dtor_get (struct call_function_by_hand_dtor *dtor_data)
+{
+  gdb_assert (dtor_data->retbuf != NULL);
+  return dtor_data->retbuf;
+}
+
+/* Register provider of inferior registers at the time DUMMY_ID frame of
+   PTID gets freed (before inferior registers get restored to those
+   before dummy_frame).  */
+
+struct call_function_by_hand_dtor *
+call_function_by_hand_dtor_setup (struct frame_id dummy_id, ptid_t ptid)
+{
+  struct call_function_by_hand_dtor *our_dtor_data;
+
+  our_dtor_data = xmalloc (sizeof (*our_dtor_data));
+  our_dtor_data->retbuf = NULL;
+  our_dtor_data->drop_done = 0;
+  register_dummy_frame_dtor (dummy_id, inferior_ptid,
+			     call_function_by_hand_dtor, our_dtor_data);
+  return our_dtor_data;
+}
+
 /* All this stuff with a dummy frame may seem unnecessarily complicated
    (why not just save registers in GDB?).  The purpose of pushing a dummy
    frame which looks just like a real frame is so that if you call a
@@ -513,6 +604,8 @@ call_function_by_hand_dummy (struct value *function,
   struct gdb_exception e;
   char name_buf[RAW_FUNCTION_ADDRESS_SIZE];
   int stack_temporaries = thread_stack_temporaries_enabled_p (inferior_ptid);
+  struct call_function_by_hand_dtor *our_dtor_data;
+  struct cleanup *our_dtor_cleanup;
 
   if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
     ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
@@ -886,6 +979,11 @@ call_function_by_hand_dummy (struct value *function,
   if (unwind_on_terminating_exception_p)
     set_std_terminate_breakpoint ();
 
+  /* Discard both inf_status and caller_state cleanups.
+     From this point on we explicitly restore the associated state
+     or discard it.  */
+  discard_cleanups (inf_status_cleanup);
+
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
      stack.  */
@@ -894,10 +992,12 @@ call_function_by_hand_dummy (struct value *function,
     register_dummy_frame_dtor (dummy_id, inferior_ptid,
 			       dummy_dtor, dummy_dtor_data);
 
-  /* Discard both inf_status and caller_state cleanups.
-     From this point on we explicitly restore the associated state
-     or discard it.  */
-  discard_cleanups (inf_status_cleanup);
+  /* call_function_by_hand_dtor_setup should be called last so that its
+     saving of inferior registers gets called first (before possible
+     DUMMY_DTOR destructor).  */
+  our_dtor_data = call_function_by_hand_dtor_setup (dummy_id, inferior_ptid);
+  our_dtor_cleanup = make_cleanup (call_function_by_hand_dtor_cleanup,
+				   our_dtor_data);
 
   /* Register a clean-up for unwind_on_terminating_exception_breakpoint.  */
   terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
@@ -1112,13 +1212,8 @@ When the function is done executing, GDB will silently stop."),
      and the dummy frame has already been popped.  */
 
   {
-    struct address_space *aspace = get_regcache_aspace (stop_registers);
-    struct regcache *retbuf = regcache_xmalloc (gdbarch, aspace);
-    struct cleanup *retbuf_cleanup = make_cleanup_regcache_xfree (retbuf);
     struct value *retval = NULL;
 
-    regcache_cpy_no_passthrough (retbuf, stop_registers);
-
     /* Inferior call is successful.  Restore the inferior status.
        At this stage, leave the RETBUF alone.  */
     restore_infcall_control_state (inf_status);
@@ -1145,7 +1240,8 @@ When the function is done executing, GDB will silently stop."),
       {
 	retval = allocate_value (values_type);
 	gdbarch_return_value (gdbarch, function, values_type,
-			      retbuf, value_contents_raw (retval), NULL);
+			      call_function_by_hand_dtor_get (our_dtor_data),
+			      value_contents_raw (retval), NULL);
 	if (stack_temporaries && class_or_union_p (values_type))
 	  {
 	    /* Values of class type returned in registers are copied onto
@@ -1160,7 +1256,7 @@ When the function is done executing, GDB will silently stop."),
 	  }
       }
 
-    do_cleanups (retbuf_cleanup);
+    do_cleanups (our_dtor_cleanup);
 
     gdb_assert (retval);
     return retval;
diff --git a/gdb/infcall.h b/gdb/infcall.h
index 77c5101..9ec8617 100644
--- a/gdb/infcall.h
+++ b/gdb/infcall.h
@@ -50,4 +50,14 @@ extern struct value *
 			       dummy_frame_dtor_ftype *dummy_dtor,
 			       void *dummy_dtor_data);
 
+struct call_function_by_hand_dtor;
+extern void call_function_by_hand_dtor_drop
+  (struct call_function_by_hand_dtor *data);
+extern void call_function_by_hand_dtor_cleanup
+  (void *data_voidp);
+extern struct regcache *call_function_by_hand_dtor_get
+  (struct call_function_by_hand_dtor *dtor_data);
+extern struct call_function_by_hand_dtor *call_function_by_hand_dtor_setup
+  (struct frame_id dummy_id, ptid_t ptid);
+
 #endif
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 11d5310..f8874d3 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -54,6 +54,7 @@
 #include "continuations.h"
 #include "linespec.h"
 #include "cli/cli-utils.h"
+#include "infcall.h"
 
 /* Local functions: */
 
@@ -1506,18 +1507,22 @@ advance_command (char *arg, int from_tty)
 }
 
 /* Return the value of the result of a function at the end of a 'finish'
-   command/BP.  */
+   command/BP.  DTOR_DATA (if not NULL) can represent inferior registers
+   right after an inferior call has finished.  */
 
 struct value *
-get_return_value (struct value *function, struct type *value_type)
+get_return_value (struct value *function, struct type *value_type,
+		  struct call_function_by_hand_dtor *dtor_data)
 {
-  struct regcache *stop_regs = stop_registers;
+  struct regcache *stop_regs = NULL;
   struct gdbarch *gdbarch;
   struct value *value;
   struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
 
   /* If stop_registers were not saved, use the current registers.  */
-  if (!stop_regs)
+  if (dtor_data != NULL)
+    stop_regs = call_function_by_hand_dtor_get (dtor_data);
+  else
     {
       stop_regs = regcache_dup (get_current_regcache ());
       make_cleanup_regcache_xfree (stop_regs);
@@ -1557,12 +1562,15 @@ get_return_value (struct value *function, struct type *value_type)
   return value;
 }
 
-/* Print the result of a function at the end of a 'finish' command.  */
+/* Print the result of a function at the end of a 'finish' command.
+   DTOR_DATA (if not NULL) can represent inferior registers right after
+   an inferior call has finished.  */
 
 static void
-print_return_value (struct value *function, struct type *value_type)
+print_return_value (struct value *function, struct type *value_type,
+		    struct call_function_by_hand_dtor *dtor_data)
 {
-  struct value *value = get_return_value (function, value_type);
+  struct value *value = get_return_value (function, value_type, dtor_data);
   struct ui_out *uiout = current_uiout;
 
   if (value)
@@ -1613,6 +1621,11 @@ struct finish_command_continuation_args
   int thread;
   struct breakpoint *breakpoint;
   struct symbol *function;
+
+  /* Inferior registers stored right before dummy_frame has been freed
+     after an inferior call.  It can be NULL if no inferior call was
+     involved, GDB will then use current inferior registers.  */
+  struct call_function_by_hand_dtor *dtor_data;
 };
 
 static void
@@ -1653,7 +1666,7 @@ finish_command_continuation (void *arg, int err)
 		  /* print_return_value can throw an exception in some
 		     circumstances.  We need to catch this so that we still
 		     delete the breakpoint.  */
-		  print_return_value (func, value_type);
+		  print_return_value (func, value_type, a->dtor_data);
 		}
 	      CATCH (ex, RETURN_MASK_ALL)
 		{
@@ -1677,7 +1690,11 @@ finish_command_continuation (void *arg, int err)
 static void
 finish_command_continuation_free_arg (void *arg)
 {
-  xfree (arg);
+  struct finish_command_continuation_args *cargs = arg;
+
+  if (cargs->dtor_data != NULL)
+    call_function_by_hand_dtor_drop (cargs->dtor_data);
+  xfree (cargs);
 }
 
 /* finish_backward -- helper function for finish_command.  */
@@ -1742,13 +1759,21 @@ finish_forward (struct symbol *function, struct frame_info *frame)
   struct symtab_and_line sal;
   struct thread_info *tp = inferior_thread ();
   struct breakpoint *breakpoint;
-  struct cleanup *old_chain;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   struct finish_command_continuation_args *cargs;
   int thread = tp->num;
+  struct call_function_by_hand_dtor *dtor_data = NULL;
 
   sal = find_pc_line (get_frame_pc (frame), 0);
   sal.pc = get_frame_pc (frame);
 
+  if (get_frame_type (frame) == DUMMY_FRAME)
+    {
+      dtor_data = call_function_by_hand_dtor_setup (get_stack_frame_id (frame),
+						    inferior_ptid);
+      make_cleanup (call_function_by_hand_dtor_cleanup, dtor_data);
+    }
+
   breakpoint = set_momentary_breakpoint (gdbarch, sal,
 					 get_stack_frame_id (frame),
                                          bp_finish);
@@ -1756,7 +1781,7 @@ finish_forward (struct symbol *function, struct frame_info *frame)
   /* set_momentary_breakpoint invalidates FRAME.  */
   frame = NULL;
 
-  old_chain = make_cleanup_delete_breakpoint (breakpoint);
+  make_cleanup_delete_breakpoint (breakpoint);
 
   set_longjmp_breakpoint (tp, frame_id);
   make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
@@ -1768,6 +1793,7 @@ finish_forward (struct symbol *function, struct frame_info *frame)
   cargs->thread = thread;
   cargs->breakpoint = breakpoint;
   cargs->function = function;
+  cargs->dtor_data = dtor_data;
   add_continuation (tp, finish_command_continuation, cargs,
                     finish_command_continuation_free_arg);
   proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 2530777..f305e79 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -165,8 +165,10 @@ extern void detach_command (char *, int);
 
 extern void notice_new_inferior (ptid_t, int, int);
 
-extern struct value *get_return_value (struct value *function,
-                                       struct type *value_type);
+struct call_function_by_hand_dtor;
+extern struct value *get_return_value
+  (struct value *function, struct type *value_type,
+   struct call_function_by_hand_dtor *dtor_data);
 
 /* Prepare for execution command.  TARGET is the target that will run
    the command.  BACKGROUND determines whether this is a foreground
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 34e9643..098d2ee 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -106,7 +106,7 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_object *bp_obj)
         value_object_to_value (self_finishbp->function_value);
       struct type *value_type =
         type_object_to_type (self_finishbp->return_type);
-      struct value *ret = get_return_value (function, value_type);
+      struct value *ret = get_return_value (function, value_type, NULL);
 
       if (ret)
         {