RE: [PATCHv2 10/13] gdb: add timeouts for inferior function calls

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

On Wednesday, January 18, 2023 5:18 PM, Andrew Burgess wrote:
> 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,
> 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 manor, a

manor -> manner

> stop event in some other thread is going to be ignored when the
> inferior function call is being done 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 300 seconds.  This is still a pretty substantial
> time to be waiting for a single inferior call to complete, but I
> didn't want to be too aggressive with the value I selected.  A user
> can, of course, still use Ctrl-c to interrupt an inferior function
> call, but this limit 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.
> ---
>  gdb/NEWS                                      |  18 ++
>  gdb/doc/gdb.texinfo                           |  66 ++++++
>  gdb/infcall.c                                 | 210 +++++++++++++++++-
>  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, 734 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 c0aac212e30..2689569a8f3 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -17,6 +17,24 @@ maintenance print record-instruction [ N ]
>    prints how GDB would undo the N-th previous instruction, and if N is
>    positive, it prints how GDB will redo the N-th following instruction.
> 
> +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 6ea173b5d0b..9eb8b1db00e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -20772,6 +20772,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 8a48909bc2c..9afca4f5bc6 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -95,6 +95,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
> @@ -589,6 +636,85 @@ 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 (ptid_t ptid, bool direct_call_p)
> +    : m_ptid (ptid)
> +  {
> +    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_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.  */
> +  ptid_t m_ptid;
> +
> +  /* 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_ptid.  */
> +  void
> +  trigger ()
> +  {
> +    m_triggered = true;
> +
> +    scoped_disable_commit_resumed disable_commit_resumed ("infcall timeout");
> +
> +    infcall_debug_printf ("Stopping thread %s",
> +			  m_ptid.to_string ().c_str ());
> +    target_stop (m_ptid);
> +  }
> +};
> +
>  /* 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
> @@ -599,13 +725,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),
> @@ -650,11 +778,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 (inferior_ptid, 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)
>      {
> @@ -1311,6 +1451,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..  */
> @@ -1337,7 +1481,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)
> @@ -1489,7 +1634,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
> @@ -1533,6 +1681,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 ());
>  	    }
> @@ -1646,6 +1824,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..895e8a36d59
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/infcall-timeout.c
> @@ -0,0 +1,36 @@
> +/* Copyright 2022 Free Software Foundation, Inc.

2022-2023?

> +
> +   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..a5b0111ed04
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp
> @@ -0,0 +1,82 @@
> +# Copyright 2022 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"
> +	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 == "off" } {

Use 'eq' instead of '==' for string comparison?
Or maybe even just '!$target_non_stop', since "off" is treated as a boolean false.

> +	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 == "off" } {

Same comment about string comparison here, too.

> +	# 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..3bd91d7377d
> --- /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 Free Software Foundation, Inc.

2022-2023?

> +
> +   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..3341ff33f19
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> @@ -0,0 +1,156 @@
> +# Copyright 2020 Free Software Foundation, Inc.

Please note the copyright year.

> +
> +# 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"
> +	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 do the same interaction

Typo: "does do"

> +    # 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 == "off" } {

String comparison comment applies here, too.

> +	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 == "off" } {

And here, again.

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

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