From patchwork Tue Oct 18 01:19:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 58968 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8EB9038582B8 for ; Tue, 18 Oct 2022 01:20:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8EB9038582B8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666056034; bh=ZNku6s8xFsueuojHGLrVTWAeY7mcB5Lyup8ojRreHzs=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=d3q3sM1jTenknT5a0x+OjU2Ua8AA3XRDsRGawm3ZGx3aKEgJvC4/9J1oxoVGkIYVV r4Jo2W+P+nMff/JycGcNJdE6kILY5Mp1Fo3CUuek+M+C3j3WQxjqzeQHuJrOgglokU +6S8mubBTKq1CtHsDPiZM+7qYPnmMmdRF0lPQ8Pc= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id EDAC53858D3C for ; Tue, 18 Oct 2022 01:20:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EDAC53858D3C Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 29I1K0xb026935 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 17 Oct 2022 21:20:04 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 29I1K0xb026935 Received: from simark.localdomain (unknown [217.28.27.60]) (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 ESMTPSA id CA6B41E0D3; Mon, 17 Oct 2022 21:19:59 -0400 (EDT) To: gdb-patches@sourceware.org Subject: [PATCH] gdb/testsuite: fix race in gdb.server/stop-reply-no-thread-multi.exp Date: Mon, 17 Oct 2022 21:19:58 -0400 Message-Id: <20221018011958.1113124-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.38.0 MIME-Version: 1.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 18 Oct 2022 01:20:00 +0000 X-Spam-Status: No, score=-3189.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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 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: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I got a random failure on my CI for this test: set scheduler-locking on^M (gdb) PASS: gdb.server/stop-reply-no-thread-multi.exp: target-non-stop=off: to_disable=T: set scheduler-locking on stepi^M 0x00005555555551d1 35 }^M (gdb) FAIL: gdb.server/stop-reply-no-thread-multi.exp: target-non-stop=off: to_disable=T: stepi The expected execution looks like: - the worker thread unlocks the main thread, and goes wait on the `while (worker_blocked)` line - the main thread, now unblocked, calls unlock_worker, where a breakpoint is hit, and everything stops - the test does a stepi on the worker thread, expecting it to land again on the `while (worker_blocked)` line, as it's an empty loop The problem is if the worker thread is slow and doesn't manage to reach the `while (worker_blocked)` line by the time everything stops. The stepi will work fine, but the thread will land at a different line than what the test expects. This can easily be reproduced by adding a `sleep (1)` at the end of unlock_worker. Fix this by inserting a temporary breakpoint in the `while (worker_blocked)` loop and continuing, rather than doing a stepi. This way, the worker thread will always stop on the expected line, regardless of where it started. Re-use the breakpt function, which I believe was not useful prior to this patch. I think this keeps the original intent of the test, which is to test what happens if a stop-reply comes back without a thread identifier, in a multi-threaded program. Because scheduler-locking is "on", only thread 2 is resumed, just like before with the stepi. The stop-reply will maybe be missing the thread identifier, depending on the iteration of the test. If the fix introduced in 8f66807b98f76 ("gdb: better handling of 'S' packets") was reverted, GDB would assume the stop-reply is for thread 1, make thread 1 current, and the "info threads" test just after would fail. Change-Id: I4aff9e4c7d53d74e87c59a4f8b44ea5171014459 --- .../gdb.server/stop-reply-no-thread-multi.c | 4 +--- .../gdb.server/stop-reply-no-thread-multi.exp | 17 ++++++----------- 2 files changed, 7 insertions(+), 14 deletions(-) base-commit: 02a8b5c25806339f801c80a5a364a963b2ece080 diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c index e7da33dcf616..a0cf9044064d 100644 --- a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c @@ -46,9 +46,7 @@ worker (void *data) unlock_main (); while (worker_blocked) - ; - - breakpt (); + breakpt (); return NULL; } diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp index 3bc7cbed52e2..cd096445bf41 100644 --- a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp @@ -103,26 +103,21 @@ proc run_test { target_non_stop disable_feature } { # Switch threads. gdb_test "thread 2" ".*" "switch to second thread" - # Now turn on scheduler-locking so that when we step thread 2 only + # Now turn on scheduler-locking so that when we resume thread 2 only # that one thread will be set running. gdb_test_no_output "set scheduler-locking on" - # Single step thread 2. Only the one thread will step. When the + gdb_test "tbreak breakpt" + + # Resume thread 2. Only the one thread will execute. When the # thread stops, if the stop packet doesn't include a thread-id # then GDB should still understand which thread stopped. - gdb_test_multiple "stepi" "" { - -re -wrap "Thread 1 received signal SIGTRAP.*" { - fail $gdb_test_name - } - -re -wrap "$hex.*$decimal.*while \\(worker_blocked\\).*" { - pass $gdb_test_name - } - } + gdb_test "continue" "Thread 2 hit Temporary breakpoint $::decimal, breakpt.*" # Check that thread 2 is still selected. gdb_test "info threads" \ " 1\[\t \]*Thread\[^\r\n\]*\r\n\\\* 2\[\t \]*Thread\[^\r\n\]*" \ - "second thread should still be selected after stepi" + "second thread should still be selected after continue" # Turn scheduler locking off again so that when we continue all # threads will be set running.