[11/12] gdb: add timeouts for inferior function calls

Message ID a64d956f87b90385cb63cb16e8670c06ce85912e.1666341010.git.aburgess@redhat.com
State New
Headers
Series Infcalls from B/P conditions in multi-threaded inferiors |

Commit Message

Andrew Burgess Oct. 21, 2022, 8:43 a.m. UTC
  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
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                                      |  16 ++
 gdb/doc/gdb.texinfo                           |  45 +++++
 gdb/infcall.c                                 | 159 ++++++++++++++++
 gdb/testsuite/gdb.base/help.exp               |   2 +-
 gdb/testsuite/gdb.base/infcall-timeout.c      |  36 ++++
 gdb/testsuite/gdb.base/infcall-timeout.exp    |  81 +++++++++
 .../infcall-from-bp-cond-timeout.c            | 169 ++++++++++++++++++
 .../infcall-from-bp-cond-timeout.exp          | 128 +++++++++++++
 8 files changed, 635 insertions(+), 1 deletion(-)
 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
  

Comments

Eli Zaretskii Oct. 21, 2022, 11:08 a.m. UTC | #1
> Date: Fri, 21 Oct 2022 09:43:47 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -115,6 +115,22 @@ set debug infcall on|off
>  show debug infcall
>    Print additional debug messages about inferior function calls.
>  
> +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 300 seconds.

IMO, 300 seconds is way too long a wait for a breakpoint condition.
It should be a matter of a few seconds at most, IMO.

> +  These timeouts will only have an effect for targets that are
> +  operating in async mode.

This should tell what happens on targets that don't support async
mode.

> +When calling a function within a program, it is possible that the
> +program could enter a state from which the called function may never
> +return.  If this happens then @value{GDBN} will appear to hang.
> +Should this happen then it is possible to interrupt the running
> +program by typing the interrupt character (often @kbd{Ctrl-c}).
> +
> +On some targets @value{GDBN} can also place a timeout on any function
> +calls made into the program.

Instead of "some targets", which leaves those targets unspecified, we
should say "targets that support async mode", with a cross-reference
to where that is described.  We should give the user a means to
determine whether the particular target does or doesn't need/support
this timeout feature.

>                              If the timeout expires and the function
> +call is still going, then @value{GDBN} will interrupt the program
> +automatically.

And what does this mean for the value returned by the interrupted
call?  This is important for the breakpoint condition use case, for
example.

> +
> +@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}.

Why integer in seconds? don't we want to be able to support shorter
timeouts, like 100 msec?  Most inferior calls should take much less
than a second, so a second resolution is not the best idea, IMO.  It
could, for example, make running a program with such a breakpoint
unbearably slow.

> +This setting only works for targets that support asynchronous
> +execution (@pxref{Background Execution}), for any other target the
> +setting is treated as @code{unlimited}.

This should be moved to the beginning of the description, as mentioned
above.  In addition, saying "treated as 'unlimited'" is not clear
enough in this context, because actually no timeout is applicable at
all, and GDB will wait indefinitely for the call to return.  We should
tell this explicitly.

> +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
> +functions made from a breakpoint condition.
   ^^^^^^^^^^^^^^
"function calls made..."

> +@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 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{300} seconds.

Here 300 seconds is definitely too long.

> +  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, e.g. 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, e.g. when evaluating a conditional breakpoint expression.\n\
> +The timeout is specified in seconds."),

These doc strings explain what is a "direct" vs "indirect" call by way
of "e.g.".  But that leaves the issue not well-defined, because it
begs the question: what are the other cases that are considered
"direct" or "indirect"?
  
Lancelot Six Nov. 4, 2022, 11:17 p.m. UTC | #2
Hi,

> 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.
> 

I do see the use of the indirect call timeouts, and I find it a good
solution for the problem you are trying to solve. I am however not sure
I see much usecase for the direct one.  It looks to me that using Ctrl-C
serves this purpose well already.  Do you have a use case in mind where
this can come in handy?  Scripting and automation maybe?

It seems to me that only having the setting for the indirect call
timeout would make the interface simpler.

That being said, once you have implemented the mechanism for the
"indirect" calls, "direct" call timeout implementation comes for free.
I guess this was your reasoning.

Lancelot.
  
Andrew Burgess Jan. 13, 2023, 4:49 p.m. UTC | #3
Lancelot SIX <lsix@lancelotsix.com> writes:

> Hi,
>
>> 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.
>> 
>
> I do see the use of the indirect call timeouts, and I find it a good
> solution for the problem you are trying to solve. I am however not sure
> I see much usecase for the direct one.  It looks to me that using Ctrl-C
> serves this purpose well already.  Do you have a use case in mind where
> this can come in handy?  Scripting and automation maybe?
>
> It seems to me that only having the setting for the indirect call
> timeout would make the interface simpler.
>
> That being said, once you have implemented the mechanism for the
> "indirect" calls, "direct" call timeout implementation comes for free.
> I guess this was your reasoning.

That was indeed why I provided both - it pretty much came for free.
Like you say, it might offer some benefits in a GDB scripting setup.

If you feel really strongly then I can drop it, but I don't feel it adds
much additional maintenance overhead.

Thanks,
Andrew
  
Andrew Burgess Jan. 14, 2023, 11 a.m. UTC | #4
Eli,

Thanks for all your feedback, sorry it has taken me so long to get back
to this series.  I'm working through most of your points, but I there
was one issue that I wanted to follow up on, see inline below.

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 21 Oct 2022 09:43:47 +0100
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> 
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -115,6 +115,22 @@ set debug infcall on|off
>>  show debug infcall
>>    Print additional debug messages about inferior function calls.
>>  
>> +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 300 seconds.
>
> IMO, 300 seconds is way too long a wait for a breakpoint condition.
> It should be a matter of a few seconds at most, IMO.
>
>> +  These timeouts will only have an effect for targets that are
>> +  operating in async mode.
>
> This should tell what happens on targets that don't support async
> mode.
>
>> +When calling a function within a program, it is possible that the
>> +program could enter a state from which the called function may never
>> +return.  If this happens then @value{GDBN} will appear to hang.
>> +Should this happen then it is possible to interrupt the running
>> +program by typing the interrupt character (often @kbd{Ctrl-c}).
>> +
>> +On some targets @value{GDBN} can also place a timeout on any function
>> +calls made into the program.
>
> Instead of "some targets", which leaves those targets unspecified, we
> should say "targets that support async mode", with a cross-reference
> to where that is described.  We should give the user a means to
> determine whether the particular target does or doesn't need/support
> this timeout feature.
>
>>                              If the timeout expires and the function
>> +call is still going, then @value{GDBN} will interrupt the program
>> +automatically.
>
> And what does this mean for the value returned by the interrupted
> call?  This is important for the breakpoint condition use case, for
> example.
>
>> +
>> +@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}.
>
> Why integer in seconds? don't we want to be able to support shorter
> timeouts, like 100 msec?  Most inferior calls should take much less
> than a second, so a second resolution is not the best idea, IMO.  It
> could, for example, make running a program with such a breakpoint
> unbearably slow.

Remember, this timeout is a safety net intended to catch situations
where either due to a bug in the inferior, or due to user error, the
breakpoint condition is never going to complete.

As such I don't think we should be trying to trim this value down as low
as possible, I just don't see any additional value, in fact, I see doing
so adding more risk that the user will hit invalid timeouts.

If you wanted to be super aggressive with the timeout, and you set it to
1 second, but you still expect your inferior calls to complete in
100msec, then the timeout being 1 second will not slow you down in any
way.  But, if for some reason your inferior call deadlocks, you'll end
up waiting that complete second before GDB gives up.

I can't imagine a use case where a user will be debugging by having an
inferior function call repeatedly timeout, and that's the only way that
having such a large timeout would be unbearable

That all said, the underlying timer mechanism does use msec, so if you
feel strongly that we should be able to have sub-second timeouts, then
it's trivial to switch over, though I do think that the default should
be multiple seconds (I've reduced the default to 30 seconds based on
your other feedback), as I'd like GDB's default behaviour to allow for
debugging slower, remote targets, where that timeout has to allow for
packets sent to the remote target, and slower remote targets.

Thanks,
Andrew

>
>> +This setting only works for targets that support asynchronous
>> +execution (@pxref{Background Execution}), for any other target the
>> +setting is treated as @code{unlimited}.
>
> This should be moved to the beginning of the description, as mentioned
> above.  In addition, saying "treated as 'unlimited'" is not clear
> enough in this context, because actually no timeout is applicable at
> all, and GDB will wait indefinitely for the call to return.  We should
> tell this explicitly.
>
>> +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
>> +functions made from a breakpoint condition.
>    ^^^^^^^^^^^^^^
> "function calls made..."
>
>> +@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 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{300} seconds.
>
> Here 300 seconds is definitely too long.
>
>> +  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, e.g. 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, e.g. when evaluating a conditional breakpoint expression.\n\
>> +The timeout is specified in seconds."),
>
> These doc strings explain what is a "direct" vs "indirect" call by way
> of "e.g.".  But that leaves the issue not well-defined, because it
> begs the question: what are the other cases that are considered
> "direct" or "indirect"?
  
Eli Zaretskii Jan. 14, 2023, 11:48 a.m. UTC | #5
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Sat, 14 Jan 2023 11:00:32 +0000
> 
> >> +@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}.
> >
> > Why integer in seconds? don't we want to be able to support shorter
> > timeouts, like 100 msec?  Most inferior calls should take much less
> > than a second, so a second resolution is not the best idea, IMO.  It
> > could, for example, make running a program with such a breakpoint
> > unbearably slow.
> 
> Remember, this timeout is a safety net intended to catch situations
> where either due to a bug in the inferior, or due to user error, the
> breakpoint condition is never going to complete.
> 
> As such I don't think we should be trying to trim this value down as low
> as possible, I just don't see any additional value, in fact, I see doing
> so adding more risk that the user will hit invalid timeouts.
> 
> If you wanted to be super aggressive with the timeout, and you set it to
> 1 second, but you still expect your inferior calls to complete in
> 100msec, then the timeout being 1 second will not slow you down in any
> way.  But, if for some reason your inferior call deadlocks, you'll end
> up waiting that complete second before GDB gives up.
> 
> I can't imagine a use case where a user will be debugging by having an
> inferior function call repeatedly timeout, and that's the only way that
> having such a large timeout would be unbearable

Suppose we have an inferior call as part of the breakpoint command
conditions, and suppose that breakpoint is hit very frequently.  What
do we want to happen in this case?  Do we want:

  . GDB to silently keep calling the inferior and waiting for it to time out?
  . GDB to announce the timeout and stop the first time it happens?
  . something else?

I thought we wanted the 1st alternative, in which case waiting for a
second could make the run very slow, since an inferior call should
take, like, microseconds in almost all cases?

But if we intend GDB to stop and not continue in this case, then yes,
1 sec could be appropriate.

Or maybe I'm missing something regarding how this feature is supposed
to be used?
  
Lancelot Six Jan. 16, 2023, 9:44 a.m. UTC | #6
Hi,

> > It seems to me that only having the setting for the indirect call
> > timeout would make the interface simpler.
> >
> > That being said, once you have implemented the mechanism for the
> > "indirect" calls, "direct" call timeout implementation comes for free.
> > I guess this was your reasoning.
> 
> That was indeed why I provided both - it pretty much came for free.
> Like you say, it might offer some benefits in a GDB scripting setup.
> 
> If you feel really strongly then I can drop it, but I don't feel it adds
> much additional maintenance overhead.

That's OK with me, as you say the extra maintenance overhead is close to
none.

Best,
Lancelot.
  
Andrew Burgess Jan. 16, 2023, 5:22 p.m. UTC | #7
Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> Date: Sat, 14 Jan 2023 11:00:32 +0000
>> 
>> >> +@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}.
>> >
>> > Why integer in seconds? don't we want to be able to support shorter
>> > timeouts, like 100 msec?  Most inferior calls should take much less
>> > than a second, so a second resolution is not the best idea, IMO.  It
>> > could, for example, make running a program with such a breakpoint
>> > unbearably slow.
>> 
>> Remember, this timeout is a safety net intended to catch situations
>> where either due to a bug in the inferior, or due to user error, the
>> breakpoint condition is never going to complete.
>> 
>> As such I don't think we should be trying to trim this value down as low
>> as possible, I just don't see any additional value, in fact, I see doing
>> so adding more risk that the user will hit invalid timeouts.
>> 
>> If you wanted to be super aggressive with the timeout, and you set it to
>> 1 second, but you still expect your inferior calls to complete in
>> 100msec, then the timeout being 1 second will not slow you down in any
>> way.  But, if for some reason your inferior call deadlocks, you'll end
>> up waiting that complete second before GDB gives up.
>> 
>> I can't imagine a use case where a user will be debugging by having an
>> inferior function call repeatedly timeout, and that's the only way that
>> having such a large timeout would be unbearable
>
> Suppose we have an inferior call as part of the breakpoint command
> conditions, and suppose that breakpoint is hit very frequently.  What
> do we want to happen in this case?  Do we want:
>
>   . GDB to silently keep calling the inferior and waiting for it to time out?
>   . GDB to announce the timeout and stop the first time it happens?
>   . something else?
>
> I thought we wanted the 1st alternative, in which case waiting for a
> second could make the run very slow, since an inferior call should
> take, like, microseconds in almost all cases?
>
> But if we intend GDB to stop and not continue in this case, then yes,
> 1 sec could be appropriate.

I guess my attempt at documenting this is not great :/

We are getting option #2, GDB will stop and report the timeout, here's
how it will look to the user:

  (gdb) set indirect-call-timeout 1
  (gdb) break breakpt()  if (deadlock() == 0)
  Breakpoint 1 at 0x401169: file test.cc, line 35.
  (gdb) r
  Starting program: /tmp/inf-call-timeout/test 
  
  Program stopped.
  0x00007ffff7b3d1e7 in nanosleep () from /lib64/libc.so.6
  Error in testing condition for breakpoint 1:
  timeout waiting for inferior function to complete
  An error occurred while in a function called from GDB.
  Evaluation of the expression containing the function
  (deadlock()) will be abandoned.
  When the function is done executing, GDB will silently stop.
  (gdb) bt
  #0  0x00007ffff7b3d1e7 in nanosleep () from /lib64/libc.so.6
  #1  0x00007ffff7b3d11e in sleep () from /lib64/libc.so.6
  #2  0x000000000040113f in deadlock_inner () at test.cc:13
  #3  0x000000000040114e in deadlock () at test.cc:20
  #4  <function called from gdb>
  #5  breakpt () at test.cc:35
  #6  0x0000000000401175 in main () at test.cc:40
  (gdb)

Here's why I think this is the right behaviour:

If we automatically unwound the stack once the timeout was detected,
then surely the only correct choice would be for the breakpoint
condition to trigger.  We can't say for sure that the condition didn't
trigger, so we should stop.  This is similar to how if I do say:

   (gdb) break foo if (*some_null_pointer == 1234)

Then GDB will immediately stop at this breakpoint with a message saying
that it was unable to access "*some_null_pointer"; the condition caused
a stop not because the condition was true, but because GDB failed to
evaluate the condition.  I think the same logic applies to a timeout in
this case.

However, for the timeout case I've left the context of the timeout on
the stack, in my example above frames #0 to #4 are all related to the
inferior function call.

I think this is useful, if the inferior is somehow deadlocked during the
inferior function call then it might be useful that the user can
investigate the current state and try to figure out what's gone wrong.

Hopefully this explains why I think a minimum 1 second timeout is
acceptable, I wonder, with the explanation above, how you feel about
this now?

I'll take another pass at the documentation, and try to make it clearer,
obviously I didn't do a good job of explaining what I'm trying to do.

Thanks,
Andrew
  
Eli Zaretskii Jan. 16, 2023, 5:27 p.m. UTC | #8
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 16 Jan 2023 17:22:04 +0000
> 
> Hopefully this explains why I think a minimum 1 second timeout is
> acceptable, I wonder, with the explanation above, how you feel about
> this now?

If GDB is going to stop on the first timeout, then I'm okay with the
rest, we just need to make it clear in the manual.

Thanks.
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 8b519a648f7..596f38f5c2f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -115,6 +115,22 @@  set debug infcall on|off
 show debug infcall
   Print additional debug messages about inferior function calls.
 
+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 300 seconds.
+
+  These timeouts will only have an effect for targets that are
+  operating in async mode.
+
 * Changed commands
 
 document user-defined
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ea66f4ee42d..1724bad673e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20630,6 +20630,51 @@ 
 
 @end table
 
+When calling a function within a program, it is possible that the
+program could enter a state from which the called function may never
+return.  If this happens then @value{GDBN} will appear to hang.
+Should this happen then it is possible to interrupt the running
+program by typing the interrupt character (often @kbd{Ctrl-c}).
+
+On some targets @value{GDBN} can also place a timeout on any function
+calls made into the program.  If the timeout expires and the function
+call is still going, then @value{GDBN} will interrupt the program
+automatically.
+
+@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 only works for targets that support asynchronous
+execution (@pxref{Background Execution}), for any other target the
+setting is treated as @code{unlimited}.
+@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
+functions 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 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{300} 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}.
+@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 99ca5792dd3..4eca505250b 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 = 300;
+
+/* 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
@@ -595,6 +642,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
@@ -656,10 +782,19 @@  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 ();
 
+      /* If the timer triggered then the inferior call failed.  */
+      if (infcall_timer.triggered_p ())
+	error (_("timeout waiting for inferior function to complete"));
+
       infcall_debug_printf ("inferior call completed successfully");
     }
   catch (gdb_exception &e)
@@ -1649,6 +1784,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, e.g. 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, e.g. when evaluating a conditional breakpoint expression.\n\
+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 5ee8ce0726d..54b0be32cf8 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.
+
+   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..2bca092aaf8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/infcall-timeout.exp
@@ -0,0 +1,81 @@ 
+# 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" } {
+	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 \
+	     ".*" \
+	     "timeout waiting for inferior function to complete" \
+	     "An error occurred while in a function called from GDB\\." \
+	     "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" } {
+	# 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.
+
+   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..17beba75db0
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
@@ -0,0 +1,128 @@ 
+# Copyright 2020 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.
+
+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"]
+
+proc run_test { other_thread_bp 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
+    }
+
+    # 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.
+    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"]
+
+    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" } {
+	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}:" \
+	     "timeout waiting for inferior function to complete" \
+	     "An error occurred while in a function called from GDB\\." \
+	     "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 { $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" } {
+	# 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 $other_thread_bp $target_async $target_non_stop
+	}
+    }
+}