From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 0C3D63858D39 for ; Wed, 31 Aug 2022 14:03:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C3D63858D39 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-36-GnTzH4d1PjCRHy5eBlqwTg-1; Wed, 31 Aug 2022 10:03:07 -0400 X-MC-Unique: GnTzH4d1PjCRHy5eBlqwTg-1 Received: by mail-wm1-f72.google.com with SMTP id i132-20020a1c3b8a000000b003a537064611so8333778wma.4 for ; Wed, 31 Aug 2022 07:03:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=/rNB2bgif3NWsJTvQnwOrtK8ojOa7rcNtyNGxQCaMnY=; b=UD/kv4aOgekkJoYjII9HESHwg/51CsWO4/gAZ7/vv8lmV0gD2YGI5m6AeHWilY0waD 5V0k3QOl/6F389ewoO5T3FvfJ+h1McuxLUYqXQVfiFDD5S/sb6CmhZezOdQtJ3hp6XDW XKD/9v2gsxwP4GUIA/SpfFTroclWm7saYCHJkS9fY/sSmsTbbKo/dmTuH5S/24JfJ+Qf 122YoqiIrVyyUZ++bHUQyieYwdD+Gjfa/GqEKzTORRvTgD86xWsYL4O+bTSYz6V3KVim razeQ+qeHlg5XSppXBdMfLPoSBdIA5RFWOG7xOS5R+IX+UbxybfW5IrSnc1zaDMUHHcl tOAA== X-Gm-Message-State: ACgBeo08UJRlrCxRi4Aqw0wYlZsndOcd7CKs6oTQCsb+UiigMpmotcDP DcTlA68HijdQ3q8CcaBXIyamcJsIcyZUeokp+d0PLkLgyTn7Vm/rvVr1x8vZ0SKgpYq6vdFDvHy SvrNecKxON3otERjpdrRhfJ5tDRwxsDHhJdrnVQTk1rp+JoaxZKsGjYpS0axEOHMB03cUyqSu X-Received: by 2002:a5d:62c7:0:b0:225:8657:df17 with SMTP id o7-20020a5d62c7000000b002258657df17mr11613147wrv.346.1661954583866; Wed, 31 Aug 2022 07:03:03 -0700 (PDT) X-Google-Smtp-Source: AA6agR521bBWHXDFrEHrxf9KoGCzfIJMep6MhmM656EaRm3UDHUr5pcDK/SpGAwzknzj+M6FeW1I6A== X-Received: by 2002:a5d:62c7:0:b0:225:8657:df17 with SMTP id o7-20020a5d62c7000000b002258657df17mr11613125wrv.346.1661954583429; Wed, 31 Aug 2022 07:03:03 -0700 (PDT) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id x14-20020adfffce000000b00224f7c1328dsm12057033wrs.67.2022.08.31.07.03.02 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 31 Aug 2022 07:03:03 -0700 (PDT) Message-ID: Date: Wed, 31 Aug 2022 16:03:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.0 Subject: Re: [PATCH] gdb: make "start" breakpoint inferior-specific To: gdb-patches@sourceware.org References: <20220804174035.2441960-1-simon.marchi@efficios.com> From: Bruno Larsen In-Reply-To: <20220804174035.2441960-1-simon.marchi@efficios.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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, 31 Aug 2022 14:03:12 -0000 On 04/08/2022 19: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 Hi Simon, This change makes a lot of sense, I just wonder if it would make sense to make breakpoints inferior-specific. I could see a situation where someone would run the same program twice, one with an ok input while the other has a buggy input for instance, to see the difference between them, and inferior-specific breakpoints could come in handy in a situation like this. If you think it would be too much work for a usecase that is too niche, this patch is pretty straight forward, I'd suggest you approve your patch. Cheers, Bruno > --- > 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