gdb: disable commit-resumed on -exec-interrupt --thread-group

As reported in PR gdb/28077, we hit an internal error when using
-exec-interrupt with --thread-group:

    info threads
    &"info threads\n"
    ~"  Id   Target Id             Frame \n"
    ~"* 1    process 403312 \"loop\" (running)\n"
    ^done
    (gdb)
    -exec-interrupt --thread-group i1
    ~"/home/simark/src/binutils-gdb/gdb/target.c:3768: internal-error: void target_stop(ptid_t): Assertion `!proc_target->commit_resumed_state' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "

This is because this code path never disables commit-resumed (a
requirement for calling target_stop, as documented in
process_stratum_target::»commit_resumed_state) before calling
target_stop.

The other 3 code paths in mi_cmd_exec_interrupt use interrupt_target_1,
which does it.  But the --thread-group code path uses its own thing
which doesn't do it.  Fix this by adding a scoped_disable_commit_resumed
in this code path.

Calling -exec-interrupt with --thread-group is apparently not tested at
the moment (which is why this bug could creep in).  Add a new test for
that.  The test runs two inferiors and tries to interrupt them with
"-exec-interrupt --thread-group X".

This will need to be merged in the gdb-11-branch, so here are ChangeLog
entries:

gdb/ChangeLog:

	* mi/mi-main.c (mi_cmd_exec_interrupt): Use
	scoped_disable_commit_resumed in the --thread-group case.

gdb/testsuite/ChangeLog:

	* gdb.mi/interrupt-thread-group.c: New.
	* gdb.mi/interrupt-thread-group.exp: New.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28077
Change-Id: I615efefcbcaf2c15d47caf5e4b9d82854b2a2fcb

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 23ae68ac3a5c31b5a3fc07f5d3acc4efc1d9682e..2fe348a35695fc7fc53699a76f56bf7e56ca1cb4 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2021-07-13  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       * mi/mi-main.c (mi_cmd_exec_interrupt): Use
+       scoped_disable_commit_resumed in the --thread-group case.
+
 2021-07-13  John Ericson  <git@JohnEricson.me>
 
        * sparc-tdep.h: Fix comment.

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e293dddc08d86dabb372725b485f1464721da284..44008d1c0a869db70c9639c76d0b5c86afaf5540 100644 (file)
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -383,6 +383,9 @@ mi_cmd_exec_interrupt (const char *command, char **argv, int argc)
     {
       struct inferior *inf = find_inferior_id (current_context->thread_group);
 
+      scoped_disable_commit_resumed disable_commit_resumed
+       ("interrupting all threads of thread group");
+
       iterate_over_threads (interrupt_thread_callback, &inf->pid);
     }
   else

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3b8d123da28b7dc59fde677257bd50a0172f1987..b4fb5e8e20f823aca651e7fc0e0eae07b3e43bc4 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-07-13  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       * gdb.mi/interrupt-thread-group.c: New.
+       * gdb.mi/interrupt-thread-group.exp: New.
+
 2021-07-12  Tom de Vries  <tdevries@suse.de>
 
        PR testsuite/28057

diff --git a/gdb/testsuite/gdb.mi/interrupt-thread-group.c b/gdb/testsuite/gdb.mi/interrupt-thread-group.c
new file mode 100644 (file)
index 0000000..738c6fc
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/interrupt-thread-group.c
@@ -0,0 +1,65 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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>
+#include <pthread.h>
+#include <assert.h>
+
+#define NUM_THREADS 4
+
+static pthread_barrier_t barrier;
+
+static void *
+thread_function (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+
+  for (int i = 0; i < 30; i++)
+    sleep (1);
+
+  return NULL;
+}
+
+static void
+all_threads_started (void)
+{}
+
+int
+main (void)
+{
+  pthread_t threads[NUM_THREADS];
+
+  pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1);
+
+  for (int i = 0; i < NUM_THREADS; i++)
+    {
+      int res = pthread_create (&threads[i], NULL, thread_function, NULL);
+      assert (res == 0);
+    }
+
+  pthread_barrier_wait (&barrier);
+  all_threads_started ();
+
+  for (int i = 0; i < NUM_THREADS; i++)
+    {
+      int res = pthread_join (threads[i], NULL);
+      assert (res == 0);
+    }
+
+  return 0;
+}
+

diff --git a/gdb/testsuite/gdb.mi/interrupt-thread-group.exp b/gdb/testsuite/gdb.mi/interrupt-thread-group.exp
new file mode 100644 (file)
index 0000000..88da829
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/interrupt-thread-group.exp
@@ -0,0 +1,130 @@
+# Copyright 2021 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 --exec-interrupt with thread-group.
+#
+# Run two inferiors, try interrupting them both with --exec-interrupt +
+# --thread-group.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .c
+
+if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile \
+       executable debug] != "" } {
+    return -1
+}
+
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -ex \"set non-stop on\" -ex \"set mi-async\""
+    mi_clean_restart $binfile
+}
+
+mi_detect_async
+
+# Create breakpoint by hand instead of using mi_runto, since we'll need it for
+# both inferiors.
+mi_create_breakpoint "all_threads_started" \
+    "set breakpoint on all_threads_started"
+
+# Run first inferior to all_threads_started (to ensure all threads are started)
+# and resume it.
+if { [mi_run_cmd] < 0 } {
+    return
+}
+
+mi_expect_stop "breakpoint-hit" "all_threads_started" ".*" ".*" ".*" {"" "disp=\"keep\""} \
+    "inferior i1 stops at all_threads_started"
+
+mi_send_resuming_command "exec-continue --thread-group i1" \
+    "continue inferior 1"
+
+# We can't run a second inferior on stub targets.  We can still test with one
+# inferior and ensure that the command has the desired effect.
+set use_second_inferior [expr {![use_gdb_stub]}]
+
+if { $use_second_inferior } {
+    # The inferior created by the -add-inferior MI command does not inherit the
+    # target connection of the first inferior.  If debugging through an
+    # extended-remote connection, that means we can't run that second inferior
+    # on the remote connection.  Use the add-inferior CLI command as a stop-gap.
+    if { [mi_is_target_remote] } {
+       mi_gdb_test "add-inferior" \
+           "\\^done" \
+           "add inferior 2"
+    } else {
+       mi_gdb_test "-add-inferior" \
+           "\\^done,inferior=\"i2\"" \
+           "add inferior 2"
+    }
+    mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \
+       "\\^done" \
+       "set executable of inferior 2"
+    # Run second inferior to all_threads_started (to ensure all threads are
+    # started) and resume it.
+    mi_gdb_test "-exec-run --thread-group i2" \
+       "\\^running.*" \
+       "run inferior 2"
+
+    mi_expect_stop "breakpoint-hit" "all_threads_started" ".*" ".*" ".*" {"" "disp=\"keep\""} \
+       "inferior i2 stops at all_threads_started"
+
+    mi_send_resuming_command "exec-continue --thread-group i2" \
+       "continue inferior 2"
+
+    mi_check_thread_states {
+       "running" "running" "running" "running" "running"
+       "running" "running" "running" "running" "running"
+    } "before interrupting"
+} else {
+    mi_check_thread_states {
+       "running" "running" "running" "running" "running"
+    } "before interrupting"
+}
+
+# Interrupt inferior 1, wait for events.
+mi_gdb_test "-exec-interrupt --thread-group i1" \
+    "\\^done" \
+    "interrupt inferior 1"
+
+for {set i 0} {$i < 5} {incr i} {
+    mi_expect_interrupt "inferior 1, interrupt $i"
+}
+
+if { $use_second_inferior } {
+    mi_check_thread_states {
+       "stopped" "stopped" "stopped" "stopped" "stopped"
+       "running" "running" "running" "running" "running"
+    } "after interrupting inferior 1"
+
+    # Interrupt inferior 2, wait for events.
+    mi_gdb_test "-exec-interrupt --thread-group i2" \
+       "\\^done" \
+       "interrupt inferior 2"
+
+    for {set i 0} {$i < 5} {incr i} {
+       mi_expect_interrupt "inferior 2, interrupt $i"
+    }
+
+    mi_check_thread_states {
+       "stopped" "stopped" "stopped" "stopped" "stopped"
+       "stopped" "stopped" "stopped" "stopped" "stopped"
+    } "after interrupting inferior 2"
+} else {
+    mi_check_thread_states {
+       "stopped" "stopped" "stopped" "stopped" "stopped"
+    } "after interrupting inferior 1"
+}