[PATCH 1/3] Change thread_fsm::return_value to thread_fsm::print_return_values

[PATCH 1/3] Change thread_fsm::return_value to thread_fsm::print_return_values

[Hannes Domani]

A later patch implements print_return_values also for step_command_fsm,
to print the return values of all stepped-over functions.

This keeps all of the return value printing code in infrun.c, so
print_return_value could be made static.
---
 gdb/infcmd.c     | 12 ++++++------
 gdb/infrun.c     | 11 ++---------
 gdb/infrun.h     |  7 -------
 gdb/thread-fsm.h |  6 ++----
 4 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index a27d3577b3a..a5fcf2f8ea5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1575,7 +1575,7 @@ print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv)
    RV points at an object representing the captured return value/type
    and its position in the value history.  */
 
-void
+static void
 print_return_value (struct ui_out *uiout, struct return_value_info *rv)
 {
   if (rv->type == nullptr
@@ -1623,7 +1623,7 @@ struct finish_command_fsm : public thread_fsm
 
   bool should_stop (struct thread_info *thread) override;
   void clean_up (struct thread_info *thread) override;
-  struct return_value_info *return_value () override;
+  void print_return_values (struct ui_out *uiout) override;
   enum async_reply_reason do_async_reply_reason () override;
 };
 
@@ -1684,13 +1684,13 @@ finish_command_fsm::clean_up (struct thread_info *thread)
   delete_longjmp_breakpoint (thread->global_num);
 }
 
-/* Implementation of the 'return_value' FSM method for the finish
+/* Implementation of the 'print_return_values' FSM method for the finish
    commands.  */
 
-struct return_value_info *
-finish_command_fsm::return_value ()
+void
+finish_command_fsm::print_return_values (struct ui_out *uiout)
 {
-  return &return_value_info;
+  print_return_value (uiout, &return_value_info);
 }
 
 /* Implementation of the 'async_reply_reason' FSM method for the
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c67458b30b6..e8ef3677245 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8490,15 +8490,8 @@ print_stop_event (struct ui_out *uiout, bool displays)
   }
 
   tp = inferior_thread ();
-  if (tp->thread_fsm () != nullptr
-      && tp->thread_fsm ()->finished_p ())
-    {
-      struct return_value_info *rv;
-
-      rv = tp->thread_fsm ()->return_value ();
-      if (rv != nullptr)
-	print_return_value (uiout, rv);
-    }
+  if (tp->thread_fsm () != nullptr)
+    tp->thread_fsm ()->print_return_values (uiout);
 }
 
 /* See infrun.h.  */
diff --git a/gdb/infrun.h b/gdb/infrun.h
index c711b9b21cc..79323bcc65f 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -225,13 +225,6 @@ extern void print_exited_reason (struct ui_out *uiout, int exitstatus);
    inferior has stopped.  */
 extern void print_no_history_reason (struct ui_out *uiout);
 
-/* Print the result of a function at the end of a 'finish' command.
-   RV points at an object representing the captured return value/type
-   and its position in the value history.  */
-
-extern void print_return_value (struct ui_out *uiout,
-				struct return_value_info *rv);
-
 /* Print current location without a level number, if we have changed
    functions or hit a breakpoint.  Print source line if we have one.
    If the execution command captured a return value, print it.  If
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
index 96f37ac9414..575b9a5e48f 100644
--- a/gdb/thread-fsm.h
+++ b/gdb/thread-fsm.h
@@ -58,11 +58,9 @@ struct thread_fsm
      function's return value here.  */
   virtual bool should_stop (struct thread_info *thread) = 0;
 
-  /* If this FSM saved a function's return value, you can use this
-     method to retrieve it.  Otherwise, this returns NULL.  */
-  virtual struct return_value_info *return_value ()
+  /* Print return values saved by this FSM.  */
+  virtual void print_return_values (struct ui_out *uiout)
   {
-    return nullptr;
   }
 
   enum async_reply_reason async_reply_reason ()
-- 
2.35.1

[PATCH 2/3] Move return_value_info and refactor code setting its members

[Hannes Domani]

This moves the function and return_buf members from finish_command_fsm
to return_value_info, and refactors the code setting its members
into functions get_callee_info and get_return_value_info.

And all of that is moved before step_command_fsm, since it will use
them in the next patch.
---
 gdb/infcmd.c | 157 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 86 insertions(+), 71 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index a5fcf2f8ea5..9c96d9a39ee 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -800,6 +800,86 @@ nexti_command (const char *count_string, int from_tty)
   step_1 (1, 1, count_string);
 }
 
+/* The captured function return value/type and its position in the
+   value history.  */
+
+struct return_value_info
+{
+  /* The function that we're stepping out of.  */
+  struct symbol *function;
+
+  /* If the current function uses the "struct return convention",
+     this holds the address at which the value being returned will
+     be stored, or zero if that address could not be determined or
+     the "struct return convention" is not being used.  */
+  CORE_ADDR return_buf;
+
+  /* The captured return value.  May be NULL if we weren't able to
+     retrieve it.  See get_return_value.  */
+  struct value *value;
+
+  /* The return type.  In some cases, we'll not be able extract the
+     return value, but we always know the type.  */
+  struct type *type;
+
+  /* If we captured a value, this is the value history index.  */
+  int value_history_index;
+};
+
+/* Get calle function symbol and return convention.  */
+
+static void
+get_callee_info (return_value_info *rv, frame_info_ptr frame)
+{
+  rv->function = find_pc_function (get_frame_pc (frame));
+
+  /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
+     attempt to determine the address of the return buffer.  */
+  if (rv->function != nullptr)
+    {
+      enum return_value_convention return_value;
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+
+      struct type * val_type
+	= check_typedef (rv->function->type ()->target_type ());
+
+      return_value = gdbarch_return_value
+	(gdbarch, read_var_value (rv->function, nullptr, frame),
+	 val_type, nullptr, nullptr, nullptr);
+
+      if (return_value == RETURN_VALUE_STRUCT_CONVENTION
+	  && val_type->code () != TYPE_CODE_VOID)
+	rv->return_buf
+	  = gdbarch_get_return_buf_addr (gdbarch, val_type, frame);
+    }
+}
+
+/* Get return value and add it to value history.  */
+
+static void
+get_return_value_info (return_value_info *rv)
+{
+  rv->type = rv->function->type ()->target_type ();
+  if (rv->type == nullptr)
+    internal_error (_("finish_command: function has no target type"));
+
+  if (check_typedef (rv->type)->code () != TYPE_CODE_VOID)
+    {
+      struct value *func;
+
+      func = read_var_value (rv->function, nullptr, get_current_frame ());
+
+      if (rv->return_buf != 0)
+	/* Retrieve return value from the buffer where it was saved.  */
+	rv->value = value_at (rv->type, rv->return_buf);
+      else
+	rv->value = get_return_value (rv->function, func);
+
+      if (rv->value != nullptr)
+	rv->value_history_index = record_latest_value (rv->value);
+    }
+}
+
 /* Data for the FSM that manages the step/next/stepi/nexti
    commands.  */
 
@@ -1517,23 +1597,6 @@ get_return_value (struct symbol *func_symbol, struct value *function)
   return value;
 }
 
-/* The captured function return value/type and its position in the
-   value history.  */
-
-struct return_value_info
-{
-  /* The captured return value.  May be NULL if we weren't able to
-     retrieve it.  See get_return_value.  */
-  struct value *value;
-
-  /* The return type.  In some cases, we'll not be able extract the
-     return value, but we always know the type.  */
-  struct type *type;
-
-  /* If we captured a value, this is the value history index.  */
-  int value_history_index;
-};
-
 /* Helper for print_return_value.  */
 
 static void
@@ -1603,19 +1666,10 @@ struct finish_command_fsm : public thread_fsm
      the caller.  */
   breakpoint_up breakpoint;
 
-  /* The function that we're stepping out of.  */
-  struct symbol *function = nullptr;
-
   /* If the FSM finishes successfully, this stores the function's
      return value.  */
   struct return_value_info return_value_info {};
 
-  /* If the current function uses the "struct return convention",
-     this holds the address at which the value being returned will
-     be stored, or zero if that address could not be determined or
-     the "struct return convention" is not being used.  */
-  CORE_ADDR return_buf;
-
   explicit finish_command_fsm (struct interp *cmd_interp)
     : thread_fsm (cmd_interp)
   {
@@ -1637,32 +1691,14 @@ finish_command_fsm::should_stop (struct thread_info *tp)
 {
   struct return_value_info *rv = &return_value_info;
 
-  if (function != nullptr
+  if (rv->function != nullptr
       && bpstat_find_breakpoint (tp->control.stop_bpstat,
 				 breakpoint.get ()) != nullptr)
     {
       /* We're done.  */
       set_finished ();
 
-      rv->type = function->type ()->target_type ();
-      if (rv->type == nullptr)
-	internal_error (_("finish_command: function has no target type"));
-
-      if (check_typedef (rv->type)->code () != TYPE_CODE_VOID)
-	{
-	  struct value *func;
-
-	  func = read_var_value (function, nullptr, get_current_frame ());
-
-	  if (return_buf != 0)
-	    /* Retrieve return value from the buffer where it was saved.  */
-	      rv->value = value_at (rv->type, return_buf);
-	  else
-	      rv->value = get_return_value (function, func);
-
-	  if (rv->value != nullptr)
-	    rv->value_history_index = record_latest_value (rv->value);
-	}
+      get_return_value_info (rv);
     }
   else if (tp->control.stop_step)
     {
@@ -1876,29 +1912,7 @@ finish_command (const char *arg, int from_tty)
 
   /* Find the function we will return from.  */
   frame_info_ptr callee_frame = get_selected_frame (nullptr);
-  sm->function = find_pc_function (get_frame_pc (callee_frame));
-  sm->return_buf = 0;    /* Initialize buffer address is not available.  */
-
-  /* Determine the return convention.  If it is RETURN_VALUE_STRUCT_CONVENTION,
-     attempt to determine the address of the return buffer.  */
-  if (sm->function != nullptr)
-    {
-      enum return_value_convention return_value;
-      struct gdbarch *gdbarch = get_frame_arch (callee_frame);
-
-      struct type * val_type
-	= check_typedef (sm->function->type ()->target_type ());
-
-      return_value = gdbarch_return_value (gdbarch,
-					   read_var_value (sm->function, nullptr,
-							   callee_frame),
-					   val_type, nullptr, nullptr, nullptr);
-
-      if (return_value == RETURN_VALUE_STRUCT_CONVENTION
-	  && val_type->code () != TYPE_CODE_VOID)
-	sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type,
-						      callee_frame);
-    }
+  get_callee_info (&sm->return_value_info, callee_frame);
 
   /* Print info on the selected frame, including level number but not
      source.  */
@@ -1908,10 +1922,11 @@ finish_command (const char *arg, int from_tty)
 	gdb_printf (_("Run back to call of "));
       else
 	{
-	  if (sm->function != nullptr && TYPE_NO_RETURN (sm->function->type ())
+	  if (sm->return_value_info.function != nullptr
+	      && TYPE_NO_RETURN (sm->return_value_info.function->type ())
 	      && !query (_("warning: Function %s does not return normally.\n"
 			   "Try to finish anyway? "),
-			 sm->function->print_name ()))
+			 sm->return_value_info.function->print_name ()))
 	    error (_("Not confirmed."));
 	  gdb_printf (_("Run till exit from "));
 	}
-- 
2.35.1

[PATCH 3/3] [RFC] Implement printing of return values of stepped-over functions

[Hannes Domani]

Considering this example (gdb.base/return.c):

(gdb) n
44        printf("in main after func1\n");
(gdb) n
45        tmp2 = func2 ();
(gdb) n
46        tmp3 = func3 ();
(gdb) n
47        printf("exiting\n");
(gdb) n
48        return 0;

When enabling the new functionality:

(gdb) set print step-return-value on
(gdb) n
44        printf("in main after func1\n");
(gdb) n
45        tmp2 = func2 ();
printf returned $1 = 20
(gdb) n
46        tmp3 = func3 ();
func2 returned $2 = -5
(gdb) n
47        printf("exiting\n");
func3 returned $3 = -5
(gdb) n
48        return 0;
printf returned $4 = 8

It shows the return values of all function which were stepped over
with 'next', and puts them into convenience variables, similar to
'finish'.

---------
I wonder if it should always collect the return value and put into the
value history, even if they are not printed, similar to how 'finish' works.

Or maybe make them available from python somehow (plus the function symbols),
so it could be used e.g. in a new special TUI window.
---
 gdb/doc/gdb.texinfo | 11 ++++++
 gdb/infcmd.c        | 91 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/infrun.c        | 10 ++++-
 gdb/thread-fsm.h    | 11 ++++++
 4 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5b566669975..8ef2a74cd5f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6361,6 +6361,17 @@ debug information.  This is the default.
 Show whether @value{GDBN} will stop in or step over functions without
 source line debug information.
 
+@kindex set print step-return-value
+@kindex show print step-return-value
+@item set print step-return-value @r{[}on|off@r{]}
+@itemx show print step-return-value
+Control whether @value{GDBN} will show the return values of functions
+that were stepped over with @code{next}.
+
+If @code{on}, it will print the return values, and enter them into the
+value history (@pxref{Value History}).  If @code{off}, the return values
+are not shown, and not entered into the value history.
+
 @kindex finish
 @kindex fin @r{(@code{finish})}
 @item finish
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9c96d9a39ee..b484da431f7 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -104,6 +104,10 @@ int stopped_by_random_signal;
 
 static bool finish_print = true;
 
+/* Whether "next" should print return values of stepped-over functions.  */
+
+static bool step_return_value_print = false;
+
 \f
 
 static void
@@ -894,6 +898,8 @@ struct step_command_fsm : public thread_fsm
   /* If true, this is a stepi/nexti, otherwise a step/step.  */
   int single_inst;
 
+  std::vector<return_value_info> return_values;
+
   explicit step_command_fsm (struct interp *cmd_interp)
     : thread_fsm (cmd_interp)
   {
@@ -901,6 +907,9 @@ struct step_command_fsm : public thread_fsm
 
   void clean_up (struct thread_info *thread) override;
   bool should_stop (struct thread_info *thread) override;
+  void print_return_values (struct ui_out *uiout) override;
+  void add_callee_info (frame_info_ptr frame) override;
+  void capture_return_value () override;
   enum async_reply_reason do_async_reply_reason () override;
 };
 
@@ -999,6 +1008,67 @@ step_command_fsm::should_stop (struct thread_info *tp)
   return true;
 }
 
+/* Implementation of the 'print_return_values' FSM method for stepping
+   commands.  */
+
+void
+step_command_fsm::print_return_values (struct ui_out *uiout)
+{
+  for (return_value_info &rv : return_values)
+    {
+      if (rv.value == nullptr)
+	continue;
+
+      try
+	{
+	  uiout->field_string ("return-from", rv.function->print_name (),
+			       function_name_style.style ());
+	  uiout->text (" returned ");
+	  uiout->field_fmt ("gdb-result-var", "$%d",
+			    rv.value_history_index);
+	  uiout->text (" = ");
+
+	  struct value_print_options opts;
+	  get_user_print_options (&opts);
+
+	  string_file stb;
+	  value_print (rv.value, &stb, &opts);
+	  uiout->field_stream ("return-value", stb);
+
+	  uiout->text ("\n");
+	}
+      catch (const gdb_exception &ex)
+	{
+	  exception_print (gdb_stdout, ex);
+	}
+    }
+}
+
+/* Implementation of the 'add_callee_info' FSM method for stepping
+   commands.  */
+
+void
+step_command_fsm::add_callee_info (frame_info_ptr frame)
+{
+  if (!step_return_value_print)
+    return;
+
+  return_value_info rv {};
+  get_callee_info (&rv, frame);
+  if (rv.function != nullptr)
+    return_values.push_back (rv);
+}
+
+/* Implementation of the 'capture_return_value' FSM method for stepping
+   commands.  */
+
+void
+step_command_fsm::capture_return_value ()
+{
+  if (!return_values.empty () && return_values.back ().value == nullptr)
+    get_return_value_info (&return_values.back ());
+}
+
 /* Implementation of the 'clean_up' FSM method for stepping commands.  */
 
 void
@@ -3078,6 +3148,18 @@ Printing of return value after `finish' is %s.\n"),
 	      value);
 }
 
+/* Implement `show print step-return-value'.  */
+
+static void
+show_print_step_return_value (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c,
+			      const char *value)
+{
+  gdb_printf (file, _("\
+Printing of return values of stepped-over functions is %s.\n"),
+	      value);
+}
+
 
 /* This help string is used for the run, start, and starti commands.
    It is defined as a macro to prevent duplication.  */
@@ -3423,4 +3505,13 @@ Show whether `finish' prints the return value."), nullptr,
 			   nullptr,
 			   show_print_finish,
 			   &setprintlist, &showprintlist);
+
+  add_setshow_boolean_cmd ("step-return-value", class_support,
+			   &step_return_value_print, _("\
+Set whether `next' prints the return value of stepped-over function."), _("\
+Show whether `next' prints the return value of stepped-over function."),
+			   nullptr,
+			   nullptr,
+			   show_print_step_return_value,
+			   &setprintlist, &showprintlist);
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e8ef3677245..8bb2807edee 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6738,6 +6738,9 @@ process_event_stop_test (struct execution_control_state *ecs)
     case BPSTAT_WHAT_STEP_RESUME:
       infrun_debug_printf ("BPSTAT_WHAT_STEP_RESUME");
 
+      if (ecs->event_thread->thread_fsm () != nullptr)
+	ecs->event_thread->thread_fsm ()->capture_return_value ();
+
       delete_step_resume_breakpoint (ecs->event_thread);
       if (ecs->event_thread->control.proceed_to_finish
 	  && execution_direction == EXEC_REVERSE)
@@ -7103,7 +7106,12 @@ process_event_stop_test (struct execution_control_state *ecs)
 		}
 	    }
 	  else
-	    insert_step_resume_breakpoint_at_caller (frame);
+	    {
+	      insert_step_resume_breakpoint_at_caller (frame);
+
+	      if (ecs->event_thread->thread_fsm () != nullptr)
+		ecs->event_thread->thread_fsm ()->add_callee_info (frame);
+	    }
 
 	  keep_going (ecs);
 	  return;
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
index 575b9a5e48f..dc23be73695 100644
--- a/gdb/thread-fsm.h
+++ b/gdb/thread-fsm.h
@@ -63,6 +63,17 @@ struct thread_fsm
   {
   }
 
+  /* Add callee information (function symbol) for later use for
+     return values.  */
+  virtual void add_callee_info (frame_info_ptr frame)
+  {
+  }
+
+  /* Capture return value of callee which just returned.  */
+  virtual void capture_return_value ()
+  {
+  }
+
   enum async_reply_reason async_reply_reason ()
   {
     /* If we didn't finish, then the stop reason must come from
-- 
2.35.1

Re: [PATCH 3/3] [RFC] Implement printing of return values of stepped-over functions

[Eli Zaretskii]

> Date: Sat, 10 Dec 2022 17:23:26 +0100
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> It shows the return values of all function which were stepped over
> with 'next', and puts them into convenience variables, similar to
> 'finish'.
> 
> ---------
> I wonder if it should always collect the return value and put into the
> value history, even if they are not printed, similar to how 'finish' works.
> 
> Or maybe make them available from python somehow (plus the function symbols),
> so it could be used e.g. in a new special TUI window.
> ---
>  gdb/doc/gdb.texinfo | 11 ++++++
>  gdb/infcmd.c        | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  gdb/infrun.c        | 10 ++++-
>  gdb/thread-fsm.h    | 11 ++++++
>  4 files changed, 122 insertions(+), 1 deletion(-)

OK for the documentation part, thanks.  I think it also warrants a
NEWS entry.