From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 7539C3858D1E for ; Thu, 10 Nov 2022 17:33:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7539C3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C1EC91E0D3; Thu, 10 Nov 2022 12:33:28 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1668101609; bh=Ta3wga3XtbnnelCl6OQcGsJ8zxE+CeedyVlb+9YD/qo=; h=Date:Subject:To:References:From:In-Reply-To:From; b=H9xa/AeKJBEWaxRudAkc4gKqaNrOFjAHHc3MYMeEdEozbYRHdR8TM5U8u1tNfQRvR 7vVmR+LmgpbQPGpIwY3pIFGaosf9UTeXGfxUfgYeH2tYpdlZIxhBkQxVjWo79pDTcO mKzUZPjTXxXJE3K4wTutdBBbW6X/O/CL03Aa4y5Y= Message-ID: <0ebeeb02-db83-53ac-2bcb-05dc92b96316@simark.ca> Date: Thu, 10 Nov 2022 12:33:28 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH v2] gdb: make "start" breakpoint inferior-specific Content-Language: en-US To: Pedro Alves , Simon Marchi , gdb-patches@sourceware.org References: <691c5a58-68ae-5fe9-2f3d-34fb7af69ad0@palves.net> <20221108212008.1792090-1-simon.marchi@efficios.com> <47b696c4-6584-8165-0799-5d742132361a@palves.net> From: Simon Marchi In-Reply-To: <47b696c4-6584-8165-0799-5d742132361a@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,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 List-Id: On 11/10/22 11:45, Pedro Alves wrote: > On 2022-11-08 9:20 p.m., Simon Marchi via Gdb-patches wrote: >> New in v2: >> >> - Change the test so it doesn't call the main function >> >> I saw this failure on a CI: >> >> (gdb) add-inferior >> [New inferior 2] >> Added inferior 2 >> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: add-inferior >> inferior 2 >> [Switching to inferior 2 [] ()] >> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: inferior 2 >> kill >> The program is not being run. >> (gdb) file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep >> Reading symbols from /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep... >> (gdb) run & >> Starting program: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior-sleep >> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: run inferior 2 >> inferior 1 >> [Switching to inferior 1 [] ()] >> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: inferior 1 >> kill >> The program is not being run. >> (gdb) file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior >> Reading symbols from /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior... >> (gdb) break should_break_here >> Breakpoint 1 at 0x11b1: file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.threads/vfork-multi-inferior.c, line 25. >> (gdb) PASS: gdb.threads/vfork-multi-inferior.exp: method=non-stop: break should_break_here >> [Thread debugging using libthread_db enabled] >> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". >> start >> Temporary breakpoint 2 at 0x11c0: -qualified main. (2 locations) >> Starting program: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/tmp/tmp.GYATAXR8Ku/gdb/testsuite/outputs/gdb.threads/vfork-multi-inferior/vfork-multi-inferior >> [Thread debugging using libthread_db enabled] >> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". >> >> Thread 2.1 "vfork-multi-inf" hit Temporary breakpoint 2, main () at /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.threads/vfork-multi-inferior-sleep.c:23 >> 23 sleep (30); >> (gdb) FAIL: gdb.threads/vfork-multi-inferior.exp: method=non-stop: start inferior 1 >> >> What happens is: >> >> 1. We start inferior 2 with "run&", it runs very slowly, takes time to >> get to main >> 2. We switch to inferior 1, and run "start" >> 3. The temporary breakpoint inserted by "start" applies to all inferiors >> 4. Inferior 2 hits that breakpoint and GDB reports that hit >> >> To avoid this, breakpoints inserted by "start" should be >> inferior-specific. However, we don't have a nice way to make >> inferior-specific breakpoints yet. It's possible to make >> pspace-specific breakpoints (for example how the internal_breakpoint >> constructor does) by creating a symtab_and_line manually. However, >> inferiors can share program spaces (usually on particular embedded >> targets), so we could have a situation where two inferiors run the same >> code in the same program space. In that case, it would just not be >> possible to insert a breakpoint in one inferior but not the other. >> >> A simple solution that should work all the time is to add a condition to >> the breakpoint inserted by "start", to check the inferior reporting the >> hit is the expected one. This is what this patch implements. >> > > Even though this does work, it still sets the breakpoint on all the pspaces > unnecessarily. It would be nice if the breakpoint was pspace specific, in > addition to inferior specific like you have (or some other way). But, what you > have is fine with me as is, as it is better than what we have today. > Maybe just add a little comment suggesting that it would be even better > to make the breakpoint apply to the current pspace only? Will do. >> Add a test that does: >> >> - start in background inferior 1 that sleeps 3 seconds before reaching >> its main function (using a sleep in a global C++ object constructor) >> - start inferior 2, which also sleeps 3 seconds before reaching its m >> ain function, with the "start" command >> - validate that we hit the breakpoint in inferior 2 >> >> Without the fix, we hit the breakpoint in inferior 1 pretty much all the >> time. There could be some unfortunate scheduling causing the test not >> to catch the bug, for instance if the scheduler decides not to schedule >> inferior 1 for a long time, but it would be really rare. If the bug is >> re-introduced, the test will catch it much more often than not, so it >> will be noticed. > > My thinking when I saw that both inferiors wait 3 seconds before reaching > main (before reading the commit log), was, "???, I don't understand this." > > So this is assuming that because the first inferior was started first, that > its 3 seconds always finish before the second inferior's 3 seconds? That > seems a bit risky. gdb and the other inferiors may all be running on > different cores, and on a fast machine, gdb may be fast enough to spawn > both processes roughtly at the same time, and then inferior 2 may end up > reporting the hit first. > > Why not make it so that inferior 3 takes like 3 seconds to reach main, > but inferior 2 takes 4 or 5 seconds? Or 2 vs 4. Something like that. > I.e., make sure that inferior 2's time is larger than inferior 1's. > That could be done by tweaking the sleep calls in the programs, or > adding a sleep call in the .exp file between "run&" and starting the > second inferior. > > But maybe I'm missing something? Hmm no, you're right, there's no reason for the two sleep times to be equal. I'll do 2 seconds for inferior 1 and 4 seconds for inferior 2. >> +proc do_test {} { >> + # With remote, to be able to start an inferior while another one is >> + # running, we need to use the non-stop variant of the protocol. >> + save_vars { ::GDBFLAGS } { >> + if { [target_info gdb_protocol] == "extended-remote"} { >> + append ::GDBFLAGS " -ex \"maintenance set target-non-stop on\"" >> + } >> + >> + clean_restart ${::binfile_other} >> + } >> + >> + gdb_test "run&" "Starting program: .*" "start background inferior" > > I was going to point out that if the inferior prints something, this can > timeout, as that output would appear after the prompt. I then looked around > the tree for "run&" uses, to confirm we are using gdb_test_multiple with that, > and found that you just recently added "gdb_test -no-prompt-anchor", for exactly > this scenario. :-) I think that should be used here. Even if, in this case, we know the inferior won't print anything? Simon