aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Marchi <simon.marchi@polymtl.ca>2024-12-04 15:07:32 -0500
committerSimon Marchi <simon.marchi@polymtl.ca>2024-12-07 15:48:50 -0500
commit7893943879cedcc6520d0849fa3d10375dda0333 (patch)
tree55636d84d95e4bdc4a388ea2a347aa55cf0c25f5
parent0623a1a5365bbb373a802dc12208a3a98943ce9b (diff)
downloadbinutils-7893943879cedcc6520d0849fa3d10375dda0333.zip
binutils-7893943879cedcc6520d0849fa3d10375dda0333.tar.gz
binutils-7893943879cedcc6520d0849fa3d10375dda0333.tar.bz2
gdbserver: simplify win32 process removal
In the spirit of encapsulation, I'm looking to remove the need for external code to access the "ptid -> thread" map of process_info, making it an internal implementation detail. The only remaining use is in function clear_inferiors, and it led me down this rabbit hole: - clear_inferiors is really only used by the Windows port and doesn't really make sense in the grand scheme of things, I think (when would you want to remove all threads of all processes, without removing those processes?) - ok, so let's remove clear_inferiors and inline the code where it's called, in function win32_clear_inferiors - the Windows port does not support multi-process, so it's not really necessary to loop over all processes like this: for_each_process ([] (process_info *process) { process->thread_list ().clear (); process->thread_map ().clear (); }); We could just do: current_process ()->thread_list ().clear (); current_process ()->thread_map ().clear (); (or pass down the process from the caller, but it's not important right now) - so, the code that we've inlined in win32_clear_inferiors does 3 things: - clear the process' thread list and map (which deletes the thread_info objects) - clear the dll list, which just basically frees some objects - switch to no current process / no current thread - let's now look at where this win32_clear_inferiors function is used: - in win32_process_target::kill, where the process is removed just after - in win32_process_target::detach, where the process is removed just after - in win32_process_target::wait, when handling a process exit. After this returns, we could be in handle_target_event (if async) or resume (if sync), both in `server.cc`. In both of these cases, target_mourn_inferior gets called, we end up in win32_process_target::mourn, which removes the process - in all 3 cases above, we end up removing the process, which takes care of the 3 actions listed above: - the thread list and map get cleared when the process gets destroyed - same with the dll list - remove_process switches to no current process / current thread if the process being removed is the current one - I conclude that it's probably unnecessary to do the cleanup in win32_clear_inferiors, because it's going to get done right after anyway. Therefore, this patch does: - remove clear_inferiors, remove the call in win32_clear_inferiors - remove clear_dlls, which is now unused - remove process_info::thread_map, which is now unused - rename win32_clear_inferiors to win32_clear_process, which seems more accurate win32_clear_inferiors also does: for_each_thread (delete_thread_info); which also makes sure to delete all threads, but it also deletes the Windows private data object (windows_thread_info), so I'll leave this one there for now. But if we could make the thread private data destruction automatic, on thread destruction, it could be removed, I think. There should be no user-visible change with this patch. Of course, operations don't happen in the same order as before, so there might be some important detail I'm missing. I'm only able to build-test this, if someone could give it a test run on Windows, it would be appreciated. Change-Id: I4a560affe763a2c965a97754cc02f3083dbe6fbf
-rw-r--r--gdbserver/dll.cc9
-rw-r--r--gdbserver/dll.h1
-rw-r--r--gdbserver/inferiors.cc15
-rw-r--r--gdbserver/inferiors.h6
-rw-r--r--gdbserver/win32-low.cc11
5 files changed, 5 insertions, 37 deletions
diff --git a/gdbserver/dll.cc b/gdbserver/dll.cc
index ff637a0..f49aa56 100644
--- a/gdbserver/dll.cc
+++ b/gdbserver/dll.cc
@@ -89,12 +89,3 @@ unloaded_dll (process_info *proc, const char *name, CORE_ADDR base_addr)
proc->dlls_changed = true;
}
}
-
-void
-clear_dlls (void)
-{
- for_each_process ([] (process_info *proc)
- {
- proc->all_dlls.clear ();
- });
-}
diff --git a/gdbserver/dll.h b/gdbserver/dll.h
index 30c6a2c..e603df4 100644
--- a/gdbserver/dll.h
+++ b/gdbserver/dll.h
@@ -32,7 +32,6 @@ struct dll_info
CORE_ADDR base_addr;
};
-extern void clear_dlls (void);
extern void loaded_dll (const char *name, CORE_ADDR base_addr);
extern void loaded_dll (process_info *proc, const char *name,
CORE_ADDR base_addr);
diff --git a/gdbserver/inferiors.cc b/gdbserver/inferiors.cc
index 7a0209b..7bb322b 100644
--- a/gdbserver/inferiors.cc
+++ b/gdbserver/inferiors.cc
@@ -112,21 +112,6 @@ process_info::remove_thread (thread_info *thread)
m_thread_list.erase (m_thread_list.iterator_to (*thread));
}
-void
-clear_inferiors (void)
-{
- for_each_process ([] (process_info *process)
- {
- process->thread_list ().clear ();
- process->thread_map ().clear ();
- });
-
- clear_dlls ();
-
- switch_to_thread (nullptr);
- current_process_ = nullptr;
-}
-
struct process_info *
add_process (int pid, int attached)
{
diff --git a/gdbserver/inferiors.h b/gdbserver/inferiors.h
index 2f3d42b..890e9b9 100644
--- a/gdbserver/inferiors.h
+++ b/gdbserver/inferiors.h
@@ -92,10 +92,6 @@ struct process_info : public intrusive_list_node<process_info>
owning_intrusive_list<thread_info> &thread_list ()
{ return m_thread_list; }
- /* Return a reference to the private thread map. */
- std::unordered_map<ptid_t, thread_info *> &thread_map ()
- { return m_ptid_thread_map; }
-
/* Return the number of threads in this process. */
unsigned int thread_count () const
{ return m_ptid_thread_map.size (); }
@@ -159,8 +155,6 @@ int have_attached_inferiors_p (void);
/* Switch to a thread of PROC. */
void switch_to_process (process_info *proc);
-void clear_inferiors (void);
-
/* Set the inferior current working directory. If CWD is empty, unset
the directory. */
void set_inferior_cwd (std::string cwd);
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index dab57f7..49f0ed3 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -660,7 +660,7 @@ gdbserver_windows_process::handle_output_debug_string
}
static void
-win32_clear_inferiors (void)
+win32_clear_process ()
{
if (windows_process.open_process_used)
{
@@ -670,7 +670,6 @@ win32_clear_inferiors (void)
for_each_thread (delete_thread_info);
windows_process.siginfo_er.ExceptionCode = 0;
- clear_inferiors ();
}
/* Implementation of target_ops::kill. */
@@ -693,9 +692,9 @@ win32_process_target::kill (process_info *process)
windows_process.handle_output_debug_string (nullptr);
}
- win32_clear_inferiors ();
-
+ win32_clear_process ();
remove_process (process);
+
return 0;
}
@@ -714,7 +713,7 @@ win32_process_target::detach (process_info *process)
return -1;
DebugSetProcessKillOnExit (FALSE);
- win32_clear_inferiors ();
+ win32_clear_process ();
remove_process (process);
return 0;
@@ -1197,7 +1196,7 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
case TARGET_WAITKIND_EXITED:
OUTMSG2 (("Child exited with retcode = %x\n",
ourstatus->exit_status ()));
- win32_clear_inferiors ();
+ win32_clear_process ();
return ptid_t (windows_process.current_event.dwProcessId);
case TARGET_WAITKIND_STOPPED:
case TARGET_WAITKIND_SIGNALLED: