gdb/mi: preserve user selected thread and frame when invoking MI commands
authorJan Vrany <jan.vrany@labware.com>
Wed, 2 Mar 2022 13:23:30 +0000 (13:23 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Tue, 8 Mar 2022 16:55:37 +0000 (16:55 +0000)
Fix for PR gdb/20684.  When invoking MI commands with --thread and/or
--frame, the user selected thread and frame was not preserved:

  (gdb)
  info thread
  &"info thread\n"
  ~"  Id   Target Id                                           Frame \n"
  ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n"
  ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
  ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
  ^done
  (gdb)
  info frame
  &"info frame\n"
  ~"Stack level 0, frame at 0x7fffffffdf90:\n"
  ~" rip = 0x555555555207 in main (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60); saved rip = 0x7ffff7c5709b\n"
  ~" source language c.\n"
  ~" Arglist at 0x7fffffffdf80, args: \n"
  ~" Locals at 0x7fffffffdf80, Previous frame's sp is 0x7fffffffdf90\n"
  ~" Saved registers:\n "
  ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
  ^done
  (gdb)
  -stack-info-depth --thread 3
  ^done,depth="4"
  (gdb)
  info thread
  &"info thread\n"
  ~"  Id   Target Id                                           Frame \n"
  ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n"
  ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
  ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
  ^done
  (gdb)
  info frame
  &"info frame\n"
  ~"Stack level 0, frame at 0x7ffff742dee0:\n"
  ~" rip = 0x555555555169 in child_sub_function (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30); saved rip = 0x555555555188\n"
  ~" called by frame at 0x7ffff742df00\n"
  ~" source language c.\n"
  ~" Arglist at 0x7ffff742ded0, args: \n"
  ~" Locals at 0x7ffff742ded0, Previous frame's sp is 0x7ffff742dee0\n"
  ~" Saved registers:\n "
  ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
  ^done
  (gdb)

This caused problems for frontends that provide access to CLI because UI
may silently change the context for CLI commands (as demonstrated above).

This commit fixes the problem by restoring thread and frame in
mi_cmd_execute (). With this change, there are only two GDB/MI commands
that can change user selected context: -thread-select and -stack-select-frame.
This allows us to remove all and rather complicated logic of notifying
about user selected context change from mi_execute_command (), leaving it
to these two commands themselves to notify.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20684

gdb/mi/mi-cmd-stack.c
gdb/mi/mi-cmds.h
gdb/mi/mi-main.c
gdb/testsuite/gdb.mi/mi-cmd-user-context.exp [new file with mode: 0644]

index e63f1706149754603d3eb6fbead0abbe07686235..1be8aa81c3d109efb3fce9a013c6f508dcb197a3 100644 (file)
@@ -36,6 +36,8 @@
 #include "mi-parse.h"
 #include "gdbsupport/gdb_optional.h"
 #include "safe-ctype.h"
+#include "inferior.h"
+#include "observable.h"
 
 enum what_to_list { locals, arguments, all };
 
@@ -756,7 +758,16 @@ mi_cmd_stack_select_frame (const char *command, char **argv, int argc)
   if (argc == 0 || argc > 1)
     error (_("-stack-select-frame: Usage: FRAME_SPEC"));
 
+  ptid_t previous_ptid = inferior_ptid;
+
   select_frame_for_mi (parse_frame_specification (argv[0]));
+
+  /* Notify if the thread has effectively changed.  */
+  if (inferior_ptid != previous_ptid)
+    {
+      gdb::observers::user_selected_context_changed.notify
+       (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
+    }
 }
 
 void
index 2a93a9f5476da9aaf53d8de6001198dbd022061b..785652ee1c94e422acc9ba4257985f414dddd386 100644 (file)
@@ -23,6 +23,7 @@
 #define MI_MI_CMDS_H
 
 #include "gdbsupport/gdb_optional.h"
+#include "mi/mi-main.h"
 
 enum print_values {
    PRINT_NO_VALUES,
@@ -163,6 +164,17 @@ struct mi_command
      wrong.  */
   void invoke (struct mi_parse *parse) const;
 
+  /* Return whether this command preserves user selected context (thread
+     and frame).  */
+  bool preserve_user_selected_context () const
+  {
+    /* Here we exploit the fact that if MI command is supposed to change
+       user context, then it should not emit change notifications.  Therefore if
+       command does not suppress user context change notifications, then it should
+       preserve the context.  */
+    return m_suppress_notification != &mi_suppress_notification.user_selected_context;
+  }
+
 protected:
 
   /* The core of command invocation, this needs to be overridden in each
index b3592964e3db2f105ee5cd33b83c8ee3ada5c186..73380f5e668d001ac25397e27e52e3f84220452c 100644 (file)
@@ -1918,34 +1918,6 @@ mi_print_exception (const char *token, const struct gdb_exception &exception)
   fputs_unfiltered ("\n", mi->raw_stdout);
 }
 
-/* Determine whether the parsed command already notifies the
-   user_selected_context_changed observer.  */
-
-static int
-command_notifies_uscc_observer (struct mi_parse *command)
-{
-  if (command->op == CLI_COMMAND)
-    {
-      /* CLI commands "thread" and "inferior" already send it.  */
-      return (startswith (command->command, "thread ")
-             || startswith (command->command, "inferior "));
-    }
-  else /* MI_COMMAND */
-    {
-      if (strcmp (command->command, "interpreter-exec") == 0
-         && command->argc > 1)
-       {
-         /* "thread" and "inferior" again, but through -interpreter-exec.  */
-         return (startswith (command->argv[1], "thread ")
-                 || startswith (command->argv[1], "inferior "));
-       }
-
-      else
-       /* -thread-select already sends it.  */
-       return strcmp (command->command, "thread-select") == 0;
-    }
-}
-
 void
 mi_execute_command (const char *cmd, int from_tty)
 {
@@ -1971,8 +1943,6 @@ mi_execute_command (const char *cmd, int from_tty)
 
   if (command != NULL)
     {
-      ptid_t previous_ptid = inferior_ptid;
-
       command->token = token;
 
       if (do_timings)
@@ -2002,37 +1972,6 @@ mi_execute_command (const char *cmd, int from_tty)
 
       bpstat_do_actions ();
 
-      if (/* The notifications are only output when the top-level
-            interpreter (specified on the command line) is MI.  */
-         top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()
-         /* Don't try report anything if there are no threads --
-            the program is dead.  */
-         && any_thread_p ()
-         /* If the command already reports the thread change, no need to do it
-            again.  */
-         && !command_notifies_uscc_observer (command.get ()))
-       {
-         int report_change = 0;
-
-         if (command->thread == -1)
-           {
-             report_change = (previous_ptid != null_ptid
-                              && inferior_ptid != previous_ptid
-                              && inferior_ptid != null_ptid);
-           }
-         else if (inferior_ptid != null_ptid)
-           {
-             struct thread_info *ti = inferior_thread ();
-
-             report_change = (ti->global_num != command->thread);
-           }
-
-         if (report_change)
-           {
-             gdb::observers::user_selected_context_changed.notify
-               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
-           }
-       }
     }
 }
 
@@ -2076,6 +2015,7 @@ mi_cmd_execute (struct mi_parse *parse)
       set_current_program_space (inf->pspace);
     }
 
+  gdb::optional<scoped_restore_current_thread> thread_saver;
   if (parse->thread != -1)
     {
       thread_info *tp = find_thread_global_id (parse->thread);
@@ -2086,9 +2026,13 @@ mi_cmd_execute (struct mi_parse *parse)
       if (tp->state == THREAD_EXITED)
        error (_("Thread id: %d has terminated"), parse->thread);
 
+      if (parse->cmd->preserve_user_selected_context ())
+       thread_saver.emplace ();
+
       switch_to_thread (tp);
     }
 
+  gdb::optional<scoped_restore_selected_frame> frame_saver;
   if (parse->frame != -1)
     {
       struct frame_info *fid;
@@ -2096,8 +2040,12 @@ mi_cmd_execute (struct mi_parse *parse)
 
       fid = find_relative_frame (get_current_frame (), &frame);
       if (frame == 0)
-       /* find_relative_frame was successful */
-       select_frame (fid);
+       {
+         if (parse->cmd->preserve_user_selected_context ())
+           frame_saver.emplace ();
+
+         select_frame (fid);
+       }
       else
        error (_("Invalid frame id: %d"), frame);
     }
diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
new file mode 100644 (file)
index 0000000..d7417d5
--- /dev/null
@@ -0,0 +1,157 @@
+# 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 GDB/MI commands preserve user selected context when
+# passed --thread and/or --frame.
+
+load_lib mi-support.exp
+
+standard_testfile user-selected-context-sync.c
+
+if {[build_executable $testfile.exp $testfile ${srcfile} "debug pthreads"] == -1} {
+    untested "failed to compile"
+    return -1
+}
+
+set main_break_line [gdb_get_line_number "main break line"]
+
+set any "\[^\r\n\]*"
+
+mi_clean_restart $binfile
+mi_create_breakpoint "$srcfile:$main_break_line" "set breakpoint in main"
+mi_run_cmd
+mi_expect_stop "breakpoint-hit" "main" "" $srcfile $main_break_line \
+        { "" "disp=\"keep\"" } "run to breakpoint in main"
+
+mi_gdb_test "thread" \
+       ".*Current thread is 1.*" \
+       "info thread 1"
+
+# Run -stack-info-depth in a different thread, the current thread
+# should not change.
+
+mi_gdb_test "-stack-info-depth --thread 3" \
+    "\\^done,depth=.*" \
+    "-stack-info-depth --thread 3"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 1.*" \
+    "info thread 2"
+
+# Expect GDB to switch to thread 3.
+mi_gdb_test "-thread-select 3" \
+    "\\^done,${any}new-thread-id=\"3\"${any}" \
+    "-thread-select 3"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 3.*" \
+    "info thread 3"
+
+# Expect GDB to switch to thread 1.
+mi_gdb_test "-thread-select --thread 2 1" \
+    "\\^done,${any}new-thread-id=\"1\"${any}" \
+    "-thread-select --thread 2 1"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 1.*" \
+    "info thread 4"
+
+# Expect GDB to switch to thread 2.
+mi_gdb_test "-thread-select --thread 2 2" \
+    "\\^done,.*" \
+    "-thread-select --thread 2 2"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 2.*" \
+    "info thread 5"
+
+# Check we're in frame 0.
+mi_gdb_test "frame" \
+    ".*#0  0x.*" \
+    "frame 1"
+
+# Ask about a different frame in the current thread, the current frame
+# should not change.
+mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \
+    "\\^done,frame=\{level=\"1\".*" \
+    "-stack-info-frame 1"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 2.*" \
+    "info thread 6"
+
+mi_gdb_test "frame" \
+    ".*#0  0x.*" \
+    "frame 2"
+
+
+# Ask about a different frame in a different thread.  After this the
+# current thread and frame should not have changed.
+mi_gdb_test "-stack-info-frame --thread 3 --frame 1" \
+    "\\^done,frame=\{level=\"1\".*" \
+    "-stack-info-frame 2"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 2.*" \
+    "info thread 7"
+
+mi_gdb_test "frame" \
+    ".*#0  0x.*" \
+    "frame 3"
+
+# Select a different frame in the current thread.  Despite the use of
+# the --frame option, we do expect the currently selected frame to
+# change.
+mi_gdb_test "-stack-select-frame --thread 2 --frame 0 1" \
+    "\\^done" \
+    "--stack-select-frame 1"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 2.*" \
+    "info thread 8"
+
+mi_gdb_test "frame" \
+    ".*#1  0x.*" \
+    "frame 4"
+
+# Similar to the previous test, but this time the --frame option is
+# the same as the frame we are selecting.  We still expect GDB to
+# update the currently selected frame.
+mi_gdb_test "-stack-select-frame --thread 2 --frame 2 2" \
+    "\\^done" \
+    "--stack-select-frame 2"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 2.*" \
+    "info thread 9"
+
+mi_gdb_test "frame" \
+    ".*#2  0x.*" \
+    "frame 5"
+
+# Now select a frame in a different thread.  We expect both the
+# currently selected thread, and the currently selected frame to be
+# updated.
+mi_gdb_test "-stack-select-frame --thread 1 --frame 0 0" \
+    "\\^done" \
+    "--stack-select-frame 3"
+
+mi_gdb_test "thread" \
+    ".*Current thread is 1.*" \
+    "info thread 10"
+
+mi_gdb_test "frame" \
+    ".*#0  main.*" \
+    "frame 6"