From 50838d1be72ddd30e0b5f081933482424ae5a6b0 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 18 Jun 2020 21:28:35 +0100 Subject: [PATCH] Don't write to inferior_ptid in windows-nat.c, part II Writing to inferior_ptid in windows_nat_target::get_windows_debug_event is just incorrect and not necessary. We'll report the event to GDB's core, which then takes care of switching inferior_ptid / current thread. Related (see windows_nat_target::get_windows_debug_event), there's also a "current_windows_thread" global that is just begging to get out of sync with core GDB's current thread. This patch removes it. gdbserver already does not have an equivalent global in win32-low.cc. gdb/ChangeLog: 2020-06-18 Pedro Alves * nat/windows-nat.c (current_windows_thread): Remove. * nat/windows-nat.h (current_windows_thread): Remove. * windows-nat.c (windows_nat_target::stopped_by_sw_breakpoint): Adjust. (display_selectors): Adjust to fetch the current windows_thread_info based on inferior_ptid. (fake_create_process): No longer write to current_windows_thread. (windows_nat_target::get_windows_debug_event): Don't set inferior_ptid or current_windows_thread. (windows_nat_target::wait): Adjust to not rely on current_windows_thread. (do_initial_windows_stuff): Now a method of windows_nat_target. Switch to the last_ptid thread. (windows_nat_target::attach): Adjust. (windows_nat_target::detach): Use switch_to_no_thread instead of writing to inferior_ptid directly. (windows_nat_target::create_inferior): Adjust. --- gdb/ChangeLog | 20 +++++++++ gdb/nat/windows-nat.c | 1 - gdb/nat/windows-nat.h | 3 -- gdb/windows-nat.c | 97 ++++++++++++++++++++++--------------------- 4 files changed, 70 insertions(+), 51 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 18da2d92b0..9d0659a3f8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,23 @@ +2020-06-18 Pedro Alves + + * nat/windows-nat.c (current_windows_thread): Remove. + * nat/windows-nat.h (current_windows_thread): Remove. + * windows-nat.c (windows_nat_target::stopped_by_sw_breakpoint): + Adjust. + (display_selectors): Adjust to fetch the current + windows_thread_info based on inferior_ptid. + (fake_create_process): No longer write to current_windows_thread. + (windows_nat_target::get_windows_debug_event): + Don't set inferior_ptid or current_windows_thread. + (windows_nat_target::wait): Adjust to not rely on + current_windows_thread. + (do_initial_windows_stuff): Now a method of windows_nat_target. + Switch to the last_ptid thread. + (windows_nat_target::attach): Adjust. + (windows_nat_target::detach): Use switch_to_no_thread instead of + writing to inferior_ptid directly. + (windows_nat_target::create_inferior): Adjust. + 2020-06-18 Pedro Alves * windows-nat.c (do_initial_windows_stuff): No longer set inferior_ptid. diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c index 709a9d3a31..be6db9719a 100644 --- a/gdb/nat/windows-nat.c +++ b/gdb/nat/windows-nat.c @@ -36,7 +36,6 @@ DEBUG_EVENT current_event; ContinueDebugEvent. */ static DEBUG_EVENT last_wait_event; -windows_thread_info *current_windows_thread; DWORD desired_stop_thread_id = -1; std::vector pending_stops; EXCEPTION_RECORD siginfo_er; diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h index 80c652b22a..f742db2acc 100644 --- a/gdb/nat/windows-nat.h +++ b/gdb/nat/windows-nat.h @@ -180,9 +180,6 @@ extern enum gdb_signal last_sig; stop. */ extern DEBUG_EVENT current_event; -/* Info on currently selected thread */ -extern windows_thread_info *current_windows_thread; - /* The ID of the thread for which we anticipate a stop event. Normally this is -1, meaning we'll accept an event in any thread. */ diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 19bc52bbbb..68df87d1bf 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -317,7 +317,9 @@ struct windows_nat_target final : public x86_nat_target bool stopped_by_sw_breakpoint () override { - return current_windows_thread->stopped_at_software_breakpoint; + windows_thread_info *th + = thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT); + return th->stopped_at_software_breakpoint; } bool supports_stopped_by_sw_breakpoint () override @@ -356,6 +358,8 @@ struct windows_nat_target final : public x86_nat_target const char *thread_name (struct thread_info *) override; int get_windows_debug_event (int pid, struct target_waitstatus *ourstatus); + + void do_initial_windows_stuff (DWORD pid, bool attaching); }; static windows_nat_target the_windows_nat_target; @@ -1131,11 +1135,15 @@ display_selector (HANDLE thread, DWORD sel) static void display_selectors (const char * args, int from_tty) { - if (!current_windows_thread) + if (inferior_ptid == null_ptid) { puts_filtered ("Impossible to display selectors now.\n"); return; } + + windows_thread_info *current_windows_thread + = thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT); + if (!args) { #ifdef __x86_64__ @@ -1367,12 +1375,11 @@ fake_create_process (void) (unsigned) GetLastError ()); /* We can not debug anything in that case. */ } - current_windows_thread - = windows_add_thread (ptid_t (current_event.dwProcessId, - current_event.dwThreadId, 0), - current_event.u.CreateThread.hThread, - current_event.u.CreateThread.lpThreadLocalBase, - true /* main_thread_p */); + windows_add_thread (ptid_t (current_event.dwProcessId, 0, + current_event.dwThreadId), + current_event.u.CreateThread.hThread, + current_event.u.CreateThread.lpThreadLocalBase, + true /* main_thread_p */); return current_event.dwThreadId; } @@ -1532,8 +1539,6 @@ windows_nat_target::get_windows_debug_event (int pid, { BOOL debug_event; DWORD continue_status, event_code; - windows_thread_info *th; - static windows_thread_info dummy_thread_info (0, 0, 0); DWORD thread_id = 0; /* If there is a relevant pending stop, report it now. See the @@ -1545,10 +1550,9 @@ windows_nat_target::get_windows_debug_event (int pid, thread_id = stop->thread_id; *ourstatus = stop->status; - inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0); - current_windows_thread = thread_rec (inferior_ptid, - INVALIDATE_CONTEXT); - current_windows_thread->reload_context = 1; + ptid_t ptid (current_event.dwProcessId, thread_id); + windows_thread_info *th = thread_rec (ptid, INVALIDATE_CONTEXT); + th->reload_context = 1; return thread_id; } @@ -1562,7 +1566,6 @@ windows_nat_target::get_windows_debug_event (int pid, event_code = current_event.dwDebugEventCode; ourstatus->kind = TARGET_WAITKIND_SPURIOUS; - th = NULL; have_saved_context = 0; switch (event_code) @@ -1588,7 +1591,7 @@ windows_nat_target::get_windows_debug_event (int pid, } /* Record the existence of this thread. */ thread_id = current_event.dwThreadId; - th = windows_add_thread + windows_add_thread (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0), current_event.u.CreateThread.hThread, current_event.u.CreateThread.lpThreadLocalBase, @@ -1605,7 +1608,6 @@ windows_nat_target::get_windows_debug_event (int pid, current_event.dwThreadId, 0), current_event.u.ExitThread.dwExitCode, false /* main_thread_p */); - th = &dummy_thread_info; break; case CREATE_PROCESS_DEBUG_EVENT: @@ -1619,7 +1621,7 @@ windows_nat_target::get_windows_debug_event (int pid, current_process_handle = current_event.u.CreateProcessInfo.hProcess; /* Add the main thread. */ - th = windows_add_thread + windows_add_thread (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0), current_event.u.CreateProcessInfo.hThread, @@ -1756,7 +1758,7 @@ windows_nat_target::get_windows_debug_event (int pid, && windows_initialization_done) { ptid_t ptid = ptid_t (current_event.dwProcessId, thread_id, 0); - th = thread_rec (ptid, INVALIDATE_CONTEXT); + windows_thread_info *th = thread_rec (ptid, INVALIDATE_CONTEXT); th->stopped_at_software_breakpoint = true; th->pc_adjusted = false; } @@ -1764,14 +1766,6 @@ windows_nat_target::get_windows_debug_event (int pid, thread_id = 0; CHECK (windows_continue (continue_status, desired_stop_thread_id, 0)); } - else - { - inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0); - current_windows_thread = th; - if (!current_windows_thread) - current_windows_thread = thread_rec (inferior_ptid, - INVALIDATE_CONTEXT); - } out: return thread_id; @@ -1828,19 +1822,24 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, { ptid_t result = ptid_t (current_event.dwProcessId, retval, 0); - if (current_windows_thread != nullptr) + if (ourstatus->kind != TARGET_WAITKIND_EXITED + && ourstatus->kind != TARGET_WAITKIND_SIGNALLED) { - current_windows_thread->stopped_at_software_breakpoint = false; - if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT - && ((current_event.u.Exception.ExceptionRecord.ExceptionCode - == EXCEPTION_BREAKPOINT) - || (current_event.u.Exception.ExceptionRecord.ExceptionCode - == STATUS_WX86_BREAKPOINT)) - && windows_initialization_done) + windows_thread_info *th = thread_rec (result, INVALIDATE_CONTEXT); + + if (th != nullptr) { - current_windows_thread->stopped_at_software_breakpoint - = true; - current_windows_thread->pc_adjusted = false; + th->stopped_at_software_breakpoint = false; + if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT + && ((current_event.u.Exception.ExceptionRecord.ExceptionCode + == EXCEPTION_BREAKPOINT) + || (current_event.u.Exception.ExceptionRecord.ExceptionCode + == STATUS_WX86_BREAKPOINT)) + && windows_initialization_done) + { + th->stopped_at_software_breakpoint = true; + th->pc_adjusted = false; + } } } @@ -1976,8 +1975,8 @@ windows_add_all_dlls (void) } } -static void -do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching) +void +windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching) { int i; struct inferior *inf; @@ -1993,8 +1992,8 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching) #endif current_event.dwProcessId = pid; memset (¤t_event, 0, sizeof (current_event)); - if (!target_is_pushed (ops)) - push_target (ops); + if (!target_is_pushed (this)) + push_target (this); disable_breakpoints_in_shlibs (); windows_clear_solib (); clear_proceed_status (0); @@ -2024,11 +2023,13 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching) windows_initialization_done = 0; + ptid_t last_ptid; + while (1) { struct target_waitstatus status; - ops->wait (minus_one_ptid, &status, 0); + last_ptid = this->wait (minus_one_ptid, &status, 0); /* Note windows_wait returns TARGET_WAITKIND_SPURIOUS for thread events. */ @@ -2036,9 +2037,11 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching) && status.kind != TARGET_WAITKIND_SPURIOUS) break; - ops->resume (minus_one_ptid, 0, GDB_SIGNAL_0); + this->resume (minus_one_ptid, 0, GDB_SIGNAL_0); } + switch_to_thread (find_thread_ptid (this, last_ptid)); + /* Now that the inferior has been started and all DLLs have been mapped, we can iterate over all DLLs and load them in. @@ -2170,7 +2173,7 @@ windows_nat_target::attach (const char *args, int from_tty) } #endif - do_initial_windows_stuff (this, pid, 1); + do_initial_windows_stuff (pid, 1); target_terminal::ours (); } @@ -2200,7 +2203,7 @@ windows_nat_target::detach (inferior *inf, int from_tty) } x86_cleanup_dregs (); - inferior_ptid = null_ptid; + switch_to_no_thread (); detach_inferior (inf); maybe_unpush_target (); @@ -3010,7 +3013,7 @@ windows_nat_target::create_inferior (const char *exec_file, else saw_create = 0; - do_initial_windows_stuff (this, pi.dwProcessId, 0); + do_initial_windows_stuff (pi.dwProcessId, 0); /* windows_continue (DBG_CONTINUE, -1, 0); */ } -- 2.34.1