[binutils-gdb] gdb: make "start" breakpoint inferior-specific

[binutils-gdb] gdb: make "start" breakpoint inferior-specific

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0be837be9fb4fc1f882a52a6fb7ad27e2f3023ae

commit 0be837be9fb4fc1f882a52a6fb7ad27e2f3023ae
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Thu Aug 4 11:49:12 2022 -0400

    gdb: make "start" breakpoint inferior-specific
    
    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 before reaching its main
       function (using a sleep in a global C++ object's constructor)
     - start inferior 2 with the "start" command, which also sleeps before
       reaching its main function
     - 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.
    
    Reviewed-By: Bruno Larsen <blarsen@redhat.com>
    Approved-By: Pedro Alves <pedro@palves.net>
    Change-Id: Ib0148498a476bfa634ed62353c95f163623c686a

Diff:
---
 gdb/infcmd.c                                       |  8 ++-
 .../gdb.multi/start-inferior-specific-other.c      | 34 ++++++++++++
 gdb/testsuite/gdb.multi/start-inferior-specific.c  | 31 +++++++++++
 .../gdb.multi/start-inferior-specific.exp          | 61 ++++++++++++++++++++++
 4 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c03ca103c91..bf4a68e3557 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -423,7 +423,13 @@ 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 ());
+      /* 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.  */
+      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..17a06a54443
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/start-inferior-specific-other.c
@@ -0,0 +1,34 @@
+/* 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>
+
+__attribute__((constructor))
+static void
+ctor (void)
+{
+  sleep (2);
+}
+
+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.c b/gdb/testsuite/gdb.multi/start-inferior-specific.c
new file mode 100644
index 00000000000..930010cbca6
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/start-inferior-specific.c
@@ -0,0 +1,31 @@
+/* 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>
+
+__attribute__((constructor))
+static void
+ctor (void)
+{
+  sleep (4);
+}
+
+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..45e8e8e2cc5
--- /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 inferior 1 in background, which sleeps for a bit
+# before reaching its main function.  We then "start" inferior 2, which also
+# sleeps before reaching its main function.  The goal is that inferior 1
+# "crosses" inferior 2's start breakpoint (at the time of writing this test, the
+# breakpoint inserted for start is global and has locations in both inferiors).
+# A buggy GDB would report a breakpoint hit in inferior 1.
+
+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 -no-prompt-anchor "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