* [PATCH 2/3] Move return_value_info and refactor code setting its members
2022-12-10 16:23 ` [PATCH 1/3] Change thread_fsm::return_value to thread_fsm::print_return_values Hannes Domani
@ 2022-12-10 16:23 ` Hannes Domani
2022-12-10 16:23 ` [PATCH 3/3] [RFC] Implement printing of return values of stepped-over functions Hannes Domani
1 sibling, 0 replies; 4+ messages in thread
From: Hannes Domani @ 2022-12-10 16:23 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 3/3] [RFC] Implement printing of return values of stepped-over functions
2022-12-10 16:23 ` [PATCH 1/3] Change thread_fsm::return_value to thread_fsm::print_return_values Hannes Domani
2022-12-10 16:23 ` [PATCH 2/3] Move return_value_info and refactor code setting its members Hannes Domani
@ 2022-12-10 16:23 ` Hannes Domani
2022-12-10 16:33 ` Eli Zaretskii
1 sibling, 1 reply; 4+ messages in thread
From: Hannes Domani @ 2022-12-10 16:23 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread