RE: [PATCHv2 12/13] gdb: introduce unwind-on-timeout setting

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

On Wednesday, January 18, 2023 5:18 PM, Andrew Burgess wrote:
> Now that inferior function calls can timeout (see the recent
> introduction of direct-call-timeout and indirect-call-timeout), this
> commit adds a new setting unwind-on-timeout.
> 
> This new setting is just like the existing unwindonsignal and
> unwind-on-terminating-exception, but the new setting will cause GDB to
> unwind the stack if an inferior function call times out.
> 
> The existing inferior function call timeout tests have been updated to
> cover the new setting.
> ---
>  gdb/NEWS                                      |  9 +++
>  gdb/doc/gdb.texinfo                           | 33 +++++++---
>  gdb/infcall.c                                 | 62 ++++++++++++++++---
>  gdb/testsuite/gdb.base/infcall-timeout.exp    | 49 +++++++++++----
>  .../infcall-from-bp-cond-timeout.exp          | 55 +++++++++++-----
>  5 files changed, 164 insertions(+), 44 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 2689569a8f3..511a750407a 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -35,6 +35,15 @@ show indirect-call-timeout
>    ignored, GDB will wait indefinitely for an inferior function to
>    complete, unless interrupted by the user using Ctrl-C.
> 
> +set unwind-on-timeout on|off
> +show unwind-on-timeout
> +  These commands control whether GDB should unwind the stack when a
> +  timeout occurs during an inferior function call.  The default is
> +  off, in which case the inferior will remain in the frame where the
> +  timeout occurred.  When on GDB will unwind the stack remocing the
> +  dummy frame that was added for the inferior call, and restoring the
> +  inferior state to how it was before the inferior call started.
> +
>  * MI changes
> 
>  ** mi now reports 'no-history' as a stop reason when hitting the end of the
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9eb8b1db00e..aa88221677e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -5719,10 +5719,10 @@
>  the called function, or the called function may receive a signal
>  (e.g.@ a @code{SIGSEGV}) as a result of some undefined behaviour, if
>  this happens then @value{GDBN} will stop.  Depending on the settings
> -@code{unwindonsignal} and @code{unwind-on-terminating-exception}
> -(@pxref{Calling,,Calling Program Functions}) @value{GDBN} may unwind
> -the stack back to the breakpoint location, or may leave the program at
> -the frame where the stop occurred.
> +@code{unwindonsignal}, @code{unwind-on-terminating-exception}, and
> +@code{unwind-on-timeout} (@pxref{Calling,,Calling Program Functions})
> +@value{GDBN} may unwind the stack back to the breakpoint location, or
> +may leave the program at the frame where the stop occurred.
> 
>  Breakpoint conditions can also be evaluated on the target's side if
>  the target supports it.  Instead of evaluating the conditions locally,
> @@ -20718,6 +20718,21 @@
>  Show the current setting of stack unwinding in the functions called by
>  @value{GDBN}.
> 
> +@item set unwind-on-timeout
> +@kindex set unwind-on-timeout
> +@cindex unwind stack in called functions when timing out
> +@cindex call dummy stack unwinding on timeout.
> +Set unwinding of the stack if a function called from @value{GDBN}
> +times out.  If set to @code{off} (the default), @value{GDBN} stops in
> +the frame where the timeout occurred.  If set to @code{on},
> +@value{GDBN} unwinds the stack it created for the call and restores
> +the context to what it was before the call.
> +
> +@item show unwind-on-timeout
> +@kindex show unwind-on-timeout
> +Show whether @value{GDBN} will unwind the stack if a function called
> +from @value{GDBN} times out.
> +
>  @item set may-call-functions
>  @kindex set may-call-functions
>  @cindex disabling calling functions in the program
> @@ -20748,11 +20763,11 @@
>  call by typing the interrupt character (often @kbd{Ctrl-c}).
> 
>  If a called function is interrupted for any reason, and the stack is
> -not unwound (due to @code{set unwind-on-terminating-exception on} or
> -@code{set unwindonsignal on}), then the dummy-frame, created by
> -@value{GDBN} to facilitate the call to the program function, will be
> -visible in the backtrace, for example frame @code{#3} in the following
> -backtrace:
> +not unwound (due to @code{set unwind-on-terminating-exception on},
> +@code{set unwind-on-timeout}, or @code{set unwindonsignal on}), then
> +the dummy-frame, created by @value{GDBN} to facilitate the call to the
> +program function, will be visible in the backtrace, for example frame
> +@code{#3} in the following backtrace:
> 
>  @smallexample
>  (@value{GDBP}) backtrace
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 9afca4f5bc6..272e3cccf87 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -217,6 +217,27 @@ show_unwind_on_terminating_exception_p (struct ui_file *file, int
> from_tty,
>  	      value);
>  }
> 
> +/* This boolean tells what gdb should do if a signal is received while
> +   in a function called from gdb (call dummy).  If set, gdb unwinds

In a former review comment, I was asked to use "GDB" (all uppercase)
as the spelling.

> +   the stack and restore the context to what as it was before the

restore -> restores
And "as" should be removed.

> +   call.
> +
> +   The default is to stop in the frame where the signal was received.  */
> +
> +static bool unwind_on_timeout_p = false;
> +
> +/* Implement 'show unwind-on-timeout'.  */
> +
> +static void
> +show_unwind_on_timeout_p (struct ui_file *file, int from_tty,
> +			  struct cmd_list_element *c, const char *value)
> +{
> +  gdb_printf (file,
> +	      _("Unwinding of stack if a timeout occurs "
> +		"while in a call dummy is %s.\n"),
> +	      value);
> +}
> +
>  /* Perform the standard coercions that are specified
>     for arguments to be passed to C, Ada or Fortran functions.
> 
> @@ -1691,14 +1712,27 @@ When the function is done executing, GDB will silently stop."),
>  	  /* A timeout results in a signal being sent to the inferior.  */
>  	  gdb_assert (stopped_by_random_signal);
> 
> -	  /* Indentation is weird here.  A later patch is going to move the
> -	    following block into an if/else, so I'm leaving the indentation
> -	    here to minimise the later patch.
> +	  if (unwind_on_timeout_p)
> +	    {
> +	      /* The user wants the context restored.  */
> +
> +	      /* We must get back to the frame we were before the
> +		 dummy call.  */
> +	      dummy_frame_pop (dummy_id, call_thread.get ());
> +
> +	      /* We also need to restore inferior status to that before the
> +		 dummy call.  */
> +	      restore_infcall_control_state (inf_status.release ());
> 
> -	    Also, the error message used below refers to 'set
> -	    unwind-on-timeout' which doesn't exist yet.  This will be added
> -	    in a later commit, I'm leaving this in for now to minimise the
> -	    churn caused by the commit that adds unwind-on-timeout.  */
> +	      error (_("\
> +The program being debugged timed out while in a function called from GDB.\n\
> +GDB has restored the context to what it was before the call.\n\
> +To change this behavior use \"set unwind-on-timeout off\".\n\
> +Evaluation of the expression containing the function\n\
> +(%s) will be abandoned."),
> +		     name.c_str ());
> +	    }
> +	  else
>  	    {
>  	      /* The user wants to stay in the frame where we stopped
>  		 (default).  Discard inferior status, we're not at the same
> @@ -1824,6 +1858,20 @@ The default is to unwind the frame."),
>  			   show_unwind_on_terminating_exception_p,
>  			   &setlist, &showlist);
> 
> +  add_setshow_boolean_cmd ("unwind-on-timeout", no_class,
> +			   &unwind_on_timeout_p, _("\
> +Set unwinding of stack if a timeout occurs while in a call dummy."), _("\
> +Show unwinding of stack if a timeout occurs while in a call dummy."),
> +			   _("\
> +The unwind on timeout flag lets the user determine what gdb should do if\n\
> +gdb times out while in a function called from gdb.  If set, gdb unwinds\n\
> +the stack and restores the context to what it was before the call.  If\n\
> +unset, gdb leaves the inferior in the frame where the timeout occurred.\n\
> +The default is to stop in the frame where the timeout occurred."),
> +			   NULL,
> +			   show_unwind_on_timeout_p,
> +			   &setlist, &showlist);
> +
>    add_setshow_uinteger_cmd ("direct-call-timeout", no_class,
>  			    &direct_call_timeout, _("\
>  Set the timeout, for direct calls to inferior function calls."), _("\
> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-
> timeout.exp
> index bd6b2bfac3e..a4d9fd6ee61 100644
> --- a/gdb/testsuite/gdb.base/infcall-timeout.exp
> +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp
> @@ -28,7 +28,11 @@ if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \
>  # the direct-call-timeout, and make an inferior function call that
>  # will never return.  GDB should eventually timeout and stop the
>  # inferior.
> -proc_with_prefix run_test { target_async target_non_stop } {
> +#
> +# When UNWIND is "off" the inferior wil be left in the frame where the
> +# timeout occurs, otherwise, when UNWIND is "on", GDB should unwind
> +# back to the frame where the inferior call was made.
> +proc_with_prefix run_test { target_async target_non_stop unwind } {
>      save_vars { ::GDBFLAGS } {
>  	append ::GDBFLAGS \
>  	    " -ex \"maint set target-non-stop $target_non_stop\""
> @@ -44,19 +48,36 @@ proc_with_prefix run_test { target_async target_non_stop } {
>      }
> 
>      gdb_test_no_output "set direct-call-timeout 5"
> +    gdb_test_no_output "set unwind-on-timeout $unwind"
> +
> +    if { $unwind } {
> +	gdb_test "print function_that_never_returns ()" \
> +	    [multi_line \
> +		 "Program stopped\\." \
> +		 ".*" \
> +		 "The program being debugged timed out while in a function called from GDB\\."
> \
> +		 "GDB has restored the context to what it was before the call\\." \
> +		 "To change this behavior use \"set unwind-on-timeout off\"\\." \
> +		 "Evaluation of the expression containing the function" \
> +		 "\\(function_that_never_returns\\) will be abandoned\\."]
> 
> -    gdb_test "print function_that_never_returns ()" \
> -	[multi_line \
> -	     "Program stopped\\." \
> -	     ".*" \
> -	     "The program being debugged timed out while in a function called from GDB\\." \
> -	     "GDB remains in the frame where the timeout occurred\\." \
> -	     "To change this behavior use \"set unwind-on-timeout on\"\\." \
> -	     "Evaluation of the expression containing the function" \
> -	     "\\(function_that_never_returns\\) will be abandoned\\." \
> -	     "When the function is done executing, GDB will silently stop\\."]
> +	gdb_test "bt" \
> +	    "#0\\s+main \\(\\).*"
> +    } else {
> +	gdb_test "print function_that_never_returns ()" \
> +	    [multi_line \
> +		 "Program stopped\\." \
> +		 ".*" \
> +		 "The program being debugged timed out while in a function called from GDB\\."
> \
> +		 "GDB remains in the frame where the timeout occurred\\." \
> +		 "To change this behavior use \"set unwind-on-timeout on\"\\." \
> +		 "Evaluation of the expression containing the function" \
> +		 "\\(function_that_never_returns\\) will be abandoned\\." \
> +		 "When the function is done executing, GDB will silently stop\\."]
> 
> -    gdb_test "bt" ".* function_that_never_returns .*<function called from gdb>.*"
> +	gdb_test "bt" \
> +	    ".* function_that_never_returns .*<function called from gdb>.*"
> +    }
>  }
> 
>  foreach_with_prefix target_async { "on" "off" } {
> @@ -70,6 +91,8 @@ foreach_with_prefix target_async { "on" "off" } {
>      }
> 
>      foreach_with_prefix target_non_stop { "on" "off" } {
> -	run_test $target_async $target_non_stop
> +	foreach_with_prefix unwind { "on" "off" } {
> +	    run_test $target_async $target_non_stop $unwind
> +	}
>      }
>  }
> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> index 9ba38e6896a..847a850673d 100644
> --- a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> @@ -41,7 +41,12 @@ set segfault_line [gdb_get_line_number "Segfault here"]
>  # thread, on which the inferior call relies, either hits a breakpoint
>  # (when OTHER_THREAD_BP is true), or crashes (when OTHER_THREAD_BP is
>  # false).
> -proc run_test { target_async target_non_stop other_thread_bp } {
> +#
> +# When UNWIND is "on" GDB will unwind the thread which performed the
> +# inferior function call back to the state where the inferior call was
> +# made (when the inferior call times out).  Otherwise, when UNWIND is
> +# "off", the inferior is left in the frame where the timeout occurred.
> +proc run_test { target_async target_non_stop other_thread_bp unwind } {
>      save_vars { ::GDBFLAGS } {
>  	append ::GDBFLAGS " -ex \"maint set target-non-stop $target_non_stop\""
>  	append ::GDBFLAGS " -ex \"maintenance set target-async ${target_async}\""
> @@ -72,6 +77,7 @@ proc run_test { target_async target_non_stop other_thread_bp } {
>      # for this timeout.  For now though, we just hope 5 seconds is
>      # enough.
>      gdb_test_no_output "set indirect-call-timeout 5"
> +    gdb_test_no_output "set unwind-on-timeout $unwind"
> 
>      gdb_breakpoint \
>  	"${::srcfile}:${::cond_bp_line} if (condition_func ())"
> @@ -92,18 +98,35 @@ proc run_test { target_async target_non_stop other_thread_bp } {
>  				 "get number for segfault breakpoint"]
>      }
> 
> -    gdb_test "continue" \
> -	[multi_line \
> -	     "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \
> -	     ".*" \
> -	     "Error in testing condition for breakpoint ${bp_num}:" \
> -	     "The program being debugged timed out while in a function called from GDB\\." \
> -	     "GDB remains in the frame where the timeout occurred\\." \
> -	     "To change this behavior use \"set unwind-on-timeout on\"\\." \
> -	     "Evaluation of the expression containing the function" \
> -	     "\\(condition_func\\) will be abandoned\\." \
> -	     "When the function is done executing, GDB will silently stop\\."] \
> -	"expected timeout waiting for inferior call to complete"
> +    if { $unwind } {
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \
> +		 ".*" \
> +		 "Error in testing condition for breakpoint ${bp_num}:" \
> +		 "The program being debugged timed out while in a function called from GDB\\."
> \
> +		 "GDB has restored the context to what it was before the call\\." \
> +		 "To change this behavior use \"set unwind-on-timeout off\"\\." \
> +		 "Evaluation of the expression containing the function" \
> +		 "\\(condition_func\\) will be abandoned\\." \
> +		 "" \
> +		 "Thread ${::decimal}\[^\r\n\]*hit Breakpoint ${bp_num}, \[^\r\n\]+" \
> +		 "\[^\r\n\]+ Conditional breakpoint here\\. \[^\r\n\]+"] \
> +	    "expected timeout waiting for inferior call to complete"
> +    } else {
> +	gdb_test "continue" \
> +	    [multi_line \
> +		 "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \
> +		 ".*" \
> +		 "Error in testing condition for breakpoint ${bp_num}:" \
> +		 "The program being debugged timed out while in a function called from GDB\\."
> \
> +		 "GDB remains in the frame where the timeout occurred\\." \
> +		 "To change this behavior use \"set unwind-on-timeout on\"\\." \
> +		 "Evaluation of the expression containing the function" \
> +		 "\\(condition_func\\) will be abandoned\\." \
> +		 "When the function is done executing, GDB will silently stop\\."] \
> +	    "expected timeout waiting for inferior call to complete"
> +    }
> 
>      # Remember that other thread that either crashed (with a segfault)
>      # or hit a breakpoint?  Now that the inferior call has timed out,
> @@ -142,8 +165,10 @@ foreach_with_prefix target_async {"on" "off" } {
>      }
> 
>      foreach_with_prefix target_non_stop {"off" "on"} {
> -	foreach_with_prefix other_thread_bp { true false } {
> -	    run_test $target_async $target_non_stop $other_thread_bp
> +	foreach_with_prefix unwind {"off" "on"} {
> +	    foreach_with_prefix other_thread_bp { true false } {
> +		run_test $target_async $target_non_stop $other_thread_bp $unwind
> +	    }
>  	}
>      }
>  }
> --
> 2.25.4

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928