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 927BC3858D37 for ; Wed, 17 Aug 2022 17:56:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 927BC3858D37 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 25BAC1E13B; Wed, 17 Aug 2022 13:56:32 -0400 (EDT) Message-ID: <64e8ab57-f463-8b5d-9934-28ee29f91dea@simark.ca> Date: Wed, 17 Aug 2022 13:56:31 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH] gdb: make "start" breakpoint inferior-specific Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org References: <20220804174035.2441960-1-simon.marchi@efficios.com> From: Simon Marchi In-Reply-To: <20220804174035.2441960-1-simon.marchi@efficios.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, SPF_HELO_PASS, 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2022 17:56:34 -0000 Ping. On 2022-08-04 13:40, Simon Marchi via Gdb-patches wrote: > 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. > > Add a test that does: > > - start a background inferior 1 that calls main in a loop > - start aninferior 2 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. > > Change-Id: Ib0148498a476bfa634ed62353c95f163623c686a > --- > gdb/infcmd.c | 3 +- > .../gdb.multi/start-inferior-specific-other.c | 30 ++++++++++ > .../gdb.multi/start-inferior-specific.c | 22 +++++++ > .../gdb.multi/start-inferior-specific.exp | 58 +++++++++++++++++++ > 4 files changed, 112 insertions(+), 1 deletion(-) > create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific-other.c > create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific.c > create mode 100644 gdb/testsuite/gdb.multi/start-inferior-specific.exp > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 69a7896b560..e9107cef520 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.c b/gdb/testsuite/gdb.multi/start-inferior-specific-other.c > new file mode 100644 > index 00000000000..b593c3aa068 > --- /dev/null > +++ b/gdb/testsuite/gdb.multi/start-inferior-specific-other.c > @@ -0,0 +1,30 @@ > +/* 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 . */ > + > +#include > + > +int > +main (int argc, const char **argv) > +{ > + if (argc == 999) > + return 0; > + > + for (;;) > + main (999, NULL); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.multi/start-inferior-specific.c b/gdb/testsuite/gdb.multi/start-inferior-specific.c > new file mode 100644 > index 00000000000..b69e218962a > --- /dev/null > +++ b/gdb/testsuite/gdb.multi/start-inferior-specific.c > @@ -0,0 +1,22 @@ > +/* 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 . */ > + > +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..5a5572f591a > --- /dev/null > +++ b/gdb/testsuite/gdb.multi/start-inferior-specific.exp > @@ -0,0 +1,58 @@ > +# 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 . > + > +# Test that "start"ing an inferior does not inadvertently stop in another > +# inferior. > +# > +# To achieve this, we have an inferior (the "other") running in background > +# constantly re-calling main. We then "start" a second inferior. A buggy GDB > +# would report a breakpoint hit in "other". > + > +standard_testfile .c -other.c > + > +if { [use_gdb_stub] } { > + return > +} > + > +set srcfile_other ${srcfile2} > +set binfile_other ${binfile}-other > + > +if { [build_executable ${testfile}.exp ${binfile} "${srcfile}" {debug}] != 0 } { > + return -1 > +} > + > +if { [build_executable ${testfile}.exp ${binfile_other} "${srcfile_other}" {debug}] != 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