Message ID | 20221108212008.1792090-1-simon.marchi@efficios.com |
---|---|
State | New |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> 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 4809E3858426 for <patchwork@sourceware.org>; Tue, 8 Nov 2022 21:20:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4809E3858426 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1667942450; bh=fuYnqs25AVxs9hQpVf0auhLFr3j7Xr/aedWpxetM+Fo=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BAERZdBECoDkn8rHd6a9PbWtAbjV+O2q5aDo8rhQg4wKnm/waWHYJ0Wn/kBw0iXHj QwUTfAHSBsAERR3zKQXvAwm0TNZbkhiER8nsE7uAgO6119ANdA7ZHGrB/GKpTx0lo3 HuxodZMSh7t/Cy4tjSCy4qdcRzfjfxtvqQHAj4Z4= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 298913858403 for <gdb-patches@sourceware.org>; Tue, 8 Nov 2022 21:20:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 298913858403 X-ASG-Debug-ID: 1667942408-0c856e167f3e8c90001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id 4EIOYGsvSP8PImNw (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Tue, 08 Nov 2022 16:20:08 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id D4F02441B21; Tue, 8 Nov 2022 16:20:08 -0500 (EST) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@efficios.com> Subject: [PATCH v2] gdb: make "start" breakpoint inferior-specific Date: Tue, 8 Nov 2022 16:20:08 -0500 X-ASG-Orig-Subj: [PATCH v2] gdb: make "start" breakpoint inferior-specific Message-Id: <20221108212008.1792090-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <691c5a58-68ae-5fe9-2f3d-34fb7af69ad0@palves.net> References: <691c5a58-68ae-5fe9-2f3d-34fb7af69ad0@palves.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1667942408 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 11132 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.102001 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-3498.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, 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 <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Simon Marchi <simon.marchi@efficios.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[v2] gdb: make "start" breakpoint inferior-specific
|
|
Commit Message
Simon Marchi
Nov. 8, 2022, 9:20 p.m. UTC
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 [<null>] (<noexec>)] (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 [<null>] (<noexec>)] (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. 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. Change-Id: Ib0148498a476bfa634ed62353c95f163623c686a --- gdb/infcmd.c | 3 +- .../start-inferior-specific-other.cc | 35 +++++++++++ .../gdb.multi/start-inferior-specific.cc | 32 ++++++++++ .../gdb.multi/start-inferior-specific.exp | 61 +++++++++++++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific-other.cc create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific.cc create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific.exp
Comments
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 [<null>] (<noexec>)] > (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 [<null>] (<noexec>)] > (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? > 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? > +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. > + gdb_test "add-inferior" "Added inferior 2.*" > + gdb_test "inferior 2" "Switching to inferior 2.*" > + gdb_file_cmd ${::binfile} > + gdb_test "start" "Thread 2.1 .* hit Temporary breakpoint .*" > +} > +
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 [<null>] (<noexec>)] >> (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 [<null>] (<noexec>)] >> (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
>> 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. FWIW, here's what I'm adding /* To avoid other inferiors hitting this breakpoint, make it inferior-specific using a condition. A better solution would be to have proper inferior-specific breakpoint support, in the breakpoint machinery. We could then avoid inserting a breakpoint in the program spaces unrelated to this inferior. */ Simon
On 2022-11-10 5:33 p.m., Simon Marchi wrote: >>> +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? Admitedly it's a bit pedantic, but it seems to me to be safer. Say someones adds some logging to the program or something. It just looks like good practice to me to not have an anchor when the inferior is left running after the prompt is printed.
On 11/10/22 12:47, Pedro Alves wrote: > On 2022-11-10 5:33 p.m., Simon Marchi wrote: > >>>> +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? > > Admitedly it's a bit pedantic, but it seems to me to be safer. Say > someones adds some logging to the program or something. It just looks > like good practice to me to not have an anchor when the inferior is left > running after the prompt is printed. Ack, I will add it. Simon
On 11/8/22 22:20, Simon Marchi via Gdb-patches wrote: > - std::string arg = string_printf ("-qualified %s", main_name ()); > + std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), > + current_inferior ()->num); Hi, it seems ada doesn't like the syntax, we get: ... (gdb) start ^M Error in expression, near `1'.^M (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure ... Thanks, - Tom (gdb) bt #0 0x00007ffff4c3dad2 in __cxxabiv1::__cxa_throw (obj=0x2d8cef0, tinfo=0x1471210 <typeinfo for gdb_exception_error>, dest=0xb70b06 <gdb_exception_error::~gdb_exception_error()>) at ../../../../libstdc++-v3/libsupc++/eh_throw.cc:78 #1 0x000000000141eb12 in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0x14789a8 "Error in expression, near `%s'.", ap=0x7fffffffb258) at /home/vries/gdb_versions/devel/src/gdbsupport/common-exceptions.cc:200 #2 0x000000000141eb8e in throw_verror (error=GENERIC_ERROR, fmt=0x14789a8 "Error in expression, near `%s'.", ap=0x7fffffffb258) at /home/vries/gdb_versions/devel/src/gdbsupport/common-exceptions.cc:208 #3 0x0000000000cce1d2 in verror (string=0x14789a8 "Error in expression, near `%s'.", args=0x7fffffffb258) at /home/vries/gdb_versions/devel/src/gdb/utils.c:164 #4 0x0000000001423811 in error (fmt=0x14789a8 "Error in expression, near `%s'.") at /home/vries/gdb_versions/devel/src/gdbsupport/errors.cc:46 #5 0x000000000043b90d in ada_yyerror (msg=0x147623a "syntax error") at /home/vries/gdb_versions/devel/src/gdb/ada-exp.y:1169 #6 0x000000000043793f in ada_yyparse () at ada-exp.c.tmp:2905 #7 0x000000000043b818 in ada_parse (par_state=0x7fffffffcca0) at /home/vries/gdb_versions/devel/src/gdb/ada-exp.y:1155 #8 0x000000000047b418 in ada_language::parser (this=0x29937d0 <ada_language_defn>, ps=0x7fffffffcca0) at /home/vries/gdb_versions/devel/src/gdb/ada-lang.c:13855 #9 0x00000000009bdcd2 in parse_exp_in_context (stringptr=0x7fffffffce08, pc=4202360, block=0x37c2280, comma=0, void_context_p=false, tracker=0x7fffffffcd20, completer=0x0) at /home/vries/gdb_versions/devel/src/gdb/parse.c:515 #10 0x00000000009bda5f in parse_exp_1 (stringptr=0x7fffffffce08, pc=4202360, block=0x37c2280, comma=0, tracker=0x0) at /home/vries/gdb_versions/devel/src/gdb/parse.c:428 #11 0x0000000000567921 in find_condition_and_thread (tok=0x376f439 "$_inferior == 1", pc=4202360, cond_string=0x7fffffffcec8, thread=0x7fffffffcec4, task=0x7fffffffcec0, rest=0x7fffffffceb8) at /home/vries/gdb_versions/devel/src/gdb/breakpoint.c:8649 #12 0x0000000000567c78 in find_condition_and_thread_for_sals ( sals=std::vector of length 1, capacity 1 = {...}, input=0x376f436 "if $_inferior == 1", cond_string=0x7fffffffcf78, thread=0x7fffffffcf34, task=0x7fffffffcfbc, rest=0x7fffffffcf80) at /home/vries/gdb_versions/devel/src/gdb/breakpoint.c:8728 #13 0x0000000000568389 in create_breakpoint (gdbarch=0x2d8d1e0, locspec=0x376f500, cond_string=0x0, thread=0, extra_string=0x376f436 "if $_inferior == 1", force_condition=false, parse_extra=1, tempflag=1, type_wanted=bp_breakpoint, ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO, ops=0x14afec0 <code_breakpoint_ops>, from_tty=0, enabled=1, internal=0, flags=0) at /home/vries/gdb_versions/devel/src/gdb/breakpoint.c:8909 #14 0x0000000000568be2 in break_command_1 (arg=0x376f436 "if $_inferior == 1", flag=1, from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/breakpoint.c:9027 #15 0x0000000000568eb6 in tbreak_command ( arg=0x376f420 "-qualified _ada_dummy if $_inferior == 1", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/breakpoint.c:9104 #16 0x000000000084f040 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_STOP_AT_MAIN) at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:433 #17 0x000000000084f555 in start_command (args=0x0, from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:537 #18 0x00000000005e9bd6 in do_simple_func (args=0x0, from_tty=0, c=0x2bcc0a0) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-decode.c:95 #19 0x00000000005ee986 in cmd_func (cmd=0x2bcc0a0, args=0x0, from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-decode.c:2543 #20 0x0000000000c3648f in execute_command (p=0x7fffffffe14f "", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/top.c:692 #21 0x00000000008faa55 in catch_command_errors ( command=0xc35ec2 <execute_command(char const*, int)>, arg=0x7fffffffe14a "start", from_tty=0, do_bp_actions=true) at /home/vries/gdb_versions/devel/src/gdb/main.c:513 #22 0x00000000008fac2d in execute_cmdargs (cmdarg_vec=0x7fffffffd7a0, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd77c) at /home/vries/gdb_versions/devel/src/gdb/main.c:608 #23 0x00000000008fbfbd in captured_main_1 (context=0x7fffffffd9e0) at /home/vries/gdb_versions/devel/src/gdb/main.c:1299 #24 0x00000000008fc1c0 in captured_main (data=0x7fffffffd9e0) at /home/vries/gdb_versions/devel/src/gdb/main.c:1320 #25 0x00000000008fc22b in gdb_main (args=0x7fffffffd9e0) at /home/vries/gdb_versions/devel/src/gdb/main.c:1345 #26 0x000000000041a24e in main (argc=13, argv=0x7fffffffdaf8) at /home/vries/gdb_versions/devel/src/gdb/gdb.c:32
On 11/11/22 07:37, Tom de Vries wrote: > On 11/8/22 22:20, Simon Marchi via Gdb-patches wrote: >> - std::string arg = string_printf ("-qualified %s", main_name ()); >> + std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), >> + current_inferior ()->num); > > Hi, > > it seems ada doesn't like the syntax, we get: > ... > (gdb) start ^M > Error in expression, near `1'.^M > (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure Huh, sorry, I missed it because it shows up as UNTESTED, which my CI job doesn't flag as a failure. Here's a patch that fixes it in a rather naive way. Ideally, we would implement proper inferior-specific breakpoints, but in any case we want un-break the tests sooner than that. From 28f370e7dda4fb2f240ed29493416e78ed47f176 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Fri, 11 Nov 2022 07:58:35 -0500 Subject: [PATCH] gdb: fix start breakpoint expression not working in some languages Commit 0be837be9fb4 ("gdb: make "start" breakpoint inferior-specific") regresses gdb.ada/start.exp: (gdb) start Error in expression, near `1'. (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure This is because in Ada, the equality operator is =, not ==. I checked the other languages supported by GDB, these other languages use = for equality: - Pascal: tests like gdb.pascal/hello.exp are affected too - Modula-2: I tried building a Modula-2 hello world using gm2, but it seems like the generated DWARF doesn't specify the Modula-2 language in the CUs, it's C++ and C, so the selected language isn't "modula-2". But if I manually do "set language modula-2" on a dummy program and then "start", I get the same error. Other languages all use ==. So, a short term fix would be to use = or == in the expression, based on the current language. If this was meant to be permanent, I would suggest adding something like an "equality_operator" method to language_defn, that returns the right equality operator for the language. But the goal is to replace all this with proper inferior-specific breakpoints, so I hope all this is temporary. Change-Id: Id4d38e14a80e6bbbb1ad2b2277f974dd55192969 --- gdb/infcmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index bf4a68e3557e..6f83949cc7c0 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -428,8 +428,13 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) have proper inferior-specific breakpoint support, in the breakpoint machinery. We could then avoid inserting a breakpoint in the program spaces unrelated to this inferior. */ - std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), - current_inferior ()->num); + const char *op + = ((current_language->la_language == language_ada + || current_language->la_language == language_pascal + || current_language->la_language == language_m2) ? "=" : "=="); + std::string arg = string_printf + ("-qualified %s if $_inferior %s %d", main_name (), op, + current_inferior ()->num); tbreak_command (arg.c_str (), 0); } base-commit: 70b9d05b26e861524d70ee90dcd28cfd77032ddd
On 11/11/22 14:53, Simon Marchi wrote: > On 11/11/22 07:37, Tom de Vries wrote: >> On 11/8/22 22:20, Simon Marchi via Gdb-patches wrote: >>> - std::string arg = string_printf ("-qualified %s", main_name ()); >>> + std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), >>> + current_inferior ()->num); >> >> Hi, >> >> it seems ada doesn't like the syntax, we get: >> ... >> (gdb) start ^M >> Error in expression, near `1'.^M >> (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure > > Huh, sorry, I missed it because it shows up as UNTESTED, which my CI job > doesn't flag as a failure. > I only noticed by glancing at gdb.log scrolling by, which got stuck waiting for "Starting program:" to appear. Which I've just realized is a testsuite error, so I've fixed this with "[gdb/testsuite] Don't timeout on prompt in gdb_start_cmd". > Here's a patch that fixes it in a rather naive way. Ideally, we would > implement proper inferior-specific breakpoints, but in any case we want > un-break the tests sooner than that. > It fixes the "UNTESTED" for me, and LGTM. I did wonder if this could be fixed in a way that the expression is parsed independent of the current language, setting language to say C for the duration of the command. And that does seem to work: ... diff --git a/gdb/infcmd.c b/gdb/infcmd.c index bf4a68e3557..f7b1d763838 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -430,7 +430,14 @@ run_command_1 (const char *args, int from_tty, enum run_ how run_how) spaces unrelated to this inferior. */ std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_nam e (), current_inferior ()->num); - tbreak_command (arg.c_str (), 0); + { + scoped_restore_current_language save_language; + scoped_restore save_language_mode + = make_scoped_restore (&language_mode); + language_mode = language_mode_manual; + current_language = language_def (language_c); + tbreak_command (arg.c_str (), 0); + } } exec_file = get_exec_file (0); ... I'm not sure if this is a better solution: it's more intrusive. Thanks, - Tom > From 28f370e7dda4fb2f240ed29493416e78ed47f176 Mon Sep 17 00:00:00 2001 > From: Simon Marchi <simon.marchi@polymtl.ca> > Date: Fri, 11 Nov 2022 07:58:35 -0500 > Subject: [PATCH] gdb: fix start breakpoint expression not working in some > languages > > Commit 0be837be9fb4 ("gdb: make "start" breakpoint inferior-specific") > regresses gdb.ada/start.exp: > > (gdb) start > Error in expression, near `1'. > (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure > > This is because in Ada, the equality operator is =, not ==. > > I checked the other languages supported by GDB, these other languages > use = for equality: > > - Pascal: tests like gdb.pascal/hello.exp are affected too > - Modula-2: I tried building a Modula-2 hello world using gm2, but it > seems like the generated DWARF doesn't specify the Modula-2 language > in the CUs, it's C++ and C, so the selected language isn't > "modula-2". But if I manually do "set language modula-2" on a dummy > program and then "start", I get the same error. > > Other languages all use ==. > > So, a short term fix would be to use = or == in the expression, based on > the current language. If this was meant to be permanent, I would > suggest adding something like an "equality_operator" method to > language_defn, that returns the right equality operator for the > language. But the goal is to replace all this with proper > inferior-specific breakpoints, so I hope all this is temporary. > > Change-Id: Id4d38e14a80e6bbbb1ad2b2277f974dd55192969 > --- > gdb/infcmd.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index bf4a68e3557e..6f83949cc7c0 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -428,8 +428,13 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) > have proper inferior-specific breakpoint support, in the breakpoint > machinery. We could then avoid inserting a breakpoint in the program > spaces unrelated to this inferior. */ > - std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), > - current_inferior ()->num); > + const char *op > + = ((current_language->la_language == language_ada > + || current_language->la_language == language_pascal > + || current_language->la_language == language_m2) ? "=" : "=="); > + std::string arg = string_printf > + ("-qualified %s if $_inferior %s %d", main_name (), op, > + current_inferior ()->num); > tbreak_command (arg.c_str (), 0); > } > > > base-commit: 70b9d05b26e861524d70ee90dcd28cfd77032ddd
On 11/11/22 10:21, Tom de Vries via Gdb-patches wrote: > On 11/11/22 14:53, Simon Marchi wrote: >> On 11/11/22 07:37, Tom de Vries wrote: >>> On 11/8/22 22:20, Simon Marchi via Gdb-patches wrote: >>>> - std::string arg = string_printf ("-qualified %s", main_name ()); >>>> + std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), >>>> + current_inferior ()->num); >>> >>> Hi, >>> >>> it seems ada doesn't like the syntax, we get: >>> ... >>> (gdb) start ^M >>> Error in expression, near `1'.^M >>> (gdb) UNTESTED: gdb.ada/start.exp: start failed to land inside the right procedure >> >> Huh, sorry, I missed it because it shows up as UNTESTED, which my CI job >> doesn't flag as a failure. >> > > I only noticed by glancing at gdb.log scrolling by, which got stuck waiting for "Starting program:" to appear. Which I've just realized is a testsuite error, so I've fixed this with "[gdb/testsuite] Don't timeout on prompt in gdb_start_cmd". Thanks. I think it's strange for these tests to emit an UNTESTED if gdb_start_cmd fails. Clearly, something is wrong if that happens. I'll send a patch that changes them to fail. > >> Here's a patch that fixes it in a rather naive way. Ideally, we would >> implement proper inferior-specific breakpoints, but in any case we want >> un-break the tests sooner than that. >> > > It fixes the "UNTESTED" for me, and LGTM. > > I did wonder if this could be fixed in a way that the expression is parsed independent of the current language, setting language to say C for the duration of the command. And that does seem to work: > ... > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index bf4a68e3557..f7b1d763838 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -430,7 +430,14 @@ run_command_1 (const char *args, int from_tty, enum run_ > how run_how) > spaces unrelated to this inferior. */ > std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_nam > e (), > current_inferior ()->num); > - tbreak_command (arg.c_str (), 0); > + { > + scoped_restore_current_language save_language; > + scoped_restore save_language_mode > + = make_scoped_restore (&language_mode); > + language_mode = language_mode_manual; > + current_language = language_def (language_c); > + tbreak_command (arg.c_str (), 0); > + } > } > > exec_file = get_exec_file (0); > ... > > I'm not sure if this is a better solution: it's more intrusive. Ah, that would be good too. We wouldn't have to bake in the knowledge of which languages use which operator. But I'm also a bit scared of other unintended consequences when looking up the main symbol, as I see the current_language is involved in some places. To be safe, I'll just go with my naive patch. Thanks for the suggestion. Simon
On 11/11/22 20:03, Simon Marchi wrote: > Thanks. I think it's strange for these tests to emit an UNTESTED if > gdb_start_cmd fails. Clearly, something is wrong if that happens. I'll > send a patch that changes them to fail. > Yes, that's a good point. After thinking about this a bit, I've filed a PR for an overall revision of the untested usage in gdb testsuite ( https://sourceware.org/bugzilla/show_bug.cgi?id=29778 ). Thanks, - Tom
On 11/11/22 20:03, Simon Marchi via Gdb-patches wrote: >> I'm not sure if this is a better solution: it's more intrusive. > Ah, that would be good too. We wouldn't have to bake in the knowledge > of which languages use which operator. But I'm also a bit scared of > other unintended consequences when looking up the main symbol, as I see > the current_language is involved in some places. To be safe, I'll just > go with my naive patch. Thanks for the suggestion. FWIW, I've just stumbled onto language unknown, and realized that strictly speaking we've got a regression because we used to have: ... $ gdb -q -batch a.out -ex "set language unknown" -ex start Temporary breakpoint 1 at 0x40050b: file /home/vries/hello.c, line 6. Temporary breakpoint 1, main () at /home/vries/hello.c:6 6 printf ("hello\n"); Warning: the current language does not match this frame. $ ... but now we have: ... $ gdb -q -batch a.out -ex "set language unknown" -ex start expression parsing not implemented for language "Unknown" $ ... I don't really care about this, I thought I just mention it. Thanks, - Tom
On 11/14/22 06:29, Tom de Vries wrote: > On 11/11/22 20:03, Simon Marchi via Gdb-patches wrote: >>> I'm not sure if this is a better solution: it's more intrusive. >> Ah, that would be good too. We wouldn't have to bake in the knowledge >> of which languages use which operator. But I'm also a bit scared of >> other unintended consequences when looking up the main symbol, as I see >> the current_language is involved in some places. To be safe, I'll just >> go with my naive patch. Thanks for the suggestion. > > FWIW, I've just stumbled onto language unknown, and realized that strictly speaking we've got a regression because we used to have: > ... > $ gdb -q -batch a.out -ex "set language unknown" -ex start > Temporary breakpoint 1 at 0x40050b: file /home/vries/hello.c, line 6. > > Temporary breakpoint 1, main () at /home/vries/hello.c:6 > 6 printf ("hello\n"); > Warning: the current language does not match this frame. > $ > ... > but now we have: > ... > $ gdb -q -batch a.out -ex "set language unknown" -ex start > expression parsing not implemented for language "Unknown" > $ > ... > > I don't really care about this, I thought I just mention it. Huh... What is "set language unknown" useful for, generally speaking? If debugging something in a language GDB doesn't know about, I think it will fall back to minimal. Could we just not expose the "unknown" language to the user? Simon
On 11/14/22 14:19, Simon Marchi wrote: > > > On 11/14/22 06:29, Tom de Vries wrote: >> On 11/11/22 20:03, Simon Marchi via Gdb-patches wrote: >>>> I'm not sure if this is a better solution: it's more intrusive. >>> Ah, that would be good too. We wouldn't have to bake in the knowledge >>> of which languages use which operator. But I'm also a bit scared of >>> other unintended consequences when looking up the main symbol, as I see >>> the current_language is involved in some places. To be safe, I'll just >>> go with my naive patch. Thanks for the suggestion. >> >> FWIW, I've just stumbled onto language unknown, and realized that strictly speaking we've got a regression because we used to have: >> ... >> $ gdb -q -batch a.out -ex "set language unknown" -ex start >> Temporary breakpoint 1 at 0x40050b: file /home/vries/hello.c, line 6. >> >> Temporary breakpoint 1, main () at /home/vries/hello.c:6 >> 6 printf ("hello\n"); >> Warning: the current language does not match this frame. >> $ >> ... >> but now we have: >> ... >> $ gdb -q -batch a.out -ex "set language unknown" -ex start >> expression parsing not implemented for language "Unknown" >> $ >> ... >> >> I don't really care about this, I thought I just mention it. > > Huh... > > What is "set language unknown" useful for, generally speaking? If > debugging something in a language GDB doesn't know about, I think it > will fall back to minimal. Could we just not expose the "unknown" > language to the user? Yeah, interestingly it also doesn't show up in the help: ... $ gdb (gdb) help set language Set the current source language. The currently understood settings are: local or auto Automatic setting based on source file c Use the C language objective-c Use the Objective-C language c++ Use the C++ language d Use the D language go Use the Go language fortran Use the Fortran language modula-2 Use the Modula-2 language asm Use the Assembly language pascal Use the Pascal language opencl Use the OpenCL C language rust Use the Rust language minimal Use the Minimal language ada Use the Ada language ... Though that may have been an oversight, I'm not sure. Looking through the testsuite, there are a few uses: ... $ find gdb/testsuite/ -type f | xargs grep "set lang.*unknown" gdb/testsuite/gdb.base/parse_number.exp:gdb_test_no_output "set language unknown" gdb/testsuite/gdb.base/langs.exp: gdb_test_no_output "set language unknown" gdb/testsuite/gdb.base/default.exp:gdb_test "set language" "Requires an argument. Valid arguments are auto, local, unknown, ada, asm, c, c.., d, fortran, go, minimal, modula-2, objective-c, opencl, pascal, rust." gdb/testsuite/gdb.cp/demangle.exp: gdb_test_no_output "set language unknown" ... But I think removing it from the set language command, and replacing it with "maint set language unknown" should be ok. Thanks, - Tom Thanks, - Tom
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> What is "set language unknown" useful for, generally speaking? If
Simon> debugging something in a language GDB doesn't know about, I think it
Simon> will fall back to minimal. Could we just not expose the "unknown"
Simon> language to the user?
I wonder if we can remove it entirely and replace it with "minimal".
Tom
On 11/16/22 11:22, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: > > Simon> What is "set language unknown" useful for, generally speaking? If > Simon> debugging something in a language GDB doesn't know about, I think it > Simon> will fall back to minimal. Could we just not expose the "unknown" > Simon> language to the user? > > I wonder if we can remove it entirely and replace it with "minimal". > > Tom I think so. I started a patch here [1] but haven't had time to follow up on it. In the uses of "set lang unknown" in the testsuite, two are to verify that trying to print an expression while the unknown language is selected gives an error message and does not crash, we don't care about those. The other one, in gdb.cp/demangle.exp, I think it's to make sure that the current language is something else than the language we pass to "demangle -l". I think using the minimal language there should be fine. Simon [1] https://review.lttng.org/c/binutils-gdb/+/8952
diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c03ca103c91..f91ae33eb25 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -423,7 +423,8 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) /* Insert temporary breakpoint in main function if requested. */ if (run_how == RUN_STOP_AT_MAIN) { - std::string arg = string_printf ("-qualified %s", main_name ()); + std::string arg = string_printf ("-qualified %s if $_inferior == %d", main_name (), + current_inferior ()->num); tbreak_command (arg.c_str (), 0); } diff --git a/gdb/testsuite/gdb.multi/start-inferior-specific-other.cc b/gdb/testsuite/gdb.multi/start-inferior-specific-other.cc new file mode 100644 index 00000000000..e6f55d326bb --- /dev/null +++ b/gdb/testsuite/gdb.multi/start-inferior-specific-other.cc @@ -0,0 +1,35 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <unistd.h> + +struct global +{ + global () + { + sleep (3); + } +} global; + +int +main (int argc, const char **argv) +{ + /* We don't want this program finishing and causing spurious "inferior + exited" notifications in GDB, so keep sleeping here. */ + sleep (60); + return 0; +} diff --git a/gdb/testsuite/gdb.multi/start-inferior-specific.cc b/gdb/testsuite/gdb.multi/start-inferior-specific.cc new file mode 100644 index 00000000000..432a6aa8ab8 --- /dev/null +++ b/gdb/testsuite/gdb.multi/start-inferior-specific.cc @@ -0,0 +1,32 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <unistd.h> + +struct global +{ + global () + { + sleep (3); + } +} global; + +int +main () +{ + return 0; +} diff --git a/gdb/testsuite/gdb.multi/start-inferior-specific.exp b/gdb/testsuite/gdb.multi/start-inferior-specific.exp new file mode 100644 index 00000000000..94d1a956b37 --- /dev/null +++ b/gdb/testsuite/gdb.multi/start-inferior-specific.exp @@ -0,0 +1,61 @@ +# Copyright 2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Test that "start"ing an inferior does not inadvertently stop in another +# inferior. +# +# To achieve this, we start an inferior (the "other") in background, which +# sleeps for 3 seconds before reaching its main function. We then "start" a +# second inferior, which also sleeps for 3 seconds before reaching its main +# function. A buggy GDB would report a breakpoint hit in "other". + +standard_testfile .cc -other.cc + +if { [use_gdb_stub] } { + return +} + +set srcfile_other ${srcfile2} +set binfile_other ${binfile}-other + +if { [build_executable ${testfile}.exp ${binfile} "${srcfile}" \ + {debug c++}] != 0 } { + return -1 +} + +if { [build_executable ${testfile}.exp ${binfile_other} "${srcfile_other}" \ + {debug c++}] != 0 } { + return -1 +} + +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" + gdb_test "add-inferior" "Added inferior 2.*" + gdb_test "inferior 2" "Switching to inferior 2.*" + gdb_file_cmd ${::binfile} + gdb_test "start" "Thread 2.1 .* hit Temporary breakpoint .*" +} + +do_test