Re: [PATCH] gdb: make "start" breakpoint inferior-specific

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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 [<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 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 <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +
> +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 <http://www.gnu.org/licenses/>.  */
> +
> +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 <http://www.gnu.org/licenses/>.
> +
> +# 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