From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A001F3850218 for ; Mon, 5 Jun 2023 13:54:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A001F3850218 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685973277; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zprFVrBwYedUzEhZ5kLhB1B0DJbxezbegAd9sYEYepw=; b=GADTR8yLEy5RjSQ+kNwcoVyNJMhEa/h9Q8PhACGKLgVBgnL32tg1mRBzpKmLbrCao0WxCn fDkBpur5I2GKVjdsYGXOkc5WN8QUtEUYM4ZNbCZrTrheSX7XPQCUFSsWR9gV/vjuWsyT2o tuGaPqGiMZSREETT0Kx47xIaVn0EmlA= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-288-v1WwKemYPhalKg8tNmdcoQ-1; Mon, 05 Jun 2023 09:54:36 -0400 X-MC-Unique: v1WwKemYPhalKg8tNmdcoQ-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3f7e7a6981cso1712675e9.1 for ; Mon, 05 Jun 2023 06:54:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685973274; x=1688565274; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zprFVrBwYedUzEhZ5kLhB1B0DJbxezbegAd9sYEYepw=; b=QObK2gHYxQ/gq/KmnwNKv86px4Fpo9RD/7Myymt0q3eM2n/AIOLxN+KXTNchoKTL/J NLj3QcC00POZwjpQLbqsnjmr5rFfUFfxxqFLBJsbnZ3VzwqSdLHNM7IAE+6cM0Lc8l2D D0FSzrTkPWbd4MC+/Yf1ajuntfZhPOu1RYUYaWnyAzt3rTMHFY1HYRaNNLF3WknElZR/ 4XocxgUfSYezHfohBfTQfgJm4MtHrJYlu1SB2Gg6mQpGotHHKFr1rwimvYHUhNBXHJZR jZFIskhXqBL6YnENOtME1dSCicRRAoYNzHxKAyQH7tFqZeoyh2zeC0U4XMZsIh5A5ocP Cf6A== X-Gm-Message-State: AC+VfDxwjgaHxgxnKlleDON8ZZHEyEMXKaQK61rt1hVSj0kYdFCaOH2X vQ1J5pb+PosE3nciDZrFd+4C4G/tzvnolGltEkmPQt0xa1UmmyMjYqywauUElnZ8gKyc3jnvKIo NvM9LNh1DKdq2piRhE5KEOiaPVdohWw== X-Received: by 2002:a5d:4b4d:0:b0:309:32d1:59d3 with SMTP id w13-20020a5d4b4d000000b0030932d159d3mr4667484wrs.48.1685973274032; Mon, 05 Jun 2023 06:54:34 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5NaaX6PRH6elWuX+pdNMuZ9Iw2YPGSQe53bbd2jLC1tJKkhpwptqUYV1jwhO8H6BpOPzRL8A== X-Received: by 2002:a5d:4b4d:0:b0:309:32d1:59d3 with SMTP id w13-20020a5d4b4d000000b0030932d159d3mr4667464wrs.48.1685973273325; Mon, 05 Jun 2023 06:54:33 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id g7-20020a5d5407000000b0030903371ef9sm9838607wrv.22.2023.06.05.06.54.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Jun 2023 06:54:32 -0700 (PDT) From: Andrew Burgess To: "Aktemur, Tankut Baris" , "gdb-patches@sourceware.org" Subject: RE: [PATCHv7 3/6] gdb: add timeouts for inferior function calls In-Reply-To: References: <76da2ce247b046aaf197b034392b3b60858e2937.1684178293.git.aburgess@redhat.com> Date: Mon, 05 Jun 2023 14:54:32 +0100 Message-ID: <8735363tgn.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: "Aktemur, Tankut Baris" writes: > On Monday, May 15, 2023 9:22 PM, Andrew Burgess wrote: >> Eli already approved the docs part: >> https://sourceware.org/pipermail/gdb-patches/2023-January/196462.html >> >> --- >> >> In the previous commits I have been working on improving inferior >> function call support. One thing that worries me about using inferior >> function calls from a conditional breakpoint is: what happens if the >> inferior function call fails? >> >> If the failure is obvious, e.g. the thread performing the call >> crashes, or hits a breakpoint, then this case is already well handled, >> and the error is reported to the user. >> >> But what if the thread performing the inferior call just deadlocks? >> If the user made the call from a 'print' or 'call' command, then the >> user might have some expectation of when the function call should >> complete, and, when this time limit is exceeded, the user >> will (hopefully) interrupt GDB and regain control of the debug >> session. >> >> But, when the inferior function call is from a breakpoint condition it >> is much harder to understand that GDB is deadlocked within an inferior >> call. Maybe the breakpoint hasn't been hit yet? Or maybe the >> condition was always false? Or maybe GDB is deadlocked in an inferior >> call? The only way to know for sure is to periodically interrupt GDB, > > Did you mean "interrupt the inferior"? Yes, that makes more sense. I think I was trying to say interrupt the inferior via GDB, but I think mentioning GDB here is not helpful. I've added your reviewed-by tag. Thanks, Andrew > >> check on all the threads, and then continue. >> >> Additionally, the focus of the previous commit was inferior function >> calls, from a conditional breakpoint, in a multi-threaded inferior. >> This opens up a whole new set of potential failure conditions. For >> example, what if the function called relies on interaction with some >> other thread, and the other thread crashes? Or hits a breakpoint? >> Given how inferior function calls work (in a synchronous manner), a >> stop event in some other thread is going to be ignored while the >> inferior function call is being executed as part of a breakpoint >> condition, and this means that GDB could get stuck waiting for the >> original condition thread, which will now never complete. >> >> In this commit I propose a solution to this problem. A timeout. For >> targets that support async-mode we can install an event-loop timer >> before starting the inferior function call. When the timer expires we >> will stop the thread performing the inferior function call. With this >> mechanism in place a user can be sure that any inferior call they make >> will either complete, or timeout eventually. >> >> Adding a timer like this is obviously a change in behaviour for the >> more common 'call' and 'print' uses of inferior function calls, so, in >> this patch, I propose having two different timers. One I call the >> 'direct-call-timeout', which is used for 'call' and 'print' commands. >> This timeout is by default set to unlimited, which, not surprisingly, >> means there is no timeout in place. >> >> A second timer, which I've called 'indirect-call-timeout', is used for >> inferior function calls from breakpoint conditions. This timeout has >> a default value of 30 seconds. This is a reasonably long time to >> wait, and hopefully should be enough in most cases to allow the >> inferior call to complete. An inferior call that takes more than 30 >> seconds, which is installed on a breakpoint condition is really going >> to slow down the debug session, so hopefully this is not a common use >> case. >> >> The user is, of course, free to reduce, or increase the timeout value, >> and can always use Ctrl-c to interrupt an inferior function call, but >> this timeout will ensure that GDB will stop at some point. >> >> The new commands added by this commit are: >> >> set direct-call-timeout SECONDS >> show direct-call-timeout >> set indirect-call-timeout SECONDS >> show indirect-call-timeout >> >> These new timeouts do depend on async-mode, so, if async-mode is >> disabled (maint set target-async off), or not supported (e.g. target >> sim), then the timeout is treated as unlimited (that is, no timeout is >> set). >> >> For targets that "fake" non-async mode, e.g. Linux native, where >> non-async mode is really just async mode, but then we park the target >> in a sissuspend, we could easily fix things so that the timeouts still >> work, however, for targets that really are not async aware, like the >> simulator, fixing things so that timeouts work correctly would be a >> much bigger task - that effort would be better spent just making the >> target async-aware. And so, I'm happy for now that this feature will >> only work on async targets. >> >> The two new show commands will display slightly different text if the >> current target is a non-async target, which should allow users to >> understand what's going on. >> >> There's a somewhat random test adjustment needed in gdb.base/help.exp, >> the test uses a regexp with the apropos command, and expects to find a >> single result. Turns out the new settings I added also matched the >> regexp, which broke the test. I've updated the regexp a little to >> exclude my new settings. >> >> In infcall.c you'll notice the thread_info::stop_requested flag being >> set when a timeout occurs. This flag setting is not required as part >> of this commit, but will be needed in a later commit. However, it >> seemed like setting this flag fitted better with this commit, which is >> why the change is added here. >> --- >> gdb/NEWS | 18 ++ >> gdb/doc/gdb.texinfo | 66 ++++++ >> gdb/infcall.c | 221 +++++++++++++++++- >> gdb/testsuite/gdb.base/help.exp | 2 +- >> gdb/testsuite/gdb.base/infcall-timeout.c | 36 +++ >> gdb/testsuite/gdb.base/infcall-timeout.exp | 82 +++++++ >> .../infcall-from-bp-cond-timeout.c | 169 ++++++++++++++ >> .../infcall-from-bp-cond-timeout.exp | 156 +++++++++++++ >> 8 files changed, 745 insertions(+), 5 deletions(-) >> create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.c >> create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.exp >> create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c >> create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp >> >> diff --git a/gdb/NEWS b/gdb/NEWS >> index 6aa0d5171f2..216c3a95d09 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -100,6 +100,24 @@ info main >> $2 = 1 >> (gdb) break func if $_shell("some command") == 0 >> >> +set direct-call-timeout SECONDS >> +show direct-call-timeout >> +set indirect-call-timeout SECONDS >> +show indirect-call-timeout >> + These new settings can be used to limit how long GDB will wait for >> + an inferior function call to complete. The direct timeout is used >> + for inferior function calls from e.g. 'call' and 'print' commands, >> + while the indirect timeout is used for inferior function calls from >> + within a conditional breakpoint expression. >> + >> + The default for the direct timeout is unlimited, while the default >> + for the indirect timeout is 30 seconds. >> + >> + These timeouts will only have an effect for targets that are >> + operating in async mode. For non-async targets the timeouts are >> + ignored, GDB will wait indefinitely for an inferior function to >> + complete, unless interrupted by the user using Ctrl-C. >> + >> * MI changes >> >> ** mi now reports 'no-history' as a stop reason when hitting the end of the >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >> index 531147f6e6b..54668d812cb 100644 >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -20903,6 +20903,72 @@ >> @code{step}, etc). In this case, when the inferior finally returns to >> the dummy-frame, @value{GDBN} will once again halt the inferior. >> >> +On targets that support asynchronous execution (@pxref{Background >> +Execution}) @value{GDBN} can place a timeout on any functions called >> +from @value{GDBN}. If the timeout expires and the function call is >> +still ongoing, then @value{GDBN} will interrupt the program. >> + >> +For targets that don't support asynchronous execution >> +(@pxref{Background Execution}) then timeouts for functions called from >> +@value{GDBN} are not supported, the timeout settings described below >> +will be treated as @code{unlimited}, meaning @value{GDBN} will wait >> +indefinitely for function call to complete, unless interrupted by the >> +user using @kbd{Ctrl-C}. >> + >> +@table @code >> +@item set direct-call-timeout @var{seconds} >> +@kindex set direct-call-timeout >> +@cindex timeout for called functions >> +Set the timeout used when calling functions in the program to >> +@var{seconds}, which should be an integer greater than zero, or the >> +special value @code{unlimited}, which indicates no timeout should be >> +used. The default for this setting is @code{unlimited}. >> + >> +This setting is used when the user calls a function directly from the >> +command prompt, for example with a @code{call} or @code{print} >> +command. >> + >> +This setting only works for targets that support asynchronous >> +execution (@pxref{Background Execution}), for any other target the >> +setting is treated as @code{unlimited}. >> + >> +@item show direct-call-timeout >> +@kindex show direct-call-timeout >> +@cindex timeout for called functions >> +Show the timeout used when calling functions in the program with a >> +@code{call} or @code{print} command. >> +@end table >> + >> +It is also possible to call functions within the program from the >> +condition of a conditional breakpoint (@pxref{Conditions, ,Break >> +Conditions}). A different setting controls the timeout used for >> +function calls made from a breakpoint condition. >> + >> +@table @code >> +@item set indirect-call-timeout @var{seconds} >> +@kindex set indirect-call-timeout >> +@cindex timeout for called functions >> +Set the timeout used when calling functions in the program from a >> +breakpoint or watchpoint condition to @var{seconds}, which should be >> +an integer greater than zero, or the special value @code{unlimited}, >> +which indicates no timeout should be used. The default for this >> +setting is @code{30} seconds. >> + >> +This setting only works for targets that support asynchronous >> +execution (@pxref{Background Execution}), for any other target the >> +setting is treated as @code{unlimited}. >> + >> +If a function called from a breakpoint or watchpoint condition times >> +out, then @value{GDBN} will stop at the point where the timeout >> +occurred. The breakpoint condition evaluation will be abandoned. >> + >> +@item show indirect-call-timeout >> +@kindex show indirect-call-timeout >> +@cindex timeout for called functions >> +Show the timeout used when calling functions in the program from a >> +breakpoint or watchpoint condition. >> +@end table >> + >> @subsection Calling functions with no debug info >> >> @cindex no debug info functions >> diff --git a/gdb/infcall.c b/gdb/infcall.c >> index 49c88add394..dea7dc83062 100644 >> --- a/gdb/infcall.c >> +++ b/gdb/infcall.c >> @@ -96,6 +96,53 @@ show_may_call_functions_p (struct ui_file *file, int from_tty, >> value); >> } >> >> +/* A timeout (in seconds) for direct inferior calls. A direct inferior >> + call is one the user triggers from the prompt, e.g. with a 'call' or >> + 'print' command. Compare with the definition of indirect calls below. */ >> + >> +static unsigned int direct_call_timeout = UINT_MAX; >> + >> +/* Implement 'show direct-call-timeout'. */ >> + >> +static void >> +show_direct_call_timeout (struct ui_file *file, int from_tty, >> + struct cmd_list_element *c, const char *value) >> +{ >> + if (target_has_execution () && !target_can_async_p ()) >> + gdb_printf (file, _("Current target does not support async mode, timeout " >> + "for direct inferior calls is \"unlimited\".\n")); >> + else if (direct_call_timeout == UINT_MAX) >> + gdb_printf (file, _("Timeout for direct inferior function calls " >> + "is \"unlimited\".\n")); >> + else >> + gdb_printf (file, _("Timeout for direct inferior function calls " >> + "is \"%s seconds\".\n"), value); >> +} >> + >> +/* A timeout (in seconds) for indirect inferior calls. An indirect inferior >> + call is one that originates from within GDB, for example, when >> + evaluating an expression for a conditional breakpoint. Compare with >> + the definition of direct calls above. */ >> + >> +static unsigned int indirect_call_timeout = 30; >> + >> +/* Implement 'show indirect-call-timeout'. */ >> + >> +static void >> +show_indirect_call_timeout (struct ui_file *file, int from_tty, >> + struct cmd_list_element *c, const char *value) >> +{ >> + if (target_has_execution () && !target_can_async_p ()) >> + gdb_printf (file, _("Current target does not support async mode, timeout " >> + "for indirect inferior calls is \"unlimited\".\n")); >> + else if (indirect_call_timeout == UINT_MAX) >> + gdb_printf (file, _("Timeout for indirect inferior function calls " >> + "is \"unlimited\".\n")); >> + else >> + gdb_printf (file, _("Timeout for indirect inferior function calls " >> + "is \"%s seconds\".\n"), value); >> +} >> + >> /* How you should pass arguments to a function depends on whether it >> was defined in K&R style or prototype style. If you define a >> function using the K&R syntax that takes a `float' argument, then >> @@ -590,6 +637,86 @@ call_thread_fsm::should_notify_stop () >> return true; >> } >> >> +/* A class to control creation of a timer that will interrupt a thread >> + during an inferior call. */ >> +struct infcall_timer_controller >> +{ >> + /* Setup an event-loop timer that will interrupt PTID if the inferior >> + call takes too long. DIRECT_CALL_P is true when this inferior call is >> + a result of the user using a 'print' or 'call' command, and false when >> + this inferior call is a result of e.g. a conditional breakpoint >> + expression, this is used to select which timeout to use. */ >> + infcall_timer_controller (thread_info *thr, bool direct_call_p) >> + : m_thread (thr) >> + { >> + unsigned int timeout >> + = direct_call_p ? direct_call_timeout : indirect_call_timeout; >> + if (timeout < UINT_MAX && target_can_async_p ()) >> + { >> + int ms = timeout * 1000; >> + int id = create_timer (ms, infcall_timer_controller::timed_out, this); >> + m_timer_id.emplace (id); >> + infcall_debug_printf ("Setting up infcall timeout timer for " >> + "ptid %s: %d milliseconds", >> + m_thread->ptid.to_string ().c_str (), ms); >> + } >> + } >> + >> + /* Destructor. Ensure that the timer is removed from the event loop. */ >> + ~infcall_timer_controller () >> + { >> + /* If the timer has already triggered, then it will have already been >> + deleted from the event loop. If the timer has not triggered, then >> + delete it now. */ >> + if (m_timer_id.has_value () && !m_triggered) >> + delete_timer (*m_timer_id); >> + >> + /* Just for clarity, discard the timer id now. */ >> + m_timer_id.reset (); >> + } >> + >> + /* Return true if there was a timer in place, and the timer triggered, >> + otherwise, return false. */ >> + bool triggered_p () >> + { >> + gdb_assert (!m_triggered || m_timer_id.has_value ()); >> + return m_triggered; >> + } >> + >> +private: >> + /* The thread we should interrupt. */ >> + thread_info *m_thread; >> + >> + /* Set true when the timer is triggered. */ >> + bool m_triggered = false; >> + >> + /* Given a value when a timer is in place. */ >> + gdb::optional 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 (context); >> + ctrl->trigger (); >> + } >> + >> + /* Called when the timer goes off. Stop thread m_thread. */ >> + void >> + trigger () >> + { >> + m_triggered = true; >> + >> + scoped_disable_commit_resumed disable_commit_resumed ("infcall timeout"); >> + >> + infcall_debug_printf ("Stopping thread %s", >> + m_thread->ptid.to_string ().c_str ()); >> + target_stop (m_thread->ptid); >> + m_thread->stop_requested = true; >> + } >> +}; >> + >> /* Subroutine of call_function_by_hand to simplify it. >> Start up the inferior and wait for it to stop. >> Return the exception if there's an error, or an exception with >> @@ -600,13 +727,15 @@ call_thread_fsm::should_notify_stop () >> >> static struct gdb_exception >> run_inferior_call (std::unique_ptr sm, >> - struct thread_info *call_thread, CORE_ADDR real_pc) >> + struct thread_info *call_thread, CORE_ADDR real_pc, >> + bool *timed_out_p) >> { >> INFCALL_SCOPED_DEBUG_ENTER_EXIT; >> >> struct gdb_exception caught_error; >> ptid_t call_thread_ptid = call_thread->ptid; >> int was_running = call_thread->state == THREAD_RUNNING; >> + *timed_out_p = false; >> >> infcall_debug_printf ("call function at %s in thread %s, was_running = %d", >> core_addr_to_string (real_pc), >> @@ -618,6 +747,16 @@ run_inferior_call (std::unique_ptr sm, >> scoped_restore restore_in_infcall >> = make_scoped_restore (&call_thread->control.in_infcall, 1); >> >> + /* If the thread making the inferior call stops with a time out then the > > "time out" -> "timeout" > >> + stop_requested flag will be set. However, we don't want changes to >> + this flag to leak back to our caller, we might be here to handle an >> + inferior call from a breakpoint condition, so leaving this flag set >> + would appear that the breakpoint stop was actually a requested stop, >> + which is not true, and will cause GDB to print extra messages to the >> + output. */ >> + scoped_restore restore_stop_requested >> + = make_scoped_restore (&call_thread->stop_requested, false); >> + >> clear_proceed_status (0); >> >> /* Associate the FSM with the thread after clear_proceed_status >> @@ -651,11 +790,23 @@ run_inferior_call (std::unique_ptr sm, >> infrun_debug_show_threads ("non-exited threads after proceed for inferior-call", >> all_non_exited_threads ()); >> >> + /* Setup a timer (if possible, and if the settings allow) to prevent >> + the inferior call running forever. */ >> + bool direct_call_p = !call_thread->control.in_cond_eval; >> + infcall_timer_controller infcall_timer (call_thread, direct_call_p); >> + >> /* Inferior function calls are always synchronous, even if the >> target supports asynchronous execution. */ >> wait_sync_command_done (); >> >> - infcall_debug_printf ("inferior call completed successfully"); >> + /* If the timer triggered then the inferior call failed. */ >> + if (infcall_timer.triggered_p ()) >> + { >> + infcall_debug_printf ("inferior call timed out"); >> + *timed_out_p = true; >> + } >> + else >> + infcall_debug_printf ("inferior call completed successfully"); >> } >> catch (gdb_exception &e) >> { >> @@ -1309,6 +1460,10 @@ call_function_by_hand_dummy (struct value *function, >> scoped_restore restore_stopped_by_random_signal >> = make_scoped_restore (&stopped_by_random_signal, 0); >> >> + /* Set to true by the call to run_inferior_call below if the inferior >> + call is artificially interrupted by GDB due to taking too long. */ >> + bool timed_out_p = false; >> + >> /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - >> If you're looking to implement asynchronous dummy-frames, then >> just below is the place to chop this function in two.. */ >> @@ -1335,7 +1490,8 @@ call_function_by_hand_dummy (struct value *function, >> struct_addr); >> { >> std::unique_ptr sm_up (sm); >> - e = run_inferior_call (std::move (sm_up), call_thread.get (), real_pc); >> + e = run_inferior_call (std::move (sm_up), call_thread.get (), real_pc, >> + &timed_out_p); >> } >> >> if (e.reason < 0) >> @@ -1487,7 +1643,10 @@ When the function is done executing, GDB will silently stop."), >> std::string name = get_function_name (funaddr, name_buf, >> sizeof (name_buf)); >> >> - if (stopped_by_random_signal) >> + /* If the inferior call timed out then it will have been interrupted >> + by a signal, but we want to report this differently to the user, >> + which is done later in this function. */ >> + if (stopped_by_random_signal && !timed_out_p) >> { >> /* We stopped inside the FUNCTION because of a random >> signal. Further execution of the FUNCTION is not >> @@ -1531,6 +1690,36 @@ GDB remains in the frame where the signal was received.\n\ >> To change this behavior use \"set unwindonsignal on\".\n\ >> Evaluation of the expression containing the function\n\ >> (%s) will be abandoned.\n\ >> +When the function is done executing, GDB will silently stop."), >> + name.c_str ()); >> + } >> + } >> + >> + if (timed_out_p) >> + { >> + /* A timeout results in a signal being sent to the inferior. */ >> + gdb_assert (stopped_by_random_signal); >> + >> + /* Indentation is weird here. A later patch is going to move the >> + following block into an if/else, so I'm leaving the indentation >> + here to minimise the later patch. >> + >> + Also, the error message used below refers to 'set >> + unwind-on-timeout' which doesn't exist yet. This will be added >> + in a later commit, I'm leaving this in for now to minimise the >> + churn caused by the commit that adds unwind-on-timeout. */ >> + { >> + /* The user wants to stay in the frame where we stopped >> + (default). Discard inferior status, we're not at the same >> + point we started at. */ >> + discard_infcall_control_state (inf_status.release ()); >> + >> + error (_("\ >> +The program being debugged timed out while in a function called from GDB.\n\ >> +GDB remains in the frame where the timeout occurred.\n\ >> +To change this behavior use \"set unwind-on-timeout on\".\n\ >> +Evaluation of the expression containing the function\n\ >> +(%s) will be abandoned.\n\ >> When the function is done executing, GDB will silently stop."), >> name.c_str ()); >> } >> @@ -1644,6 +1833,30 @@ The default is to unwind the frame."), >> show_unwind_on_terminating_exception_p, >> &setlist, &showlist); >> >> + add_setshow_uinteger_cmd ("direct-call-timeout", no_class, >> + &direct_call_timeout, _("\ >> +Set the timeout, for direct calls to inferior function calls."), _("\ >> +Show the timeout, for direct calls to inferior function calls."), _("\ >> +If running on a target that supports, and is running in, async mode\n\ >> +then this timeout is used for any inferior function calls triggered\n\ >> +directly from the prompt, i.e. from a 'call' or 'print' command. The\n\ >> +timeout is specified in seconds."), >> + nullptr, >> + show_direct_call_timeout, >> + &setlist, &showlist); >> + >> + add_setshow_uinteger_cmd ("indirect-call-timeout", no_class, >> + &indirect_call_timeout, _("\ >> +Set the timeout, for indirect calls to inferior function calls."), _("\ >> +Show the timeout, for indirect calls to inferior function calls."), _("\ >> +If running on a target that supports, and is running in, async mode\n\ >> +then this timeout is used for any inferior function calls triggered\n\ >> +indirectly, i.e. being made as part of a breakpoint, or watchpoint,\n\ >> +condition expression. The timeout is specified in seconds."), >> + nullptr, >> + show_indirect_call_timeout, >> + &setlist, &showlist); >> + >> add_setshow_boolean_cmd >> ("infcall", class_maintenance, &debug_infcall, >> _("Set inferior call debugging."), >> diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp >> index 87919a819ab..504bf90cc15 100644 >> --- a/gdb/testsuite/gdb.base/help.exp >> +++ b/gdb/testsuite/gdb.base/help.exp >> @@ -121,7 +121,7 @@ gdb_test "help info bogus-gdb-command" "Undefined info command: \"bogus- >> gdb-comm >> gdb_test "help gotcha" "Undefined command: \"gotcha\"\. Try \"help\"\." >> >> # Test apropos regex. >> -gdb_test "apropos \\\(print\[\^\[ bsiedf\\\".-\]\\\)" "handle -- Specify how to handle >> signals\." >> +gdb_test "apropos \\\(print\[\^\[ bsiedf\\\"'.-\]\\\)" "handle -- Specify how to handle >> signals\." >> # Test apropos >1 word string. >> gdb_test "apropos handle signal" "handle -- Specify how to handle signals\." >> # Test apropos apropos. >> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.c b/gdb/testsuite/gdb.base/infcall- >> timeout.c >> new file mode 100644 >> index 00000000000..12774ca2599 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/infcall-timeout.c >> @@ -0,0 +1,36 @@ >> +/* Copyright 2022-2023 Free Software Foundation, Inc. >> + >> + This file is part of GDB. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +#include >> + >> +/* This function is called from GDB. */ >> +int >> +function_that_never_returns () >> +{ >> + while (1) >> + sleep (1); >> + >> + return 0; >> +} >> + >> +int >> +main () >> +{ >> + alarm (300); >> + >> + return 0; >> +} >> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall- >> timeout.exp >> new file mode 100644 >> index 00000000000..5e9cdc2fa0e >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp >> @@ -0,0 +1,82 @@ >> +# Copyright 2022-2023 Free Software Foundation, Inc. >> + >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 3 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see . >> + >> +# Test GDB's direct-call-timeout setting, that is, ensure that if an >> +# inferior function call, invoked from e.g. a 'print' command, takes >> +# too long, then GDB can interrupt it, and return control to the user. >> + >> +standard_testfile >> + >> +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ >> + {debug}] == -1 } { >> + return >> +} >> + >> +# Start GDB according to TARGET_ASYNC and TARGET_NON_STOP, then adjust >> +# the direct-call-timeout, and make an inferior function call that >> +# will never return. GDB should eventually timeout and stop the >> +# inferior. >> +proc_with_prefix run_test { target_async target_non_stop } { >> + save_vars { ::GDBFLAGS } { >> + append ::GDBFLAGS \ >> + " -ex \"maint set target-non-stop $target_non_stop\"" >> + append ::GDBFLAGS \ >> + " -ex \"maintenance set target-async ${target_async}\"" >> + >> + clean_restart ${::binfile} >> + } >> + >> + if {![runto_main]} { >> + fail "run to main" > > runto_main already emits a fail, so no need for this. > >> + return >> + } >> + >> + gdb_test_no_output "set direct-call-timeout 5" >> + >> + # When non-stop mode is off we get slightly different output from GDB. >> + if { [gdb_is_remote_or_extended_remote_target] && !$target_non_stop } { >> + set stopped_line_pattern "Program received signal SIGINT, Interrupt\\." >> + } else { >> + set stopped_line_pattern "Program stopped\\." >> + } >> + >> + gdb_test "print function_that_never_returns ()" \ >> + [multi_line \ >> + $stopped_line_pattern \ >> + ".*" \ >> + "The program being debugged timed out while in a function called from GDB\\." \ >> + "GDB remains in the frame where the timeout occurred\\." \ >> + "To change this behavior use \"set unwind-on-timeout on\"\\." \ >> + "Evaluation of the expression containing the function" \ >> + "\\(function_that_never_returns\\) will be abandoned\\." \ >> + "When the function is done executing, GDB will silently stop\\."] >> + >> + gdb_test "bt" ".* function_that_never_returns .*.*" >> +} >> + >> +foreach_with_prefix target_async { "on" "off" } { >> + >> + if { !$target_async } { >> + # GDB can't timeout while waiting for a thread if the target >> + # runs with async-mode turned off; once the target is running >> + # GDB is effectively blocked until the target stops for some >> + # reason. >> + continue >> + } >> + >> + foreach_with_prefix target_non_stop { "on" "off" } { >> + run_test $target_async $target_non_stop >> + } >> +} >> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c >> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c >> new file mode 100644 >> index 00000000000..4da4245746e >> --- /dev/null >> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c >> @@ -0,0 +1,169 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2022-2023 Free Software Foundation, Inc. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define NUM_THREADS 5 >> + >> +/* Semaphores, used to track when threads have started, and to control >> + when the threads finish. */ >> +sem_t startup_semaphore; >> +sem_t finish_semaphore; >> +sem_t thread_1_semaphore; >> +sem_t thread_2_semaphore; >> + >> +/* Mutex to control when the first worker thread hit a breakpoint >> + location. */ >> +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; >> + >> +/* Global variable to poke, just so threads have something to do. */ >> +volatile int global_var = 0; >> + >> +int >> +condition_func () >> +{ >> + /* Let thread 2 run. */ >> + if (sem_post (&thread_2_semaphore) != 0) >> + abort (); >> + >> + /* Wait for thread 2 to complete its actions. */ >> + if (sem_wait (&thread_1_semaphore) != 0) >> + abort (); >> + >> + return 1; >> +} >> + >> +void >> +do_segfault () >> +{ >> + volatile int *p = 0; >> + *p = 0; /* Segfault here. */ >> +} >> + >> +void * >> +worker_func (void *arg) >> +{ >> + int tid = *((int *) arg); >> + >> + /* Let the main thread know that this worker has started. */ >> + if (sem_post (&startup_semaphore) != 0) >> + abort (); >> + >> + switch (tid) >> + { >> + case 0: >> + /* Wait for MUTEX to become available, then pass through the >> + conditional breakpoint location. */ >> + if (pthread_mutex_lock (&mutex) != 0) >> + abort (); >> + global_var = 99; /* Conditional breakpoint here. */ >> + if (pthread_mutex_unlock (&mutex) != 0) >> + abort (); >> + break; >> + >> + case 1: >> + if (sem_wait (&thread_2_semaphore) != 0) >> + abort (); >> + do_segfault (); >> + if (sem_post (&thread_1_semaphore) != 0) >> + abort (); >> + >> + /* Fall through. */ >> + default: >> + /* Wait until we are allowed to finish. */ >> + if (sem_wait (&finish_semaphore) != 0) >> + abort (); >> + break; >> + } >> +} >> + >> +void >> +stop_marker () >> +{ >> + global_var = 99; /* Stop marker. */ >> +} >> + >> +/* The main program entry point. */ >> + >> +int >> +main () >> +{ >> + pthread_t threads[NUM_THREADS]; >> + int args[NUM_THREADS]; >> + void *retval; >> + >> + /* An alarm, just in case the thread deadlocks. */ >> + alarm (300); >> + >> + /* Semaphore initialization. */ >> + if (sem_init (&startup_semaphore, 0, 0) != 0) >> + abort (); >> + if (sem_init (&finish_semaphore, 0, 0) != 0) >> + abort (); >> + if (sem_init (&thread_1_semaphore, 0, 0) != 0) >> + abort (); >> + if (sem_init (&thread_2_semaphore, 0, 0) != 0) >> + abort (); >> + >> + /* Lock MUTEX, this prevents the first worker thread from rushing ahead. */ >> + if (pthread_mutex_lock (&mutex) != 0) >> + abort (); >> + >> + /* Worker thread creation. */ >> + for (int i = 0; i < NUM_THREADS; i++) >> + { >> + args[i] = i; >> + pthread_create (&threads[i], NULL, worker_func, &args[i]); >> + } >> + >> + /* Wait for every thread to start. */ >> + for (int i = 0; i < NUM_THREADS; i++) >> + { >> + if (sem_wait (&startup_semaphore) != 0) >> + abort (); >> + } >> + >> + /* Unlock the first thread so it can proceed. */ >> + if (pthread_mutex_unlock (&mutex) != 0) >> + abort (); >> + >> + /* Wait for the first thread only. */ >> + pthread_join (threads[0], &retval); >> + >> + /* Now post FINISH_SEMAPHORE to allow all the other threads to finish. */ >> + for (int i = 1; i < NUM_THREADS; i++) >> + sem_post (&finish_semaphore); >> + >> + /* Now wait for the remaining threads to complete. */ >> + for (int i = 1; i < NUM_THREADS; i++) >> + pthread_join (threads[i], &retval); >> + >> + /* Semaphore cleanup. */ >> + sem_destroy (&finish_semaphore); >> + sem_destroy (&startup_semaphore); >> + sem_destroy (&thread_1_semaphore); >> + sem_destroy (&thread_2_semaphore); >> + >> + stop_marker (); >> + >> + return 0; >> +} >> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp >> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp >> new file mode 100644 >> index 00000000000..4159288a39c >> --- /dev/null >> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp >> @@ -0,0 +1,156 @@ >> +# Copyright 2022-2023 Free Software Foundation, Inc. >> + >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 3 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see . >> + >> +# Tests inferior calls executed from a breakpoint condition in >> +# a multi-threaded program. >> +# >> +# This test has the inferior function call timeout, and checks how GDB >> +# handles this situation. >> + >> +standard_testfile >> + >> +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ >> + {debug pthreads}] } { >> + return >> +} >> + >> +set cond_bp_line [gdb_get_line_number "Conditional breakpoint here"] >> +set final_bp_line [gdb_get_line_number "Stop marker"] >> +set segfault_line [gdb_get_line_number "Segfault here"] >> + >> +# Setup GDB based on TARGET_ASYNC and TARGET_NON_STOP. Setup some >> +# breakpoints in the inferior, one of which has an inferior call >> +# within its condition. >> +# >> +# Continue GDB, the breakpoint with inferior call will be hit, but the >> +# inferior call will never return. We expect GDB to timeout. >> +# >> +# The reason that the inferior call never completes is that a second >> +# thread, on which the inferior call relies, either hits a breakpoint >> +# (when OTHER_THREAD_BP is true), or crashes (when OTHER_THREAD_BP is >> +# false). >> +proc run_test { target_async target_non_stop other_thread_bp } { >> + save_vars { ::GDBFLAGS } { >> + append ::GDBFLAGS " -ex \"maint set target-non-stop $target_non_stop\"" >> + append ::GDBFLAGS " -ex \"maintenance set target-async ${target_async}\"" >> + >> + clean_restart ${::binfile} >> + } >> + >> + if {![runto_main]} { >> + fail "run to main" > > runto_main already emits a fail, so no need for this. > >> + return >> + } >> + >> + # The default timeout for indirect inferior calls (e.g. inferior >> + # calls for conditional breakpoint expressions) is pretty high. >> + # We don't want the test to take too long, so reduce this. >> + # >> + # However, the test relies on a second thread hitting some event >> + # (either a breakpoint or signal) before this timeout expires. >> + # >> + # There is a chance that on a really slow system this might not >> + # happen, in which case the test might fail. >> + # >> + # However, we still allocate 5 seconds, which feels like it should >> + # be enough time in most cases, but maybe we need to do something >> + # smarter here? Possibly we could have some initial run where the >> + # inferior doesn't timeout, but does perform the same interaction >> + # between threads, we could time that, and use that as the basis >> + # for this timeout. For now though, we just hope 5 seconds is >> + # enough. >> + gdb_test_no_output "set indirect-call-timeout 5" >> + >> + gdb_breakpoint \ >> + "${::srcfile}:${::cond_bp_line} if (condition_func ())" >> + set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ >> + "get number for conditional breakpoint"] >> + >> + gdb_breakpoint "${::srcfile}:${::final_bp_line}" >> + set final_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ >> + "get number for final breakpoint"] >> + >> + # The thread performing an inferior call relies on a second >> + # thread. The second thread will segfault unless it hits a >> + # breakpoint first. In either case the initial thread will not >> + # complete its inferior call. >> + if { $other_thread_bp } { >> + gdb_breakpoint "${::srcfile}:${::segfault_line}" >> + set segfault_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ >> + "get number for segfault breakpoint"] >> + } >> + >> + # When non-stop mode is off we get slightly different output from GDB. >> + if { [gdb_is_remote_or_extended_remote_target] && !$target_non_stop} { >> + set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal >> SIGINT, Interrupt\\." >> + } else { >> + set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." >> + } >> + >> + gdb_test "continue" \ >> + [multi_line \ >> + $stopped_line_pattern \ >> + ".*" \ >> + "Error in testing condition for breakpoint ${bp_num}:" \ >> + "The program being debugged timed out while in a function called from GDB\\." \ >> + "GDB remains in the frame where the timeout occurred\\." \ >> + "To change this behavior use \"set unwind-on-timeout on\"\\." \ >> + "Evaluation of the expression containing the function" \ >> + "\\(condition_func\\) will be abandoned\\." \ >> + "When the function is done executing, GDB will silently stop\\."] \ >> + "expected timeout waiting for inferior call to complete" >> + >> + # Remember that other thread that either crashed (with a segfault) >> + # or hit a breakpoint? Now that the inferior call has timed out, >> + # if we try to resume then we should see the pending event from >> + # that other thread. >> + if { $other_thread_bp } { >> + gdb_test "continue" \ >> + [multi_line \ >> + "Continuing\\." \ >> + ".*" \ >> + "" \ >> + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${segfault_bp_num}, >> do_segfault \[^\r\n\]+:${::segfault_line}" \ >> + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \ >> + "hit the segfault breakpoint" >> + } else { >> + gdb_test "continue" \ >> + [multi_line \ >> + "Continuing\\." \ >> + ".*" \ >> + "Thread ${::decimal} \"infcall-from-bp\" received signal SIGSEGV, >> Segmentation fault\\." \ >> + "\\\[Switching to Thread \[^\r\n\]+\\\]" \ >> + "${::hex} in do_segfault \\(\\) at \[^\r\n\]+:${::segfault_line}" \ >> + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \ >> + "hit the segfault" >> + } >> +} >> + >> +foreach_with_prefix target_async {"on" "off" } { >> + >> + if { !$target_async } { >> + # GDB can't timeout while waiting for a thread if the target >> + # runs with async-mode turned off; once the target is running >> + # GDB is effectively blocked until the target stops for some >> + # reason. >> + continue >> + } >> + >> + foreach_with_prefix target_non_stop {"off" "on"} { >> + foreach_with_prefix other_thread_bp { true false } { >> + run_test $target_async $target_non_stop $other_thread_bp >> + } >> + } >> +} >> -- >> 2.25.4 > > Reviewed-By: Tankut Baris Aktemur > > Thanks. > -Baris > > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, 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