RE: [PATCHv7 3/6] gdb: add timeouts for inferior function calls

[Andrew Burgess <aburgess@redhat.com>]

"Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com> writes:

> On Monday, May 15, 2023 9:22 PM, Andrew Burgess wrote:
>> Eli already approved the docs part:
>>   https://sourceware.org/pipermail/gdb-patches/2023-January/196462.html
>> 
>> ---
>> 
>> In the previous commits I have been working on improving inferior
>> function call support.  One thing that worries me about using inferior
>> function calls from a conditional breakpoint is: what happens if the
>> inferior function call fails?
>> 
>> If the failure is obvious, e.g. the thread performing the call
>> crashes, or hits a breakpoint, then this case is already well handled,
>> and the error is reported to the user.
>> 
>> But what if the thread performing the inferior call just deadlocks?
>> If the user made the call from a 'print' or 'call' command, then the
>> user might have some expectation of when the function call should
>> complete, and, when this time limit is exceeded, the user
>> will (hopefully) interrupt GDB and regain control of the debug
>> session.
>> 
>> But, when the inferior function call is from a breakpoint condition it
>> is much harder to understand that GDB is deadlocked within an inferior
>> call.  Maybe the breakpoint hasn't been hit yet?  Or maybe the
>> condition was always false?  Or maybe GDB is deadlocked in an inferior
>> call?  The only way to know for sure is to periodically interrupt GDB,
>
> Did you mean "interrupt the inferior"?

Yes, that makes more sense.  I think I was trying to say interrupt the
inferior via GDB, but I think mentioning GDB here is not helpful.

I've added your reviewed-by tag.

Thanks,
Andrew

>
>> check on all the threads, and then continue.
>> 
>> Additionally, the focus of the previous commit was inferior function
>> calls, from a conditional breakpoint, in a multi-threaded inferior.
>> This opens up a whole new set of potential failure conditions.  For
>> example, what if the function called relies on interaction with some
>> other thread, and the other thread crashes?  Or hits a breakpoint?
>> Given how inferior function calls work (in a synchronous manner), a
>> stop event in some other thread is going to be ignored while the
>> inferior function call is being executed as part of a breakpoint
>> condition, and this means that GDB could get stuck waiting for the
>> original condition thread, which will now never complete.
>> 
>> In this commit I propose a solution to this problem.  A timeout.  For
>> targets that support async-mode we can install an event-loop timer
>> before starting the inferior function call.  When the timer expires we
>> will stop the thread performing the inferior function call.  With this
>> mechanism in place a user can be sure that any inferior call they make
>> will either complete, or timeout eventually.
>> 
>> Adding a timer like this is obviously a change in behaviour for the
>> more common 'call' and 'print' uses of inferior function calls, so, in
>> this patch, I propose having two different timers.  One I call the
>> 'direct-call-timeout', which is used for 'call' and 'print' commands.
>> This timeout is by default set to unlimited, which, not surprisingly,
>> means there is no timeout in place.
>> 
>> A second timer, which I've called 'indirect-call-timeout', is used for
>> inferior function calls from breakpoint conditions.  This timeout has
>> a default value of 30 seconds.  This is a reasonably long time to
>> wait, and hopefully should be enough in most cases to allow the
>> inferior call to complete.  An inferior call that takes more than 30
>> seconds, which is installed on a breakpoint condition is really going
>> to slow down the debug session, so hopefully this is not a common use
>> case.
>> 
>> The user is, of course, free to reduce, or increase the timeout value,
>> and can always use Ctrl-c to interrupt an inferior function call, but
>> this timeout will ensure that GDB will stop at some point.
>> 
>> The new commands added by this commit are:
>> 
>>   set direct-call-timeout SECONDS
>>   show direct-call-timeout
>>   set indirect-call-timeout SECONDS
>>   show indirect-call-timeout
>> 
>> These new timeouts do depend on async-mode, so, if async-mode is
>> disabled (maint set target-async off), or not supported (e.g. target
>> sim), then the timeout is treated as unlimited (that is, no timeout is
>> set).
>> 
>> For targets that "fake" non-async mode, e.g. Linux native, where
>> non-async mode is really just async mode, but then we park the target
>> in a sissuspend, we could easily fix things so that the timeouts still
>> work, however, for targets that really are not async aware, like the
>> simulator, fixing things so that timeouts work correctly would be a
>> much bigger task - that effort would be better spent just making the
>> target async-aware.  And so, I'm happy for now that this feature will
>> only work on async targets.
>> 
>> The two new show commands will display slightly different text if the
>> current target is a non-async target, which should allow users to
>> understand what's going on.
>> 
>> There's a somewhat random test adjustment needed in gdb.base/help.exp,
>> the test uses a regexp with the apropos command, and expects to find a
>> single result.  Turns out the new settings I added also matched the
>> regexp, which broke the test.  I've updated the regexp a little to
>> exclude my new settings.
>> 
>> In infcall.c you'll notice the thread_info::stop_requested flag being
>> set when a timeout occurs.  This flag setting is not required as part
>> of this commit, but will be needed in a later commit.  However, it
>> seemed like setting this flag fitted better with this commit, which is
>> why the change is added here.
>> ---
>>  gdb/NEWS                                      |  18 ++
>>  gdb/doc/gdb.texinfo                           |  66 ++++++
>>  gdb/infcall.c                                 | 221 +++++++++++++++++-
>>  gdb/testsuite/gdb.base/help.exp               |   2 +-
>>  gdb/testsuite/gdb.base/infcall-timeout.c      |  36 +++
>>  gdb/testsuite/gdb.base/infcall-timeout.exp    |  82 +++++++
>>  .../infcall-from-bp-cond-timeout.c            | 169 ++++++++++++++
>>  .../infcall-from-bp-cond-timeout.exp          | 156 +++++++++++++
>>  8 files changed, 745 insertions(+), 5 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.c
>>  create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.exp
>>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
>>  create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
>> 
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 6aa0d5171f2..216c3a95d09 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -100,6 +100,24 @@ info main
>>     $2 = 1
>>     (gdb) break func if $_shell("some command") == 0
>> 
>> +set direct-call-timeout SECONDS
>> +show direct-call-timeout
>> +set indirect-call-timeout SECONDS
>> +show indirect-call-timeout
>> +  These new settings can be used to limit how long GDB will wait for
>> +  an inferior function call to complete.  The direct timeout is used
>> +  for inferior function calls from e.g. 'call' and 'print' commands,
>> +  while the indirect timeout is used for inferior function calls from
>> +  within a conditional breakpoint expression.
>> +
>> +  The default for the direct timeout is unlimited, while the default
>> +  for the indirect timeout is 30 seconds.
>> +
>> +  These timeouts will only have an effect for targets that are
>> +  operating in async mode.  For non-async targets the timeouts are
>> +  ignored, GDB will wait indefinitely for an inferior function to
>> +  complete, unless interrupted by the user using Ctrl-C.
>> +
>>  * 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 531147f6e6b..54668d812cb 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -20903,6 +20903,72 @@
>>  @code{step}, etc).  In this case, when the inferior finally returns to
>>  the dummy-frame, @value{GDBN} will once again halt the inferior.
>> 
>> +On targets that support asynchronous execution (@pxref{Background
>> +Execution}) @value{GDBN} can place a timeout on any functions called
>> +from @value{GDBN}.  If the timeout expires and the function call is
>> +still ongoing, then @value{GDBN} will interrupt the program.
>> +
>> +For targets that don't support asynchronous execution
>> +(@pxref{Background Execution}) then timeouts for functions called from
>> +@value{GDBN} are not supported, the timeout settings described below
>> +will be treated as @code{unlimited}, meaning @value{GDBN} will wait
>> +indefinitely for function call to complete, unless interrupted by the
>> +user using @kbd{Ctrl-C}.
>> +
>> +@table @code
>> +@item set direct-call-timeout @var{seconds}
>> +@kindex set direct-call-timeout
>> +@cindex timeout for called functions
>> +Set the timeout used when calling functions in the program to
>> +@var{seconds}, which should be an integer greater than zero, or the
>> +special value @code{unlimited}, which indicates no timeout should be
>> +used.  The default for this setting is @code{unlimited}.
>> +
>> +This setting is used when the user calls a function directly from the
>> +command prompt, for example with a @code{call} or @code{print}
>> +command.
>> +
>> +This setting only works for targets that support asynchronous
>> +execution (@pxref{Background Execution}), for any other target the
>> +setting is treated as @code{unlimited}.
>> +
>> +@item show direct-call-timeout
>> +@kindex show direct-call-timeout
>> +@cindex timeout for called functions
>> +Show the timeout used when calling functions in the program with a
>> +@code{call} or @code{print} command.
>> +@end table
>> +
>> +It is also possible to call functions within the program from the
>> +condition of a conditional breakpoint (@pxref{Conditions, ,Break
>> +Conditions}).  A different setting controls the timeout used for
>> +function calls made from a breakpoint condition.
>> +
>> +@table @code
>> +@item set indirect-call-timeout @var{seconds}
>> +@kindex set indirect-call-timeout
>> +@cindex timeout for called functions
>> +Set the timeout used when calling functions in the program from a
>> +breakpoint or watchpoint condition to @var{seconds}, which should be
>> +an integer greater than zero, or the special value @code{unlimited},
>> +which indicates no timeout should be used.  The default for this
>> +setting is @code{30} seconds.
>> +
>> +This setting only works for targets that support asynchronous
>> +execution (@pxref{Background Execution}), for any other target the
>> +setting is treated as @code{unlimited}.
>> +
>> +If a function called from a breakpoint or watchpoint condition times
>> +out, then @value{GDBN} will stop at the point where the timeout
>> +occurred.  The breakpoint condition evaluation will be abandoned.
>> +
>> +@item show indirect-call-timeout
>> +@kindex show indirect-call-timeout
>> +@cindex timeout for called functions
>> +Show the timeout used when calling functions in the program from a
>> +breakpoint or watchpoint condition.
>> +@end table
>> +
>>  @subsection Calling functions with no debug info
>> 
>>  @cindex no debug info functions
>> diff --git a/gdb/infcall.c b/gdb/infcall.c
>> index 49c88add394..dea7dc83062 100644
>> --- a/gdb/infcall.c
>> +++ b/gdb/infcall.c
>> @@ -96,6 +96,53 @@ show_may_call_functions_p (struct ui_file *file, int from_tty,
>>  	      value);
>>  }
>> 
>> +/* A timeout (in seconds) for direct inferior calls.  A direct inferior
>> +   call is one the user triggers from the prompt, e.g. with a 'call' or
>> +   'print' command.  Compare with the definition of indirect calls below.  */
>> +
>> +static unsigned int direct_call_timeout = UINT_MAX;
>> +
>> +/* Implement 'show direct-call-timeout'.  */
>> +
>> +static void
>> +show_direct_call_timeout (struct ui_file *file, int from_tty,
>> +			  struct cmd_list_element *c, const char *value)
>> +{
>> +  if (target_has_execution () && !target_can_async_p ())
>> +    gdb_printf (file, _("Current target does not support async mode, timeout "
>> +			"for direct inferior calls is \"unlimited\".\n"));
>> +  else if (direct_call_timeout == UINT_MAX)
>> +    gdb_printf (file, _("Timeout for direct inferior function calls "
>> +			"is \"unlimited\".\n"));
>> +  else
>> +    gdb_printf (file, _("Timeout for direct inferior function calls "
>> +			"is \"%s seconds\".\n"), value);
>> +}
>> +
>> +/* A timeout (in seconds) for indirect inferior calls.  An indirect inferior
>> +   call is one that originates from within GDB, for example, when
>> +   evaluating an expression for a conditional breakpoint.  Compare with
>> +   the definition of direct calls above.  */
>> +
>> +static unsigned int indirect_call_timeout = 30;
>> +
>> +/* Implement 'show indirect-call-timeout'.  */
>> +
>> +static void
>> +show_indirect_call_timeout (struct ui_file *file, int from_tty,
>> +			  struct cmd_list_element *c, const char *value)
>> +{
>> +  if (target_has_execution () && !target_can_async_p ())
>> +    gdb_printf (file, _("Current target does not support async mode, timeout "
>> +			"for indirect inferior calls is \"unlimited\".\n"));
>> +  else if (indirect_call_timeout == UINT_MAX)
>> +    gdb_printf (file, _("Timeout for indirect inferior function calls "
>> +			"is \"unlimited\".\n"));
>> +  else
>> +    gdb_printf (file, _("Timeout for indirect inferior function calls "
>> +			"is \"%s seconds\".\n"), value);
>> +}
>> +
>>  /* How you should pass arguments to a function depends on whether it
>>     was defined in K&R style or prototype style.  If you define a
>>     function using the K&R syntax that takes a `float' argument, then
>> @@ -590,6 +637,86 @@ call_thread_fsm::should_notify_stop ()
>>    return true;
>>  }
>> 
>> +/* A class to control creation of a timer that will interrupt a thread
>> +   during an inferior call.  */
>> +struct infcall_timer_controller
>> +{
>> +  /* Setup an event-loop timer that will interrupt PTID if the inferior
>> +     call takes too long.  DIRECT_CALL_P is true when this inferior call is
>> +     a result of the user using a 'print' or 'call' command, and false when
>> +     this inferior call is a result of e.g. a conditional breakpoint
>> +     expression, this is used to select which timeout to use.  */
>> +  infcall_timer_controller (thread_info *thr, bool direct_call_p)
>> +    : m_thread (thr)
>> +  {
>> +    unsigned int timeout
>> +      = direct_call_p ? direct_call_timeout : indirect_call_timeout;
>> +    if (timeout < UINT_MAX && target_can_async_p ())
>> +      {
>> +	int ms = timeout * 1000;
>> +	int id = create_timer (ms, infcall_timer_controller::timed_out, this);
>> +	m_timer_id.emplace (id);
>> +	infcall_debug_printf ("Setting up infcall timeout timer for "
>> +			      "ptid %s: %d milliseconds",
>> +			      m_thread->ptid.to_string ().c_str (), ms);
>> +      }
>> +  }
>> +
>> +  /* Destructor.  Ensure that the timer is removed from the event loop.  */
>> +  ~infcall_timer_controller ()
>> +  {
>> +    /* If the timer has already triggered, then it will have already been
>> +       deleted from the event loop.  If the timer has not triggered, then
>> +       delete it now.  */
>> +    if (m_timer_id.has_value () && !m_triggered)
>> +      delete_timer (*m_timer_id);
>> +
>> +    /* Just for clarity, discard the timer id now.  */
>> +    m_timer_id.reset ();
>> +  }
>> +
>> +  /* Return true if there was a timer in place, and the timer triggered,
>> +     otherwise, return false.  */
>> +  bool triggered_p ()
>> +  {
>> +    gdb_assert (!m_triggered || m_timer_id.has_value ());
>> +    return m_triggered;
>> +  }
>> +
>> +private:
>> +  /* The thread we should interrupt.  */
>> +  thread_info *m_thread;
>> +
>> +  /* Set true when the timer is triggered.  */
>> +  bool m_triggered = false;
>> +
>> +  /* Given a value when a timer is in place.  */
>> +  gdb::optional<int> m_timer_id;
>> +
>> +  /* Callback for the timer, forwards to ::trigger below.  */
>> +  static void
>> +  timed_out (gdb_client_data context)
>> +  {
>> +    infcall_timer_controller *ctrl
>> +      = static_cast<infcall_timer_controller *> (context);
>> +    ctrl->trigger ();
>> +  }
>> +
>> +  /* Called when the timer goes off.  Stop thread m_thread.  */
>> +  void
>> +  trigger ()
>> +  {
>> +    m_triggered = true;
>> +
>> +    scoped_disable_commit_resumed disable_commit_resumed ("infcall timeout");
>> +
>> +    infcall_debug_printf ("Stopping thread %s",
>> +			  m_thread->ptid.to_string ().c_str ());
>> +    target_stop (m_thread->ptid);
>> +    m_thread->stop_requested = true;
>> +  }
>> +};
>> +
>>  /* Subroutine of call_function_by_hand to simplify it.
>>     Start up the inferior and wait for it to stop.
>>     Return the exception if there's an error, or an exception with
>> @@ -600,13 +727,15 @@ call_thread_fsm::should_notify_stop ()
>> 
>>  static struct gdb_exception
>>  run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>> -		   struct thread_info *call_thread, CORE_ADDR real_pc)
>> +		   struct thread_info *call_thread, CORE_ADDR real_pc,
>> +		   bool *timed_out_p)
>>  {
>>    INFCALL_SCOPED_DEBUG_ENTER_EXIT;
>> 
>>    struct gdb_exception caught_error;
>>    ptid_t call_thread_ptid = call_thread->ptid;
>>    int was_running = call_thread->state == THREAD_RUNNING;
>> +  *timed_out_p = false;
>> 
>>    infcall_debug_printf ("call function at %s in thread %s, was_running = %d",
>>  			core_addr_to_string (real_pc),
>> @@ -618,6 +747,16 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>>    scoped_restore restore_in_infcall
>>      = make_scoped_restore (&call_thread->control.in_infcall, 1);
>> 
>> +  /* If the thread making the inferior call stops with a time out then the
>
> "time out" -> "timeout"
>
>> +     stop_requested flag will be set.  However, we don't want changes to
>> +     this flag to leak back to our caller, we might be here to handle an
>> +     inferior call from a breakpoint condition, so leaving this flag set
>> +     would appear that the breakpoint stop was actually a requested stop,
>> +     which is not true, and will cause GDB to print extra messages to the
>> +     output.  */
>> +  scoped_restore restore_stop_requested
>> +    = make_scoped_restore (&call_thread->stop_requested, false);
>> +
>>    clear_proceed_status (0);
>> 
>>    /* Associate the FSM with the thread after clear_proceed_status
>> @@ -651,11 +790,23 @@ run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
>>        infrun_debug_show_threads ("non-exited threads after proceed for inferior-call",
>>  				 all_non_exited_threads ());
>> 
>> +      /* Setup a timer (if possible, and if the settings allow) to prevent
>> +	 the inferior call running forever.  */
>> +      bool direct_call_p = !call_thread->control.in_cond_eval;
>> +      infcall_timer_controller infcall_timer (call_thread, direct_call_p);
>> +
>>        /* Inferior function calls are always synchronous, even if the
>>  	 target supports asynchronous execution.  */
>>        wait_sync_command_done ();
>> 
>> -      infcall_debug_printf ("inferior call completed successfully");
>> +      /* If the timer triggered then the inferior call failed.  */
>> +      if (infcall_timer.triggered_p ())
>> +	{
>> +	  infcall_debug_printf ("inferior call timed out");
>> +	  *timed_out_p = true;
>> +	}
>> +      else
>> +	infcall_debug_printf ("inferior call completed successfully");
>>      }
>>    catch (gdb_exception &e)
>>      {
>> @@ -1309,6 +1460,10 @@ call_function_by_hand_dummy (struct value *function,
>>    scoped_restore restore_stopped_by_random_signal
>>      = make_scoped_restore (&stopped_by_random_signal, 0);
>> 
>> +  /* Set to true by the call to run_inferior_call below if the inferior
>> +     call is artificially interrupted by GDB due to taking too long.  */
>> +  bool timed_out_p = false;
>> +
>>    /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
>>       If you're looking to implement asynchronous dummy-frames, then
>>       just below is the place to chop this function in two..  */
>> @@ -1335,7 +1490,8 @@ call_function_by_hand_dummy (struct value *function,
>>  			      struct_addr);
>>      {
>>        std::unique_ptr<call_thread_fsm> sm_up (sm);
>> -      e = run_inferior_call (std::move (sm_up), call_thread.get (), real_pc);
>> +      e = run_inferior_call (std::move (sm_up), call_thread.get (), real_pc,
>> +			     &timed_out_p);
>>      }
>> 
>>      if (e.reason < 0)
>> @@ -1487,7 +1643,10 @@ When the function is done executing, GDB will silently stop."),
>>        std::string name = get_function_name (funaddr, name_buf,
>>  					    sizeof (name_buf));
>> 
>> -      if (stopped_by_random_signal)
>> +      /* If the inferior call timed out then it will have been interrupted
>> +	 by a signal, but we want to report this differently to the user,
>> +	 which is done later in this function.  */
>> +      if (stopped_by_random_signal && !timed_out_p)
>>  	{
>>  	  /* We stopped inside the FUNCTION because of a random
>>  	     signal.  Further execution of the FUNCTION is not
>> @@ -1531,6 +1690,36 @@ GDB remains in the frame where the signal was received.\n\
>>  To change this behavior use \"set unwindonsignal on\".\n\
>>  Evaluation of the expression containing the function\n\
>>  (%s) will be abandoned.\n\
>> +When the function is done executing, GDB will silently stop."),
>> +		     name.c_str ());
>> +	    }
>> +	}
>> +
>> +      if (timed_out_p)
>> +	{
>> +	  /* 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.
>> +
>> +	    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.  */
>> +	    {
>> +	      /* The user wants to stay in the frame where we stopped
>> +		 (default).  Discard inferior status, we're not at the same
>> +		 point we started at.  */
>> +	      discard_infcall_control_state (inf_status.release ());
>> +
>> +	      error (_("\
>> +The program being debugged timed out while in a function called from GDB.\n\
>> +GDB remains in the frame where the timeout occurred.\n\
>> +To change this behavior use \"set unwind-on-timeout on\".\n\
>> +Evaluation of the expression containing the function\n\
>> +(%s) will be abandoned.\n\
>>  When the function is done executing, GDB will silently stop."),
>>  		     name.c_str ());
>>  	    }
>> @@ -1644,6 +1833,30 @@ The default is to unwind the frame."),
>>  			   show_unwind_on_terminating_exception_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."), _("\
>> +Show the timeout, for direct calls to inferior function calls."), _("\
>> +If running on a target that supports, and is running in, async mode\n\
>> +then this timeout is used for any inferior function calls triggered\n\
>> +directly from the prompt, i.e. from a 'call' or 'print' command.  The\n\
>> +timeout is specified in seconds."),
>> +			    nullptr,
>> +			    show_direct_call_timeout,
>> +			    &setlist, &showlist);
>> +
>> +  add_setshow_uinteger_cmd ("indirect-call-timeout", no_class,
>> +			    &indirect_call_timeout, _("\
>> +Set the timeout, for indirect calls to inferior function calls."), _("\
>> +Show the timeout, for indirect calls to inferior function calls."), _("\
>> +If running on a target that supports, and is running in, async mode\n\
>> +then this timeout is used for any inferior function calls triggered\n\
>> +indirectly, i.e. being made as part of a breakpoint, or watchpoint,\n\
>> +condition expression.  The timeout is specified in seconds."),
>> +			    nullptr,
>> +			    show_indirect_call_timeout,
>> +			    &setlist, &showlist);
>> +
>>    add_setshow_boolean_cmd
>>      ("infcall", class_maintenance, &debug_infcall,
>>       _("Set inferior call debugging."),
>> diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp
>> index 87919a819ab..504bf90cc15 100644
>> --- a/gdb/testsuite/gdb.base/help.exp
>> +++ b/gdb/testsuite/gdb.base/help.exp
>> @@ -121,7 +121,7 @@ gdb_test "help info bogus-gdb-command" "Undefined info command: \"bogus-
>> gdb-comm
>>  gdb_test "help gotcha" "Undefined command: \"gotcha\"\.  Try \"help\"\."
>> 
>>  # Test apropos regex.
>> -gdb_test "apropos \\\(print\[\^\[ bsiedf\\\".-\]\\\)" "handle -- Specify how to handle
>> signals\."
>> +gdb_test "apropos \\\(print\[\^\[ bsiedf\\\"'.-\]\\\)" "handle -- Specify how to handle
>> signals\."
>>  # Test apropos >1 word string.
>>  gdb_test "apropos handle signal" "handle -- Specify how to handle signals\."
>>  # Test apropos apropos.
>> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.c b/gdb/testsuite/gdb.base/infcall-
>> timeout.c
>> new file mode 100644
>> index 00000000000..12774ca2599
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/infcall-timeout.c
>> @@ -0,0 +1,36 @@
>> +/* Copyright 2022-2023 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include <unistd.h>
>> +
>> +/* This function is called from GDB.  */
>> +int
>> +function_that_never_returns ()
>> +{
>> +  while (1)
>> +    sleep (1);
>> +
>> +  return 0;
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  alarm (300);
>> +
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-
>> timeout.exp
>> new file mode 100644
>> index 00000000000..5e9cdc2fa0e
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp
>> @@ -0,0 +1,82 @@
>> +# Copyright 2022-2023 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test GDB's direct-call-timeout setting, that is, ensure that if an
>> +# inferior function call, invoked from e.g. a 'print' command, takes
>> +# too long, then GDB can interrupt it, and return control to the user.
>> +
>> +standard_testfile
>> +
>> +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \
>> +	  {debug}] == -1 } {
>> +    return
>> +}
>> +
>> +# Start GDB according to TARGET_ASYNC and TARGET_NON_STOP, then adjust
>> +# 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 } {
>> +    save_vars { ::GDBFLAGS } {
>> +	append ::GDBFLAGS \
>> +	    " -ex \"maint set target-non-stop $target_non_stop\""
>> +	append ::GDBFLAGS \
>> +	    " -ex \"maintenance set target-async ${target_async}\""
>> +
>> +	clean_restart ${::binfile}
>> +    }
>> +
>> +    if {![runto_main]} {
>> +	fail "run to main"
>
> runto_main already emits a fail, so no need for this.
>
>> +	return
>> +    }
>> +
>> +    gdb_test_no_output "set direct-call-timeout 5"
>> +
>> +    # When non-stop mode is off we get slightly different output from GDB.
>> +    if { [gdb_is_remote_or_extended_remote_target] && !$target_non_stop } {
>> +	set stopped_line_pattern "Program received signal SIGINT, Interrupt\\."
>> +    } else {
>> +	set stopped_line_pattern "Program stopped\\."
>> +    }
>> +
>> +    gdb_test "print function_that_never_returns ()" \
>> +	[multi_line \
>> +	     $stopped_line_pattern \
>> +	     ".*" \
>> +	     "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>.*"
>> +}
>> +
>> +foreach_with_prefix target_async { "on" "off" } {
>> +
>> +    if { !$target_async } {
>> +	# GDB can't timeout while waiting for a thread if the target
>> +	# runs with async-mode turned off; once the target is running
>> +	# GDB is effectively blocked until the target stops for some
>> +	# reason.
>> +	continue
>> +    }
>> +
>> +    foreach_with_prefix target_non_stop { "on" "off" } {
>> +	run_test $target_async $target_non_stop
>> +    }
>> +}
>> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
>> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
>> new file mode 100644
>> index 00000000000..4da4245746e
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c
>> @@ -0,0 +1,169 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2022-2023 Free Software Foundation, Inc.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include <stdio.h>
>> +#include <pthread.h>
>> +#include <unistd.h>
>> +#include <stdlib.h>
>> +#include <errno.h>
>> +#include <semaphore.h>
>> +
>> +#define NUM_THREADS 5
>> +
>> +/* Semaphores, used to track when threads have started, and to control
>> +   when the threads finish.  */
>> +sem_t startup_semaphore;
>> +sem_t finish_semaphore;
>> +sem_t thread_1_semaphore;
>> +sem_t thread_2_semaphore;
>> +
>> +/* Mutex to control when the first worker thread hit a breakpoint
>> +   location.  */
>> +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
>> +
>> +/* Global variable to poke, just so threads have something to do.  */
>> +volatile int global_var = 0;
>> +
>> +int
>> +condition_func ()
>> +{
>> +  /* Let thread 2 run.  */
>> +  if (sem_post (&thread_2_semaphore) != 0)
>> +    abort ();
>> +
>> +  /* Wait for thread 2 to complete its actions.  */
>> +  if (sem_wait (&thread_1_semaphore) != 0)
>> +    abort ();
>> +
>> +  return 1;
>> +}
>> +
>> +void
>> +do_segfault ()
>> +{
>> +  volatile int *p = 0;
>> +  *p = 0;	/* Segfault here.  */
>> +}
>> +
>> +void *
>> +worker_func (void *arg)
>> +{
>> +  int tid = *((int *) arg);
>> +
>> +  /* Let the main thread know that this worker has started.  */
>> +  if (sem_post (&startup_semaphore) != 0)
>> +    abort ();
>> +
>> +  switch (tid)
>> +    {
>> +    case 0:
>> +      /* Wait for MUTEX to become available, then pass through the
>> +	 conditional breakpoint location.  */
>> +      if (pthread_mutex_lock (&mutex) != 0)
>> +	abort ();
>> +      global_var = 99;	/* Conditional breakpoint here.  */
>> +      if (pthread_mutex_unlock (&mutex) != 0)
>> +	abort ();
>> +      break;
>> +
>> +    case 1:
>> +      if (sem_wait (&thread_2_semaphore) != 0)
>> +	abort ();
>> +      do_segfault ();
>> +      if (sem_post (&thread_1_semaphore) != 0)
>> +	abort ();
>> +
>> +      /* Fall through.  */
>> +    default:
>> +      /* Wait until we are allowed to finish.  */
>> +      if (sem_wait (&finish_semaphore) != 0)
>> +	abort ();
>> +      break;
>> +    }
>> +}
>> +
>> +void
>> +stop_marker ()
>> +{
>> +  global_var = 99;	/* Stop marker.  */
>> +}
>> +
>> +/* The main program entry point.  */
>> +
>> +int
>> +main ()
>> +{
>> +  pthread_t threads[NUM_THREADS];
>> +  int args[NUM_THREADS];
>> +  void *retval;
>> +
>> +  /* An alarm, just in case the thread deadlocks.  */
>> +  alarm (300);
>> +
>> +  /* Semaphore initialization.  */
>> +  if (sem_init (&startup_semaphore, 0, 0) != 0)
>> +    abort ();
>> +  if (sem_init (&finish_semaphore, 0, 0) != 0)
>> +    abort ();
>> +  if (sem_init (&thread_1_semaphore, 0, 0) != 0)
>> +    abort ();
>> +  if (sem_init (&thread_2_semaphore, 0, 0) != 0)
>> +    abort ();
>> +
>> +  /* Lock MUTEX, this prevents the first worker thread from rushing ahead.  */
>> +  if (pthread_mutex_lock (&mutex) != 0)
>> +    abort ();
>> +
>> +  /* Worker thread creation.  */
>> +  for (int i = 0; i < NUM_THREADS; i++)
>> +    {
>> +      args[i] = i;
>> +      pthread_create (&threads[i], NULL, worker_func, &args[i]);
>> +    }
>> +
>> +  /* Wait for every thread to start.  */
>> +  for (int i = 0; i < NUM_THREADS; i++)
>> +    {
>> +      if (sem_wait (&startup_semaphore) != 0)
>> +	abort ();
>> +    }
>> +
>> +  /* Unlock the first thread so it can proceed.  */
>> +  if (pthread_mutex_unlock (&mutex) != 0)
>> +    abort ();
>> +
>> +  /* Wait for the first thread only.  */
>> +  pthread_join (threads[0], &retval);
>> +
>> +  /* Now post FINISH_SEMAPHORE to allow all the other threads to finish.  */
>> +  for (int i = 1; i < NUM_THREADS; i++)
>> +    sem_post (&finish_semaphore);
>> +
>> +  /* Now wait for the remaining threads to complete.  */
>> +  for (int i = 1; i < NUM_THREADS; i++)
>> +    pthread_join (threads[i], &retval);
>> +
>> +  /* Semaphore cleanup.  */
>> +  sem_destroy (&finish_semaphore);
>> +  sem_destroy (&startup_semaphore);
>> +  sem_destroy (&thread_1_semaphore);
>> +  sem_destroy (&thread_2_semaphore);
>> +
>> +  stop_marker ();
>> +
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
>> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
>> new file mode 100644
>> index 00000000000..4159288a39c
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
>> @@ -0,0 +1,156 @@
>> +# Copyright 2022-2023 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Tests inferior calls executed from a breakpoint condition in
>> +# a multi-threaded program.
>> +#
>> +# This test has the inferior function call timeout, and checks how GDB
>> +# handles this situation.
>> +
>> +standard_testfile
>> +
>> +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \
>> +	  {debug pthreads}] } {
>> +    return
>> +}
>> +
>> +set cond_bp_line [gdb_get_line_number "Conditional breakpoint here"]
>> +set final_bp_line [gdb_get_line_number "Stop marker"]
>> +set segfault_line [gdb_get_line_number "Segfault here"]
>> +
>> +# Setup GDB based on TARGET_ASYNC and TARGET_NON_STOP.  Setup some
>> +# breakpoints in the inferior, one of which has an inferior call
>> +# within its condition.
>> +#
>> +# Continue GDB, the breakpoint with inferior call will be hit, but the
>> +# inferior call will never return.  We expect GDB to timeout.
>> +#
>> +# The reason that the inferior call never completes is that a second
>> +# 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 } {
>> +    save_vars { ::GDBFLAGS } {
>> +	append ::GDBFLAGS " -ex \"maint set target-non-stop $target_non_stop\""
>> +	append ::GDBFLAGS " -ex \"maintenance set target-async ${target_async}\""
>> +
>> +	clean_restart ${::binfile}
>> +    }
>> +
>> +    if {![runto_main]} {
>> +	fail "run to main"
>
> runto_main already emits a fail, so no need for this.
>
>> +	return
>> +    }
>> +
>> +    # The default timeout for indirect inferior calls (e.g. inferior
>> +    # calls for conditional breakpoint expressions) is pretty high.
>> +    # We don't want the test to take too long, so reduce this.
>> +    #
>> +    # However, the test relies on a second thread hitting some event
>> +    # (either a breakpoint or signal) before this timeout expires.
>> +    #
>> +    # There is a chance that on a really slow system this might not
>> +    # happen, in which case the test might fail.
>> +    #
>> +    # However, we still allocate 5 seconds, which feels like it should
>> +    # be enough time in most cases, but maybe we need to do something
>> +    # smarter here?  Possibly we could have some initial run where the
>> +    # inferior doesn't timeout, but does perform the same interaction
>> +    # between threads, we could time that, and use that as the basis
>> +    # for this timeout.  For now though, we just hope 5 seconds is
>> +    # enough.
>> +    gdb_test_no_output "set indirect-call-timeout 5"
>> +
>> +    gdb_breakpoint \
>> +	"${::srcfile}:${::cond_bp_line} if (condition_func ())"
>> +    set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \
>> +		    "get number for conditional breakpoint"]
>> +
>> +    gdb_breakpoint "${::srcfile}:${::final_bp_line}"
>> +    set final_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \
>> +			  "get number for final breakpoint"]
>> +
>> +    # The thread performing an inferior call relies on a second
>> +    # thread.  The second thread will segfault unless it hits a
>> +    # breakpoint first.  In either case the initial thread will not
>> +    # complete its inferior call.
>> +    if { $other_thread_bp } {
>> +	gdb_breakpoint "${::srcfile}:${::segfault_line}"
>> +	set segfault_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \
>> +				 "get number for segfault breakpoint"]
>> +    }
>> +
>> +    # When non-stop mode is off we get slightly different output from GDB.
>> +    if { [gdb_is_remote_or_extended_remote_target] && !$target_non_stop} {
>> +	set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal
>> SIGINT, Interrupt\\."
>> +    } else {
>> +	set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\."
>> +    }
>> +
>> +    gdb_test "continue" \
>> +	[multi_line \
>> +	     $stopped_line_pattern \
>> +	     ".*" \
>> +	     "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,
>> +    # if we try to resume then we should see the pending event from
>> +    # that other thread.
>> +    if { $other_thread_bp } {
>> +	gdb_test "continue" \
>> +	    [multi_line \
>> +		 "Continuing\\." \
>> +		 ".*" \
>> +		 "" \
>> +		 "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${segfault_bp_num},
>> do_segfault \[^\r\n\]+:${::segfault_line}" \
>> +		 "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \
>> +	    "hit the segfault breakpoint"
>> +    } else {
>> +	gdb_test "continue" \
>> +	    [multi_line \
>> +		 "Continuing\\." \
>> +		 ".*" \
>> +		 "Thread ${::decimal} \"infcall-from-bp\" received signal SIGSEGV,
>> Segmentation fault\\." \
>> +		 "\\\[Switching to Thread \[^\r\n\]+\\\]" \
>> +		 "${::hex} in do_segfault \\(\\) at \[^\r\n\]+:${::segfault_line}" \
>> +		 "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \
>> +	    "hit the segfault"
>> +    }
>> +}
>> +
>> +foreach_with_prefix target_async {"on" "off" } {
>> +
>> +    if { !$target_async } {
>> +	# GDB can't timeout while waiting for a thread if the target
>> +	# runs with async-mode turned off; once the target is running
>> +	# GDB is effectively blocked until the target stops for some
>> +	# reason.
>> +	continue
>> +    }
>> +
>> +    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
>> +	}
>> +    }
>> +}
>> --
>> 2.25.4
>
> Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
>
> Thanks.
> -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