From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 77F28383D015 for ; Mon, 5 Jul 2021 18:47:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 77F28383D015 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 165IkwPJ005916 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 5 Jul 2021 14:47:03 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 165IkwPJ005916 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 2C8AB1E939; Mon, 5 Jul 2021 14:46:57 -0400 (EDT) Subject: Re: [PATCH 1/3] gdb/testsuite: update test gdb.base/step-over-syscall.exp To: Andrew Burgess , gdb-patches@sourceware.org References: <11296b5772ede552372625ac37df166117940add.1625081357.git.andrew.burgess@embecosm.com> From: Simon Marchi Message-ID: <0d4dd2ad-36aa-5b2d-0321-e141a861a4e4@polymtl.ca> Date: Mon, 5 Jul 2021 14:46:57 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <11296b5772ede552372625ac37df166117940add.1625081357.git.andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 5 Jul 2021 18:46:58 +0000 X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 05 Jul 2021 18:47:05 -0000 On 2021-06-30 3:55 p.m., Andrew Burgess wrote: > 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. That looks reasonable to me. Making the test fail more consistently / be less random is a good thing. Simon