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

Message ID 20221210162326.854-3-ssbssa@yahoo.de
State New
Headers
Series [1/3] Change thread_fsm::return_value to thread_fsm::print_return_values |

Commit Message

Hannes Domani Dec. 10, 2022, 4:23 p.m. UTC
  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(-)
  

Comments

Eli Zaretskii Dec. 10, 2022, 4:33 p.m. UTC | #1
> 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.
  

Patch

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;
+
 
 
 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