[all-commits] [llvm/llvm-project] 102227: [lldb] Avoid data race in IOHandlerProcessSTDIO

[Jonas Devlieghere via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 10222764a9a3b09678013d3d13b66790ea3298d9
      https://github.com/llvm/llvm-project/commit/10222764a9a3b09678013d3d13b66790ea3298d9
  Author: Jonas Devlieghere <jonas at devlieghere.com>
  Date:   2022-03-02 (Wed, 02 Mar 2022)

  Changed paths:
    M lldb/source/Target/Process.cpp
    A lldb/test/API/iohandler/stdio/Makefile
    A lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
    A lldb/test/API/iohandler/stdio/main.cpp

  Log Message:
  -----------
  [lldb] Avoid data race in IOHandlerProcessSTDIO

This patch fixes a data race in IOHandlerProcessSTDIO. The race is
happens between the main thread and the event handling thread. The main
thread is running the IOHandler (IOHandlerProcessSTDIO::Run()) when an
event comes in that makes us pop the process IO handler which involves
cancelling the IOHandler (IOHandlerProcessSTDIO::Cancel). The latter
calls SetIsDone(true) which modifies m_is_done. At the same time, we
have the main thread reading the variable through GetIsDone().

This patch avoids the race by using a mutex to synchronize the two
threads. On the event thread, in IOHandlerProcessSTDIO ::Cancel method,
we obtain the lock before changing the value of m_is_done. On the main
thread, in IOHandlerProcessSTDIO::Run(), we obtain the lock before
reading the value of m_is_done. Additionally, we delay calling SetIsDone
until after the loop exists, to avoid a potential race between the two
writes.

  Write of size 1 at 0x00010b66bb68 by thread T7 (mutexes: write M2862, write M718324145051843688):
    #0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 (liblldb.15.0.0git.dylib:arm64+0x971d84)
    #1 IOHandlerProcessSTDIO::Cancel() Process.cpp:4382 (liblldb.15.0.0git.dylib:arm64+0x5ddfec)
    #2 lldb_private::Debugger::PopIOHandler(std::__1::shared_ptr<lldb_private::IOHandler> const&) Debugger.cpp:1156 (liblldb.15.0.0git.dylib:arm64+0x3cb2a8)
    #3 lldb_private::Debugger::RemoveIOHandler(std::__1::shared_ptr<lldb_private::IOHandler> const&) Debugger.cpp:1063 (liblldb.15.0.0git.dylib:arm64+0x3cbd2c)
    #4 lldb_private::Process::PopProcessIOHandler() Process.cpp:4487 (liblldb.15.0.0git.dylib:arm64+0x5c583c)
    #5 lldb_private::Debugger::HandleProcessEvent(std::__1::shared_ptr<lldb_private::Event> const&) Debugger.cpp:1549 (liblldb.15.0.0git.dylib:arm64+0x3ceabc)
    #6 lldb_private::Debugger::DefaultEventHandler() Debugger.cpp:1622 (liblldb.15.0.0git.dylib:arm64+0x3cf2c0)
    #7 std::__1::__function::__func<lldb_private::Debugger::StartEventHandlerThread()::$_2, std::__1::allocator<lldb_private::Debugger::StartEventHandlerThread()::$_2>, void* ()>::operator()() function.h:352 (liblldb.15.0.0git.dylib:arm64+0x3d1bd8)
    #8 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) HostNativeThreadBase.cpp:62 (liblldb.15.0.0git.dylib:arm64+0x4c71ac)
    #9 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) HostThreadMacOSX.mm:18 (liblldb.15.0.0git.dylib:arm64+0x29ef544)

  Previous read of size 1 at 0x00010b66bb68 by main thread:
    #0 lldb_private::IOHandler::GetIsDone() IOHandler.h:92 (liblldb.15.0.0git.dylib:arm64+0x971db8)
    #1 IOHandlerProcessSTDIO::Run() Process.cpp:4339 (liblldb.15.0.0git.dylib:arm64+0x5ddc7c)
    #2 lldb_private::Debugger::RunIOHandlers() Debugger.cpp:982 (liblldb.15.0.0git.dylib:arm64+0x3cb48c)
    #3 lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&) CommandInterpreter.cpp:3298 (liblldb.15.0.0git.dylib:arm64+0x506478)
    #4 lldb::SBDebugger::RunCommandInterpreter(bool, bool) SBDebugger.cpp:1166 (liblldb.15.0.0git.dylib:arm64+0x53604)
    #5 Driver::MainLoop() Driver.cpp:634 (lldb:arm64+0x100006294)
    #6 main Driver.cpp:853 (lldb:arm64+0x100007344)

Differential revision: https://reviews.llvm.org/D120762


  Commit: 37eb15ad7ab25e0135b759f3ecdd6ada5ce82330
      https://github.com/llvm/llvm-project/commit/37eb15ad7ab25e0135b759f3ecdd6ada5ce82330
  Author: Jonas Devlieghere <jonas at devlieghere.com>
  Date:   2022-03-02 (Wed, 02 Mar 2022)

  Changed paths:
    M lldb/include/lldb/Core/IOHandler.h

  Log Message:
  -----------
  [lldb] Devirtualize IOHandler::{IsActive,SetIsDone,GetIsDone} (NFC)

There are no implementations overriding these methods.

Differential revision: https://reviews.llvm.org/D120766


Compare: https://github.com/llvm/llvm-project/compare/b05918f2f791...37eb15ad7ab2