From patchwork Thu Dec 14 20:22:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 82167 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 06AC73861881 for ; Thu, 14 Dec 2023 20:23:01 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by sourceware.org (Postfix) with ESMTPS id DE6FD385C6DD for ; Thu, 14 Dec 2023 20:22:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DE6FD385C6DD Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DE6FD385C6DD Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.51 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702585368; cv=none; b=tyBf2pVFQxu4nHQpZPMBFenXBrjHpwGkHrkFnokcJoZeeElM6Cpa6t6Zo+OFrMpiIe1jnQZw6bt1qj7yrvDHp958gjWtVoxTI4GvDRVbuZW9xCBQu8k7kGazkRdAIRtlid9AN/02lRBbJqziab7S5Eqry9haEsEkVX7y+LtgCVA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702585368; c=relaxed/simple; bh=9MRAx0PMJhJk7ma/DlxZTPvu9S/MxrJIBCQgkf6C+DQ=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=XTkVDG9yyEPDqNF4zg/Dq4oAHfoKvbb3n4xQchNo0CTNd9w4xw04auEq0QGoyhPWqGWlNlG2YIexmErbiXDwXCLCKa/+Sv2Cp253TDjduhSMR3IvJ2qEwxOgamNDyg7P+bKebQ4jblNhQ6tNQGE4RUkxURLyEDY4ilSvhfjsACo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-336447f240cso1087529f8f.3 for ; Thu, 14 Dec 2023 12:22:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702585364; x=1703190164; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8+Svmzskv6A4yq094F0huGCx4/mjQOR3fs87alIeD9M=; b=wBJRUqyqmnyxjor4c0YQTFJygkg3BziiK4AOR+K0myk3dsYt2kRLasoygmRLdrU0B7 T8rPvbAuRqW11x8R3h3xC8uaomUu2+JFBb9BR88Ecjndwe81BBdl6lv6xQkXm5urW36K cg5AGtus4auAy/lSTfGf1GqySl2+mmwzHsHjo8MfgYhpLIAdDAsex2TYsLPEdisCLsC0 e5rpTwlqable4jL0g6D6rFWUfxLqOByYd/N2hdnahIs0nGc/v6wcucj3sXd98l1fOuP0 gllP3U6KafkcJySlZ+x9NC8As2sgA+EYMqHaRqPQ5PliLT67XE829OjPLF0ZUR5M+3sZ EmAQ== X-Gm-Message-State: AOJu0Yx91NCCn/AkxOlKhdV1hTCcoJ3e36Ja5y3b8PHFm9DklZXklfC3 JrF1HXXiEJUaJ/SFeQ0UOH2NWR8sPtwuQg== X-Google-Smtp-Source: AGHT+IGv+bsGp+4txsAXNoVqOxbhVD+axm2ouG7OPZSd/kva2nsdkYzkL/cyBmqcR8NsjQ+FePHPBw== X-Received: by 2002:a5d:620c:0:b0:334:b14d:c57 with SMTP id y12-20020a5d620c000000b00334b14d0c57mr5616848wru.22.1702585363731; Thu, 14 Dec 2023 12:22:43 -0800 (PST) Received: from localhost ([2001:8a0:f923:4f00:2646:535c:5a04:e380]) by smtp.gmail.com with UTF8SMTPSA id e10-20020adfe7ca000000b003335ddce799sm3625857wrn.103.2023.12.14.12.22.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Dec 2023 12:22:43 -0800 (PST) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 1/8] gdb.threads/step-over-thread-exit.exp improvements Date: Thu, 14 Dec 2023 20:22:31 +0000 Message-ID: <20231214202238.1065676-2-pedro@palves.net> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20231214202238.1065676-1-pedro@palves.net> References: <20231214202238.1065676-1-pedro@palves.net> MIME-Version: 1.0 X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org This commit makes the following improvements to gdb.threads/step-over-thread-exit.exp: - Add a third axis to stepping over the breakpoint with displaced vs inline stepping -- also test with no breakpoint at all. - Check that when GDB reports "Command aborted, thread exited.", the selected thread is the thread that exited. This is always true currently on GNU/Linux by coincidence, but a similar testcase on AMD GPU exposed a problem here. Better make the testcase catch any potential regression. - Fixes a race that Simon ran into with GDBserver testing. (gdb) next [New Thread 2143071.2143438] Thread 3 "step-over-threa" hit Breakpoint 2, 0x000055555555524e in my_exit_syscall () at .../testsuite/lib/my-syscalls.S:74 74 SYSCALL (my_exit, __NR_exit) (gdb) FAIL: gdb.threads/step-over-thread-exit.exp: displaced-stepping=auto: non-stop=on: target-non-stop=on: schedlock=off: cmd=next: ns_stop_all=0: command aborts when thread exits I was not able to reproduce it, but I believe that what happens is the following: Once we continue, the thread 2 exits, and the main thread thus unblocks from its pthread_join, and spawns a new thread. That new thread may hit the breakpoint at my_exit_syscall very quickly. GDB could then see/process that breakpoint event before the thread exit event for the thread we care about, which would result in the failure seen above. The fix here is to not loop and start a new thread at all in the scenario where the race can happen. We only need to loop and spawn new threads when testing with "cmd=continue" and schedlock off, in which case GDB doesn't abort the command when the thread exits. Change-Id: I90c95c32f00630a3f682b1541c23aff52451f9b6 --- .../gdb.threads/step-over-thread-exit.c | 16 ++- .../gdb.threads/step-over-thread-exit.exp | 127 +++++++++++++++--- 2 files changed, 119 insertions(+), 24 deletions(-) diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit.c b/gdb/testsuite/gdb.threads/step-over-thread-exit.c index 878e5924c5c..218f003b205 100644 --- a/gdb/testsuite/gdb.threads/step-over-thread-exit.c +++ b/gdb/testsuite/gdb.threads/step-over-thread-exit.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "../lib/my-syscalls.h" static void * @@ -30,13 +31,19 @@ thread_func (void *arg) abort (); } +/* Number of threads we'll create. */ +int n_threads = 100; + int -main (void) +main (int argc, char **argv) { int i; - /* Spawn and join a thread, 100 times. */ - for (i = 0; i < 100; i++) + if (argc > 1) + n_threads = atoi (argv[1]); + + /* Spawn and join a thread, N_THREADS times. */ + for (i = 0; i < n_threads; i++) { pthread_t thread; int ret; @@ -48,5 +55,8 @@ main (void) assert (ret == 0); } + /* Some time to make sure that GDB processes the thread exit event + before the whole-process exit. */ + sleep (3); return 0; } diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit.exp b/gdb/testsuite/gdb.threads/step-over-thread-exit.exp index 615bd838763..32f64ce1a3e 100644 --- a/gdb/testsuite/gdb.threads/step-over-thread-exit.exp +++ b/gdb/testsuite/gdb.threads/step-over-thread-exit.exp @@ -25,11 +25,29 @@ if { [build_executable "failed to prepare" $testfile \ return } -# Each argument is a different testing axis, most of them obvious. +# Test stepping/continuing at an exit syscall instruction. +# +# Each argument is a different testing axis. +# +# STEP_OVER_MODE can be one of: +# +# - none: don't put a breakpoint on the exit syscall instruction. +# +# - inline: put a breakpoint on the exit syscall instruction, and +# use in-line stepping to step over it (disable +# displaced-stepping). +# +# - displaced: same, but use displaced stepping. +# +# SCHEDLOCK can be "on" or "off". +# +# CMD is the GDB command to run when at the exit syscall instruction. +# # NS_STOP_ALL is only used if testing "set non-stop on", and indicates # whether to have GDB explicitly stop all threads before continuing to # thread exit. -proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all} { +# +proc test {step_over_mode non-stop target-non-stop schedlock cmd ns_stop_all} { if {${non-stop} == "off" && $ns_stop_all} { error "invalid arguments" } @@ -40,23 +58,29 @@ proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all clean_restart $::binfile } - gdb_test_no_output "set displaced-stepping ${displaced-stepping}" - - if { ![runto_main] } { - return + if { $step_over_mode == "none" } { + # Nothing to do. + } elseif { $step_over_mode == "inline" } { + gdb_test_no_output "set displaced-stepping off" + } elseif { $step_over_mode == "displaced" } { + gdb_test_no_output "set displaced-stepping on" + } else { + error "Invalid step_over_mode value: $step_over_mode" } - gdb_breakpoint "my_exit_syscall" - if {$schedlock || (${non-stop} == "on" && $ns_stop_all)} { - gdb_test "continue" \ - "Thread 2 .*hit Breakpoint $::decimal.* my_exit_syscall .*" \ - "continue until syscall" + + gdb_test_no_output "set args 1" + + if { ![runto my_exit_syscall] } { + return + } if {${non-stop} == "on"} { # The test only spawns one thread at a time, so this just - # stops the main thread. + # stops the main thread. IOW, we only need to wait for + # one stop. gdb_test_multiple "interrupt -a" "" { -re "$::gdb_prompt " { gdb_test_multiple "" $gdb_test_name { @@ -66,12 +90,19 @@ proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all } } } - } - gdb_test "thread 2" "Switching to thread 2 .*" + gdb_test "thread 2" "Switching to thread 2 .*" + } gdb_test_no_output "set scheduler-locking ${schedlock}" + # If testing a step-over is requested, leave the breakpoint at + # the current instruction to force a step-over; otherwise, + # remove it. + if { $step_over_mode == "none" } { + delete_breakpoints + } + if {$cmd == "continue"} { gdb_test "continue" \ "No unwaited-for children left." \ @@ -84,9 +115,50 @@ proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all } } } else { + # Schedlock is off here. + # + # With "continue" and no scheduler-locking, GDB doesn't stop + # with "Command aborted, thread exited." when the thread + # exits, it just lets the inferior continue running freely. + # So we test that we can move past the thread exit, and that + # other threads can be freely scheduled. We do that by + # spawning another thread as soon as the first exit. We test + # that a number of times. This should also exercise GDB's + # handling of inline or displaced step-overs, that GDB handles + # the related resource accounting correctly when the stepping + # thread exits, etc. + # + # With "continue" and $step_over_mode == "none" however, after + # the first my_exit_syscall breakpoint hit, we will remove the + # breakpoint, so no other thread would ever hit it again. So + # might as well just test one thread. + # + # With step/next, GDB aborts the execution command with + # "Command aborted, thread exited." when the stepping thread + # exits. If we let the main spawn another thread as soon as + # the first exits, it would be possible for that new thread to + # hit the exit syscall insn breakpoint quickly enough that it + # would be reported to be user before the first thread exit + # would be, which would confuse testing. To avoid that, we + # only spawn one thread, too. + # + if {$cmd != "continue" || $step_over_mode == "none"} { + set n_threads 1 + } else { + set n_threads 100 + } + + gdb_test_no_output "set args $n_threads" + + if { ![runto_main] } { + return + } + + gdb_breakpoint "my_exit_syscall" + gdb_test_no_output "set scheduler-locking ${schedlock}" - if {$cmd != "continue"} { + if {$cmd != "continue" || $step_over_mode == "none"} { set thread "" gdb_test_multiple "continue" "" { -re -wrap "Thread ($::decimal) .*hit Breakpoint $::decimal.* my_exit_syscall .*" { @@ -98,10 +170,23 @@ proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all "switch to event thread" } - gdb_test_multiple $cmd "command aborts when thread exits" { - -re "Command aborted, thread exited\\.\r\n$::gdb_prompt " { - pass $gdb_test_name + # If testing a step-over is requested, leave the breakpoint at + # the current instruction to force a step-over; otherwise, + # remove it. + if { $step_over_mode == "none" } { + delete_breakpoints + } + + if {$cmd == "continue"} { + gdb_continue_to_end "continue to end" "continue" 1 + } else { + gdb_test_multiple $cmd "command aborts when thread exits" { + -re "Command aborted, thread exited\\.\r\n$::gdb_prompt " { + pass $gdb_test_name + } } + gdb_test "p \$_thread == $thread" "= 1" \ + "selected thread didn't change" } } else { for { set i 0 } { $i < 100 } { incr i } { @@ -130,7 +215,7 @@ proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all } } -foreach_with_prefix displaced-stepping {off auto} { +foreach_with_prefix step_over_mode {none inline displaced} { foreach_with_prefix non-stop {off on} { foreach_with_prefix target-non-stop {off on} { if {${non-stop} == "on" && ${target-non-stop} == "off"} { @@ -142,11 +227,11 @@ foreach_with_prefix displaced-stepping {off auto} { foreach_with_prefix cmd {"next" "continue"} { if {${non-stop} == "on"} { foreach_with_prefix ns_stop_all {0 1} { - test ${displaced-stepping} ${non-stop} ${target-non-stop} \ + test ${step_over_mode} ${non-stop} ${target-non-stop} \ ${schedlock} ${cmd} ${ns_stop_all} } } else { - test ${displaced-stepping} ${non-stop} ${target-non-stop} ${schedlock} ${cmd} 0 + test ${step_over_mode} ${non-stop} ${target-non-stop} ${schedlock} ${cmd} 0 } } }