From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id AE4FA3893C4A for ; Wed, 30 Jun 2021 19:55:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AE4FA3893C4A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wm1-x32c.google.com with SMTP id w13so2887628wmc.3 for ; Wed, 30 Jun 2021 12:55:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=EXxtI5zt+FZuoHZD3/CI2dCk1VA212W7hxviwM2kG4w=; b=CaJVL2R0tyAqy3nvOnj0bdEYPdH6BI1rhBx0yBwLWRH4x960/COyD3giujtAAbnp45 d+DuzRZeBUE1qtRCXQIJo44+dF4Dpn4my2Mh0noSQjH8t45xZ0wX0stSUVgDu4nGhD89 +iS8gAnaEiFR1r7mNd1W1za2UNXp7rae6Gq8qITtG/hNcNPlO1ic7Eaj82lTOSSm2628 Gc85ZMg1STmr+fq1toeh2YrgNQSIfntR2s9eYLumez+WUNLFlffIB+RFsxo1XwU869nx ytmZgk+RwnerNCuQtBdAt+3XkEvXazndSjRZOxpJogdEtw/x1s2TX0cmcsqDgLxgDoqw Kt/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=EXxtI5zt+FZuoHZD3/CI2dCk1VA212W7hxviwM2kG4w=; b=PUYWGxfscR2xDsWGQn/PFKlfute81/L59Cvr+ihNQcgaF7DUG95CKHwGHCpQvggLV9 Kl57MaRpqX7cx5HxpK3VkJxuk0im+rteem/K9hTe4cDQ56ei0WerV/GlbgAkL6o+1xYJ +tBNJm18P/99Z5QZiFjNLgNa2u9H7aUCAL4tV1RU+CsasRjf1jh2abvWZMunsdm9wo9j JuqZAmYtASLfFRCh1LhrikUN+eF1X+S7nGHUaQbCQWbhwLpov0ceTPsIEfAEydL0IGK2 oslYO5yuTRRBcJZA4aGs9I38Oc7XzBZT0iBPRiB1f8hYOfAcBZE8ZWfz1lkaEg78dwl1 Yx+g== X-Gm-Message-State: AOAM531lFTRN5jtTENKoWswwd560wYhNMd6KNJ8uD6kQAY5mizxOYFa3 62ri+768eUsnoBrsnj6BGV775h7k2715iw== X-Google-Smtp-Source: ABdhPJzBBmxjuHu/Q3Th2JqMFl2W2m3ED8UnFW+Si2AMXMGiC7DvDEP6DZ161AelS1EklRLy5N6BTw== X-Received: by 2002:a7b:c117:: with SMTP id w23mr6511171wmi.102.1625082927521; Wed, 30 Jun 2021 12:55:27 -0700 (PDT) Received: from localhost (host86-140-92-85.range86-140.btcentralplus.com. [86.140.92.85]) by smtp.gmail.com with ESMTPSA id u12sm23256904wrr.40.2021.06.30.12.55.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 12:55:27 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH 1/3] gdb/testsuite: update test gdb.base/step-over-syscall.exp Date: Wed, 30 Jun 2021 20:55:02 +0100 Message-Id: <11296b5772ede552372625ac37df166117940add.1625081357.git.andrew.burgess@embecosm.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-List-Received-Date: Wed, 30 Jun 2021 19:55:30 -0000 I was looking at bug gdb/19675 and the related test gdb.base/step-over-syscall.exp. This test includes a call to kfail when we are testing a displaced step over a clone syscall. While looking at the test I removed the call to kfail and ran the test, and was surprised that the test passed. I ran the test a few times and it does sometimes fail, but mostly it passed fine. Bug gdb/19675 describes how, when we displaced step over a clone, the new thread is created with a $pc in the displaced step buffer. GDB then fails to "fix" this $pc (for the new thread), and the thread will be set running with its current $pc value. This means that the new thread will just start executing from whatever happens to be after the displaced stepping buffer. In the original gdb/19675 bug report Yao Qi was seeing the new thread cause a segfault. But the problem is, what actually happens is totally undefined. On my machine, I'm seeing the new thread reenter main, it then starts trying to run the test again (in the new thread). This just happens to be safe enough (in this simple test) that most of the time the inferior doesn't crash. In this commit I try to make the test slightly more likely to fail by doing a couple of things. First, I added a static variable to main, this is set true when the first thread enters main, if a second thread ever enters main then I force an abort. Second, when the test is finishing I want to ensure that the new threads have had a chance to do "something bad" if they are going to. So I added a global counter, as each thread starts successfully it decrements the counter. The main thread does not proceed to the final marker function (where GDB has placed a breakpoint) until all threads have started successfully. With these two changes my hope is that the test should fail more reliably, and so, I have also changed the test to call setup_kfail before the specific steps that we expect to misbehave instead of just calling kfail and skipping parts of the test completely. gdb/testsuite/ChangeLog: PR gdb/19675 * gdb.base/step-over-clone.c (global_lock): New global variable. (global_thread_count): Likewise. (clone_fn): Decrement global_thread_count under a lock. (main): Prevent thread reentering main, setup global_thread_count, and wait for all threads to start before calling marker. * gdb.base/step-over-syscall.exp (check_pc_after_cross_syscall): Take additional parameter, figure out the current thread, and check GDB is stopped in the correct thread. (step_over_syscall): Link against pthreads library for clone syscall test. Remove general call to kfail, pass extra variable to check_pc_after_cross_syscall, call setup_kfail when appropriate. --- gdb/testsuite/ChangeLog | 16 +++++ gdb/testsuite/gdb.base/step-over-clone.c | 39 +++++++++++ gdb/testsuite/gdb.base/step-over-syscall.exp | 69 ++++++++++++++++---- 3 files changed, 113 insertions(+), 11 deletions(-) diff --git a/gdb/testsuite/gdb.base/step-over-clone.c b/gdb/testsuite/gdb.base/step-over-clone.c index 581bf5fdde5..ef6fd922eb1 100644 --- a/gdb/testsuite/gdb.base/step-over-clone.c +++ b/gdb/testsuite/gdb.base/step-over-clone.c @@ -19,6 +19,7 @@ #include #include #include +#include static void marker () @@ -26,9 +27,22 @@ marker () #define STACK_SIZE 0x1000 +/* These are used to signal that the threads have started correctly. The + GLOBAL_THREAD_COUNT is set to the number of threads in main, then + decremented (under a lock) in each new thread. */ +pthread_mutex_t global_lock = PTHREAD_MUTEX_INITIALIZER; +int global_thread_count = 0; + static int clone_fn (void *unused) { + /* Signal that this thread has started correctly. */ + if (pthread_mutex_lock (&global_lock) != 0) + abort (); + global_thread_count--; + if (pthread_mutex_unlock (&global_lock) != 0) + abort (); + return 0; } @@ -38,9 +52,21 @@ main (void) int i, pid; unsigned char *stack[6]; + /* Due to bug gdb/19675 the cloned thread _might_ try to reenter main + (this depends on where the displaced instruction is placed for + execution). However, if we do reenter main then lets ensure we fail + hard rather then just silently executing the code below. */ + static int started = 0; + if (!started) + started = 1; + else + abort (); + for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++) stack[i] = malloc (STACK_SIZE); + global_thread_count = (sizeof (stack) / sizeof (stack[0])); + for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++) { pid = clone (clone_fn, stack[i] + STACK_SIZE, CLONE_FILES | CLONE_VM, @@ -50,5 +76,18 @@ main (void) for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++) free (stack[i]); + /* Set an alarm so we don't end up stuck waiting for threads that might + never start correctly. */ + alarm (120); + + /* Now wait for all the threads to start up. */ + while (global_thread_count != 0) + { + /* Force memory barrier so GLOBAL_THREAD_COUNT will be refetched. */ + asm volatile ("" ::: "memory"); + sleep (1); + } + + /* Call marker, this is what GDB is waiting for. */ marker (); } diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp index ecfb7be481d..5d4a75f810b 100644 --- a/gdb/testsuite/gdb.base/step-over-syscall.exp +++ b/gdb/testsuite/gdb.base/step-over-syscall.exp @@ -41,11 +41,50 @@ if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } { return -1 } -proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr } { +proc_with_prefix check_pc_after_cross_syscall { displaced syscall syscall_insn_next_addr } { + global gdb_prompt + set syscall_insn_next_addr_found [get_hexadecimal_valueof "\$pc" "0"] + # After the 'stepi' we expect thread 1 to still be selected. + # However, when displaced stepping over a clone bug gdb/19675 + # means this might not be the case. + # + # Which thread we end up in depends on a race between the original + # thread-1, and the new thread (created by the clone), so we can't + # guarantee which thread we will be in at this point. + # + # For the fork/vfork syscalls, which are correctly handled by + # displaced stepping we will always be in thread-1 or the original + # process at this point. + set curr_thread "unknown" + gdb_test_multiple "info threads" "" { + -re "Id\\s+Target Id\\s+Frame\\s*\r\n" { + exp_continue + } + -re "^\\* (\\d+)\\s+\[^\r\n\]+\r\n" { + set curr_thread $expect_out(1,string) + exp_continue + } + -re "^\\s+\\d+\\s+\[^\r\n\]+\r\n" { + exp_continue + } + -re "$gdb_prompt " { + } + } + + # If we are displaced stepping over a clone, and we ended up in + # the wrong thread then the following check of the $pc value will + # fail. + if { $displaced == "on" && $syscall == "clone" && $curr_thread != 1 } { + # GDB doesn't support stepping over clone syscall with + # displaced stepping. + setup_kfail "*-*-*" "gdb/19675" + } + gdb_assert {$syscall_insn_next_addr != 0 \ - && $syscall_insn_next_addr == $syscall_insn_next_addr_found} \ + && $syscall_insn_next_addr == $syscall_insn_next_addr_found \ + && $curr_thread == 1} \ "single step over $syscall final pc" } @@ -203,7 +242,12 @@ proc step_over_syscall { syscall } { set testfile "step-over-$syscall" - if [build_executable ${testfile}.exp ${testfile} ${testfile}.c {debug}] { + set options [list debug] + if { $syscall == "clone" } { + lappend options "pthreads" + } + + if [build_executable ${testfile}.exp ${testfile} ${testfile}.c $options] { untested "failed to compile" return -1 } @@ -213,13 +257,6 @@ proc step_over_syscall { syscall } { continue } - if { $displaced == "on" && $syscall == "clone" } { - # GDB doesn't support stepping over clone syscall with - # displaced stepping. - kfail "gdb/19675" "single step over clone" - continue - } - set ret [setup $syscall] set syscall_insn_addr [lindex $ret 0] @@ -256,12 +293,22 @@ proc step_over_syscall { syscall } { if {[gdb_test "stepi" "x/i .*=>.*" "single step over $syscall"] != 0} { return -1 } - check_pc_after_cross_syscall $syscall $syscall_insn_next_addr + check_pc_after_cross_syscall $displaced $syscall $syscall_insn_next_addr # Delete breakpoint syscall insns to avoid interference to other syscalls. delete_breakpoints gdb_test "break marker" "Breakpoint.*at.* file .*${testfile}.c, line.*" + + # If we are displaced stepping over a clone syscall then + # we expect the following check to fail. See also the + # code in check_pc_after_cross_syscall. + if { $displaced == "on" && $syscall == "clone" } { + # GDB doesn't support stepping over clone syscall with + # displaced stepping. + setup_kfail "*-*-*" "gdb/19675" + } + gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, marker \\(\\) at.*" \ "continue to marker ($syscall)" } -- 2.25.4