From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id RRk+JnebiWEmMwAAWB0awg (envelope-from ) for ; Mon, 08 Nov 2021 16:49:43 -0500 Received: by simark.ca (Postfix, from userid 112) id 87E621F0C1; Mon, 8 Nov 2021 16:49:43 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C5FEB1E813 for ; Mon, 8 Nov 2021 16:49:42 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5B2143858001 for ; Mon, 8 Nov 2021 21:49:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5B2143858001 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1636408182; bh=qftgzsT/U4HlZO2SulB6g//e4cLfCxxHWhPaBFNNYz0=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=DigjxJ0rR53YGtLkoLFtVw+giS+c98376i5RiTIlwrRs+qocUyn2CNXkXpXSRSXJW T6IjGa7ukEl2KBz3GcmteEaIhCdjyAjz1/8FO52f+TfmBMYFiTMf6EmrIc/y8Q31F1 XFEHYJaMKQzM5bfZ+W4vE3oKJYw+N9QVkPh7hkAo= Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 26A7C3858C2C for ; Mon, 8 Nov 2021 21:49:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 26A7C3858C2C X-ASG-Debug-ID: 1636408162-0c856e038795b070001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id sWU0jPie5a37uH4o (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 08 Nov 2021 16:49:22 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 12E0A441B21; Mon, 8 Nov 2021 16:49:22 -0500 (EST) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Subject: [PATCH] gdb: fix "set scheduler-locking" thread exit hang Date: Mon, 8 Nov 2021 16:49:21 -0500 X-ASG-Orig-Subj: [PATCH] gdb: fix "set scheduler-locking" thread exit hang Message-Id: <20211108214921.373638-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.33.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1636408162 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 6787 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.93830 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" GDB hangs when doing this: - launch inferior with multiple threads - multiple threads hit some breakpoint(s) - one breakpoint hit is presented as a stop, the rest are saved as pending wait statuses - "set scheduler-locking on" - resume the currently selected thread (because of scheduler-locking, it's the only one resumed), let it execute until exit - GDB hangs, not showing the prompt, impossible to interrupt with ^C When the resumed thread exits, we expect the target to return a TARGET_WAITKIND_NO_RESUMED event, and that's what we see: [infrun] fetch_inferior_event: enter [infrun] scoped_disable_commit_resumed: reason=handling event [infrun] random_pending_event_thread: None found. [Thread 0x7ffff7d9c700 (LWP 309357) exited] [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) = [infrun] print_target_wait_results: -1.0.0 [process -1], [infrun] print_target_wait_results: status->kind = no-resumed [infrun] handle_inferior_event: status->kind = no-resumed [infrun] handle_no_resumed: TARGET_WAITKIND_NO_RESUMED (ignoring: found resumed) [infrun] prepare_to_wait: prepare_to_wait [infrun] reset: reason=handling event [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target native, no resumed threads [infrun] fetch_inferior_event: exit The problem is in handle_no_resumed: we check if some other thread is actually resumed, to see if we should ignore that event (see comments in that function for more info). If this condition is true: (thread->executing () || thread->has_pending_waitstatus ()) ... then we ignore the event. The problem is that there are some non-resumed threads with a pending event, which makes us ignore the event. But these threads are not resumed, so we end up waiting while nothing executes, hence waiting for ever. My first fix was to change the condition to: (thread->executing () || (thread->resumed () && thread->has_pending_waitstatus ())) ... but then it occured to me that we could simply check for: (thread->resumed ()) Since "executing" implies "resumed", checking simply for "resumed" covers threads that are resumed and executing, as well as threads that are resumed with a pending status, which is what we want. Change-Id: Ie796290f8ae7f34c026ca3a8fcef7397414f4780 --- gdb/infrun.c | 3 +- .../gdb.threads/schedlock-thread-exit.c | 46 +++++++++++++++++++ .../gdb.threads/schedlock-thread-exit.exp | 42 +++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/schedlock-thread-exit.c create mode 100644 gdb/testsuite/gdb.threads/schedlock-thread-exit.exp diff --git a/gdb/infrun.c b/gdb/infrun.c index c6341272bbd..95c8bfb2e7d 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5138,8 +5138,7 @@ handle_no_resumed (struct execution_control_state *ecs) swap_terminal = false; } - if (!ignore_event - && (thread->executing () || thread->has_pending_waitstatus ())) + if (!ignore_event && thread->resumed ()) { /* Either there were no unwaited-for children left in the target at some point, but there are now, or some target diff --git a/gdb/testsuite/gdb.threads/schedlock-thread-exit.c b/gdb/testsuite/gdb.threads/schedlock-thread-exit.c new file mode 100644 index 00000000000..ea8a349ed13 --- /dev/null +++ b/gdb/testsuite/gdb.threads/schedlock-thread-exit.c @@ -0,0 +1,46 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 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 + +static void * +thread_func (void *p) +{ + return NULL; +} + +int +main (void) +{ + const int nthreads = 10; + pthread_t threads[nthreads]; + + for (int i = 0; i < nthreads; ++i) + { + int ret = pthread_create (&threads[i], NULL, thread_func, NULL); + assert (ret == 0); + } + + for (int i = 0; i < nthreads; ++i) + { + int ret = pthread_join (threads[i], NULL); + assert (ret == 0); + } + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/schedlock-thread-exit.exp b/gdb/testsuite/gdb.threads/schedlock-thread-exit.exp new file mode 100644 index 00000000000..818baa781db --- /dev/null +++ b/gdb/testsuite/gdb.threads/schedlock-thread-exit.exp @@ -0,0 +1,42 @@ +# Copyright 2021 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 that GDB handles well a thread exiting with sched-lock on. A buggy GDB +# would hang, a fixed GDB shows the "No unwaited-for children left" message and +# shows the prompt. +# +# The explanation for the original bug involved some threads with a pending +# wait status, so we launch multiple threads, which very likely gives us that. + +standard_testfile + +if { [build_executable "failed to prepare" ${testfile} ${srcfile} {debug pthreads}] } { + return +} + +proc do_test { } { + clean_restart $::binfile + + # One of the launched threads will report a stop on thread_func. Some + # others will also stop on thread_func and have a pending status. + if { ![runto "thread_func"] } { + return + } + + gdb_test_no_output "set scheduler-locking on" + gdb_test "continue" "No unwaited-for children left." +} + +do_test -- 2.33.0