[lldb] Fix data race in Process

[augusto2112]

Summary

Thread sanitizer reports a data race in Process.cpp in the usage of
m_process_input_reader. Fix this data race by introducing a mutex
guarding the access to this variable.

[JDevlieghere]

Is the reported race specifically about the shared pointer being accessed concurrently or operations on the IOHandler?

[augusto2112]

In D157648#4578420, @JDevlieghere wrote:

Is the reported race specifically about the shared pointer being accessed concurrently or operations on the IOHandler?

I believe it's the shared pointer being accessed. Here's an example of what's being reported:

WARNING: ThreadSanitizer: data race (pid=52249)
  Read of size 8 at 0x00010731ce38 by thread T3:
    #0 lldb_private::Process::PopProcessIOHandler() Process.cpp:4559 (liblldb.18.0.0git.dylib:arm64+0x53a5a4) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #1 lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr<lldb_private::Event>&) Process.cpp:3775 (liblldb.18.0.0git.dylib:arm64+0x54204c) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #2 lldb_private::Process::RunPrivateStateThread(bool) Process.cpp:3904 (liblldb.18.0.0git.dylib:arm64+0x54874c) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #3 std::__1::__function::__func<lldb_private::Process::StartPrivateStateThread(bool)::$_5, std::__1::allocator<lldb_private::Process::StartPrivateStateThread(bool)::$_5>, void* ()>::operator()() function.h:356 (liblldb.18.0.0git.dylib:arm64+0x555de4) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #4 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) HostNativeThreadBase.cpp:62 (liblldb.18.0.0git.dylib:arm64+0x3e83fc) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #5 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) HostThreadMacOSX.mm:18 (liblldb.18.0.0git.dylib:arm64+0x175056c) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)

  Previous write of size 8 at 0x00010731ce38 by main thread (mutexes: write M0):
    #0 lldb_private::Process::SetSTDIOFileDescriptor(int) Process.cpp:4528 (liblldb.18.0.0git.dylib:arm64+0x54aa5c) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #1 lldb_private::Platform::DebugProcess(lldb_private::ProcessLaunchInfo&, lldb_private::Debugger&, lldb_private::Target&, lldb_private::Status&) Platform.cpp:1120 (liblldb.18.0.0git.dylib:arm64+0x52bc54) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #2 lldb_private::PlatformDarwin::DebugProcess(lldb_private::ProcessLaunchInfo&, lldb_private::Debugger&, lldb_private::Target&, lldb_private::Status&) PlatformDarwin.cpp:711 (liblldb.18.0.0git.dylib:arm64+0x872cc8) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)
    #3 lldb_private::Target::Launch(lldb_private::ProcessLaunchInfo&, lldb_private::Stream*) Target.cpp:3235 (liblldb.18.0.0git.dylib:arm64+0x5b118c) (BuildId: 6e77321523b936d2955d63d4d25df7cd32000000200000000100000000000e00)

Line 4559 is

IOHandlerSP io_handler_sp(m_process_input_reader);

Line 4528 is

m_process_input_reader =
    std::make_shared<IOHandlerProcessSTDIO>(this, fd);

[augusto2112]

@JDevlieghere do you have any opinions on this?

lldb/include/lldb/Target/Process.h

lldb::DynamicCheckerFunctionsUP m_dynamic_checkers_up; ///< The functions used
                                                          /// data that
                                                          /// expressions use.
   lldb::OperatingSystemUP m_os_up;
   lldb::SystemRuntimeUP m_system_runtime_up;
   lldb::UnixSignalsSP
       m_unix_signals_sp; /// This is the current signal set for this process.
   lldb::ABISP m_abi_sp;
   lldb::IOHandlerSP m_process_input_reader;
+  mutable std::mutex m_process_input_reader_mutex;
   ThreadedCommunication m_stdio_communication;
   std::recursive_mutex m_stdio_communication_mutex;
   bool m_stdin_forward; /// Remember if stdin must be forwarded to remote debug
                         /// server
   std::string m_stdout_data;
   std::string m_stderr_data;
   std::recursive_mutex m_profile_data_comm_mutex;
   std::vector<std::string> m_profile_data;
protected:
 
   bool PushProcessIOHandler();
 
   bool PopProcessIOHandler();
 
   bool ProcessIOHandlerIsActive();
 
   bool ProcessIOHandlerExists() const {
+    std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
     return static_cast<bool>(m_process_input_reader);
   }
 
   Status StopForDestroyOrDetach(lldb::EventSP &exit_event_sp);
 
   virtual Status UpdateAutomaticSignalFiltering();
 
   void LoadOperatingSystemPlugin(bool flush);

lldb/source/Target/Process.cpp

StateType Process::GetNextEvent(EventSP &event_sp) {
 
   return state;
 }
 
 void Process::SyncIOHandler(uint32_t iohandler_id,
                             const Timeout<std::micro> &timeout) {
   // don't sync (potentially context switch) in case where there is no process
   // IO
+  std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
   if (!m_process_input_reader)
     return;
 
   auto Result = m_iohandler_sync.WaitForValueNotEqualTo(iohandler_id, timeout);
 
   Log *log = GetLog(LLDBLog::Process);
   if (Result) {
     LLDB_LOG(

 Status Process::LaunchPrivate(ProcessLaunchInfo &launch_info, StateType &state,
                               EventSP &event_sp) {
   Status error;
   m_abi_sp.reset();
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_system_runtime_up.reset();
   m_os_up.reset();
+
+  {
+    std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
     m_process_input_reader.reset();
+  }
 
   Module *exe_module = GetTarget().GetExecutableModulePointer();
 
   // The "remote executable path" is hooked up to the local Executable
   // module.  But we should be able to debug a remote process even if the
   // executable module only exists on the remote.  However, there needs to
   // be a way to express this path, without actually having a module.
   // The way to do that is to set the ExecutableFile in the LaunchInfo.

 
 Status Process::WillAttachToProcessWithName(const char *process_name,
                                             bool wait_for_launch) {
   return DoWillAttachToProcessWithName(process_name, wait_for_launch);
 }
 
 Status Process::Attach(ProcessAttachInfo &attach_info) {
   m_abi_sp.reset();
+  {
+    std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
     m_process_input_reader.reset();
+  }
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_system_runtime_up.reset();
   m_os_up.reset();
 
   lldb::pid_t attach_pid = attach_info.GetProcessID();
   Status error;
   if (attach_pid == LLDB_INVALID_PROCESS_ID) {
if (log) {
           exe_module_sp ? exe_module_sp->GetFileSpec().GetPath().c_str()
                         : "<none>");
     }
   }
 }
 
 Status Process::ConnectRemote(llvm::StringRef remote_url) {
   m_abi_sp.reset();
+  {
+    std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
     m_process_input_reader.reset();
+  }
 
   // Find the process and its architecture.  Make sure it matches the
   // architecture of the current Target, and if not adjust it.
 
   Status error(DoConnectRemote(remote_url));
   if (error.Success()) {
     if (GetID() != LLDB_INVALID_PROCESS_ID) {
       EventSP event_sp;
if (error.Success()) {
     if (error.Success()) {
       DidDestroy();
       StopPrivateStateThread();
     }
     m_stdio_communication.StopReadThread();
     m_stdio_communication.Disconnect();
     m_stdin_forward = false;
 
+    {
+      std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
       if (m_process_input_reader) {
         m_process_input_reader->SetIsDone(true);
         m_process_input_reader->Cancel();
         m_process_input_reader.reset();
       }
+    }
 
     // If we exited when we were waiting for a process to stop, then forward
     // the event here so we don't lose the event
     if (exit_event_sp) {
       // Directly broadcast our exited event because we shut down our private
       // state thread above
       BroadcastEvent(exit_event_sp);
     }
void Process::SetSTDIOFileDescriptor(int fd) {
   m_stdio_communication.SetConnection(
       std::make_unique<ConnectionFileDescriptor>(fd, true));
   if (m_stdio_communication.IsConnected()) {
     m_stdio_communication.SetReadThreadBytesReceivedCallback(
         STDIOReadThreadBytesReceived, this);
     m_stdio_communication.StartReadThread();
 
     // Now read thread is set up, set up input reader.
-
+    {
+      std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
       if (!m_process_input_reader)
         m_process_input_reader =
             std::make_shared<IOHandlerProcessSTDIO>(this, fd);
     }
   }
+}
 
 bool Process::ProcessIOHandlerIsActive() {
+  std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp)
     return GetTarget().GetDebugger().IsTopIOHandler(io_handler_sp);
   return false;
 }
+
 bool Process::PushProcessIOHandler() {
+  std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp) {
     Log *log = GetLog(LLDBLog::Process);
     LLDB_LOGF(log, "Process::%s pushing IO handler", __FUNCTION__);
 
     io_handler_sp->SetIsDone(false);
     // If we evaluate an utility function, then we don't cancel the current
     // IOHandler. Our IOHandler is non-interactive and shouldn't disturb the
     // existing IOHandler that potentially provides the user interface (e.g.
     // the IOHandler for Editline).
     bool cancel_top_handler = !m_mod_id.IsRunningUtilityFunction();
     GetTarget().GetDebugger().RunIOHandlerAsync(io_handler_sp,
                                                 cancel_top_handler);
     return true;
   }
   return false;
 }
 
 bool Process::PopProcessIOHandler() {
+  std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp)
     return GetTarget().GetDebugger().RemoveIOHandler(io_handler_sp);
   return false;
 }
 
 // The process needs to know about installed plug-ins
 void Process::SettingsInitialize() { Thread::SettingsInitialize(); }