From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by sourceware.org (Postfix) with ESMTPS id 58D92384C366 for ; Mon, 12 Dec 2022 20:31:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 58D92384C366 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f42.google.com with SMTP id o15so6594237wmr.4 for ; Mon, 12 Dec 2022 12:31:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Q/AzHeEyFb5IJ1gyjs+2WZzNDJvE57VYEfQxyEssntM=; b=Ll4Hwgn7Mc+Aaq8ivyPVBry42t+HtruFzlJPX3PdSiA24ayAz12cabalDgpDXLNzJS WxbtevlQglLc1gbYX185i4azsbH/QbYBiafkUWBmZBNUc+HiFyYyuJ6uAsgc7aUFs5/I 7vIUj+oXYtg0Nom9d1HZBHhC6e/STfTUoyoqKQV9213U+A6Y2iCjcG4I64vc5oOGJDpp bAX/+noqLe7lfGzuZxAMwj8qWWhcTejl6AfgxsN3bbF1+EGg5P3d2ESVDsfgqToMb2CX Z17gI3q8xewAVSrqUg1P5WTK7MhLyHZl8pX82cbaa3j4NYNTh6sGyCZoTEdwv0iAXKMd eS2g== X-Gm-Message-State: ANoB5plHFaYo2CnJqJxwYF0Zir/Le+LEKfbpRxZ3geMoOahArjLPgd+r VigQcHiCEwqEAdfJyQb82LjisOfzLsqHiw== X-Google-Smtp-Source: AA0mqf7TzpKavFXNeB/YUruua3ufIkkzsHAEnAtSZo+Fvjv/xq6QW/kNZH0uSnpim7pPx2DLrVjNjA== X-Received: by 2002:a05:600c:4f45:b0:3cf:615e:480e with SMTP id m5-20020a05600c4f4500b003cf615e480emr17010921wmq.12.1670877089736; Mon, 12 Dec 2022 12:31:29 -0800 (PST) Received: from localhost ([2001:8a0:f912:6700:afd9:8b6d:223f:6170]) by smtp.gmail.com with ESMTPSA id t2-20020a1c7702000000b003cffd3c3d6csm10450567wmi.12.2022.12.12.12.31.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 12 Dec 2022 12:31:29 -0800 (PST) From: Pedro Alves To: gdb-patches@sourceware.org Cc: Eli Zaretskii Subject: [PATCH 23/31] Don't resume new threads if scheduler-locking is in effect Date: Mon, 12 Dec 2022 20:30:53 +0000 Message-Id: <20221212203101.1034916-24-pedro@palves.net> X-Mailer: git-send-email 2.36.0 In-Reply-To: <20221212203101.1034916-1-pedro@palves.net> References: <20221212203101.1034916-1-pedro@palves.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP 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: If scheduler-locking is in effect, like e.g., with "set scheduler-locking on", and you step over a function that spawns a new thread, the new thread is allowed to run free, at least until some event is hit, at which point, whether the new thread is re-resumed depends on a number of seemingly random factors. E.g., if the target is all-stop, and the parent thread hits a breakpoint, and gdb decides the breakpoint isn't interesting to report to the user, then the parent thread is resumed, but the new thread is left stopped. I think that letting the new threads run with scheduler-locking enabled is a defect. This commit fixes that, making use of the new clone events on Linux, and of target_thread_events() on targets where new threads have no connection to the thread that spawned them. Testcase and documentation changes included. Approved-By: Eli Zaretskii Change-Id: Ie12140138b37534b7fc1d904da34f0f174aa11ce --- gdb/NEWS | 7 ++ gdb/doc/gdb.texinfo | 4 +- gdb/infrun.c | 41 +++++++++--- .../gdb.threads/schedlock-new-thread.c | 54 +++++++++++++++ .../gdb.threads/schedlock-new-thread.exp | 67 +++++++++++++++++++ 5 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.c create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.exp diff --git a/gdb/NEWS b/gdb/NEWS index c4ccfcc9e32..0aa153b83da 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -15,6 +15,13 @@ from the current process state. GDB will show this additional information automatically, or through one of the memory-tag subcommands. +* Scheduler-locking and new threads + + When scheduler-locking is in effect, only the current thread may run + when the inferior is resumed. However, previously, new threads + created by the resumed thread would still be able to run free. Now, + they are held stopped. + * "info breakpoints" now displays enabled breakpoint locations of disabled breakpoints as in the "y-" state. For example: diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 5b566669975..5e75f32e0cd 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -7037,7 +7037,9 @@ the following: There is no locking and any thread may run at any time. @item on -Only the current thread may run when the inferior is resumed. +Only the current thread may run when the inferior is resumed. New +threads created by the resumed thread are held stopped at their entry +point, before they execute any instruction. @item step Behaves like @code{on} when stepping, and @code{off} otherwise. diff --git a/gdb/infrun.c b/gdb/infrun.c index 31321d758da..09391d85256 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -103,6 +103,8 @@ static bool start_step_over (void); static bool step_over_info_valid_p (void); +static bool schedlock_applies (struct thread_info *tp); + /* Asynchronous signal handler registered as event loop source for when we have pending events ready to be passed to the core. */ static struct async_event_handler *infrun_async_inferior_event_token; @@ -1891,7 +1893,13 @@ static void update_thread_events_after_step_over (thread_info *event_thread, const target_waitstatus &event_status) { - if (target_supports_set_thread_options (0)) + if (schedlock_applies (event_thread)) + { + /* If scheduler-locking applies, continue reporting + thread-created/thread-cloned events. */ + return; + } + else if (target_supports_set_thread_options (0)) { /* We can control per-thread options. Disable events for the event thread, unless the thread is gone. */ @@ -2464,9 +2472,14 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig) to start stopped. We need to release the displaced stepping buffer if the stepped thread exits, so we also enable thread-exit events. + + - If scheduler-locking applies, threads that the current thread + spawns should remain halted. It's not strictly necessary to + enable thread-exit events in this case, but it doesn't hurt. */ if (step_over_info_valid_p () - || displaced_step_in_progress_thread (tp)) + || displaced_step_in_progress_thread (tp) + || schedlock_applies (tp)) { gdb_thread_options options = GDB_THREAD_OPTION_CLONE | GDB_THREAD_OPTION_EXIT; @@ -2475,6 +2488,13 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig) else target_thread_events (true); } + else + { + if (target_supports_set_thread_options (0)) + tp->set_thread_options (0); + else if (!displaced_step_in_progress_any_thread ()) + target_thread_events (false); + } /* If we're resuming more than one thread simultaneously, then any thread other than the leader is being set to run free. Clear any @@ -6103,16 +6123,21 @@ handle_inferior_event (struct execution_control_state *ecs) parent->set_running (false); /* If resuming the child, mark it running. */ - if (ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED - || (follow_child || (!detach_fork && (non_stop || sched_multi)))) + if ((ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED + && !schedlock_applies (ecs->event_thread)) + || (ecs->ws.kind () != TARGET_WAITKIND_THREAD_CLONED + && (follow_child + || (!detach_fork && (non_stop || sched_multi))))) child->set_running (true); /* In non-stop mode, also resume the other branch. */ if ((ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED - && target_is_non_stop_p ()) - || (!detach_fork && (non_stop - || (sched_multi - && target_is_non_stop_p ())))) + && target_is_non_stop_p () + && !schedlock_applies (ecs->event_thread)) + || (ecs->ws.kind () != TARGET_WAITKIND_THREAD_CLONED + && (!detach_fork && (non_stop + || (sched_multi + && target_is_non_stop_p ()))))) { if (follow_child) switch_to_thread (parent); diff --git a/gdb/testsuite/gdb.threads/schedlock-new-thread.c b/gdb/testsuite/gdb.threads/schedlock-new-thread.c new file mode 100644 index 00000000000..aba32f3c19d --- /dev/null +++ b/gdb/testsuite/gdb.threads/schedlock-new-thread.c @@ -0,0 +1,54 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021-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 . */ + +#include +#include +#include + +static void * +thread_func (void *arg) +{ +#if !SCHEDLOCK + while (1) + sleep (1); +#endif + + return NULL; +} + +int +main (void) +{ + pthread_t thread; + int ret; + + ret = pthread_create (&thread, NULL, thread_func, NULL); /* set break 1 here */ + assert (ret == 0); + +#if SCHEDLOCK + /* When testing with schedlock enabled, the new thread won't run, so + we can't join it, as that would hang forever. Instead, sleep for + a bit, enough that if the spawned thread is scheduled, it hits + the thread_func breakpoint before the main thread reaches the + "return 0" line below. */ + sleep (3); +#else + pthread_join (thread, NULL); +#endif + + return 0; /* set break 2 here */ +} diff --git a/gdb/testsuite/gdb.threads/schedlock-new-thread.exp b/gdb/testsuite/gdb.threads/schedlock-new-thread.exp new file mode 100644 index 00000000000..0a22cd178f2 --- /dev/null +++ b/gdb/testsuite/gdb.threads/schedlock-new-thread.exp @@ -0,0 +1,67 @@ +# Copyright 2021-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 . + +# Test continuing over a thread spawn with scheduler-locking on. + +standard_testfile .c + +foreach_with_prefix schedlock {off on} { + set sl [expr $schedlock == "on" ? 1 : 0] + if { [build_executable "failed to prepare" $testfile-$sl \ + $srcfile \ + [list debug pthreads additional_flags=-DSCHEDLOCK=$sl]] \ + == -1 } { + return + } +} + +proc test {non-stop schedlock} { + save_vars ::GDBFLAGS { + append ::GDBFLAGS " -ex \"set non-stop ${non-stop}\"" + set sl [expr $schedlock == "on" ? 1 : 0] + clean_restart $::binfile-$sl + } + + set linenum1 [gdb_get_line_number "set break 1 here"] + + if { ![runto $::srcfile:$linenum1] } { + return + } + + delete_breakpoints + + set linenum2 [gdb_get_line_number "set break 2 here"] + gdb_breakpoint $linenum2 + + gdb_breakpoint "thread_func" + + gdb_test_no_output "set scheduler-locking $schedlock" + + if {$schedlock} { + gdb_test "continue" \ + "return 0.*set break 2 here .*" \ + "continue does not stop in new thread" + } else { + gdb_test "continue" \ + "thread_func .*" \ + "continue stops in new thread" + } +} + +foreach_with_prefix non-stop {off on} { + foreach_with_prefix schedlock {off on} { + test ${non-stop} ${schedlock} + } +} -- 2.36.0