From a9c82bc13cf8dd5d9076e746f744ee711eb55507 Mon Sep 17 00:00:00 2001 From: Jan Vrany Date: Wed, 2 Mar 2022 13:23:30 +0000 Subject: [PATCH] gdb/mi: preserve user selected thread and frame when invoking MI commands 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 | 11 ++ gdb/mi/mi-cmds.h | 12 ++ gdb/mi/mi-main.c | 74 ++------- gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 157 +++++++++++++++++++ 4 files changed, 191 insertions(+), 63 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c index e63f1706149..1be8aa81c3d 100644 --- a/gdb/mi/mi-cmd-stack.c +++ b/gdb/mi/mi-cmd-stack.c @@ -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 diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h index 2a93a9f5476..785652ee1c9 100644 --- a/gdb/mi/mi-cmds.h +++ b/gdb/mi/mi-cmds.h @@ -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 diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index b3592964e3d..73380f5e668 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -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 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 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 index 00000000000..d7417d5ea7c --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp @@ -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 . + +# 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" -- 2.30.2