public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
From: gdb-buildbot@sergiodj.net
To: gdb-testers@sourceware.org
Subject: [binutils-gdb] Don't write to inferior_ptid in windows-nat.c, part II
Date: Fri, 19 Jun 2020 15:47:58 -0400	[thread overview]
Message-ID: <50838d1be72ddd30e0b5f081933482424ae5a6b0@gdb-build> (raw)

*** TEST RESULTS FOR COMMIT 50838d1be72ddd30e0b5f081933482424ae5a6b0 ***

commit 50838d1be72ddd30e0b5f081933482424ae5a6b0
Author:     Pedro Alves <palves@redhat.com>
AuthorDate: Thu Jun 18 21:28:35 2020 +0100
Commit:     Pedro Alves <palves@redhat.com>
CommitDate: Thu Jun 18 23:17:01 2020 +0100

    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  <palves@redhat.com>
    
            * 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.

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  <palves@redhat.com>
+
+	* 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  <palves@redhat.com>
 
 	* 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_stop> 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<inf_child_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<inf_child_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 (&current_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); */
 }


             reply	other threads:[~2020-06-19 19:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 19:47 gdb-buildbot [this message]
2020-06-19 19:47 ` Failures on Ubuntu-Aarch64-native-extended-gdbserver-m64, branch master gdb-buildbot
2020-07-20 10:23 ` Failures on Fedora-i686, " gdb-buildbot
2020-07-20 10:52 ` Failures on Fedora-x86_64-cc-with-index, " gdb-buildbot
2020-07-20 10:58 ` Failures on Fedora-x86_64-m32, " gdb-buildbot
2020-07-20 11:26 ` Failures on Fedora-x86_64-m64, " gdb-buildbot
2020-07-20 11:41 ` Failures on Fedora-x86_64-native-extended-gdbserver-m32, " gdb-buildbot
2020-07-20 12:13 ` Failures on Fedora-x86_64-native-extended-gdbserver-m64, " gdb-buildbot
2020-07-20 12:17 ` Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot
2020-07-20 12:52 ` Failures on Fedora-x86_64-native-gdbserver-m64, " gdb-buildbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50838d1be72ddd30e0b5f081933482424ae5a6b0@gdb-build \
    --to=gdb-buildbot@sergiodj.net \
    --cc=gdb-testers@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).