JDevlieghere created this revision. JDevlieghere added reviewers: labath, jingham. Herald added a subscriber: kristof.beyls. Herald added a project: All. JDevlieghere requested review of this revision.
This patch introduces a small utility called `LockableStream` to protect the debugger's output and error stream. Nothing prevents someone from bypassing this and going straight through the debugger's `Get{Output,Error}File`. I don't think we want to get rid of that, but at least now we have something that allows us to access these streams in a safe way. This fixes a real world race between the main thread and the default event handler thread: Write of size 8 at 0x000106c01bc0 by thread T1 (mutexes: write M4069, write M2753): #0 lldb_private::IOHandlerEditline::PrintAsync(lldb_private::Stream*, char const*, unsigned long) IOHandler.cpp:636 (liblldb.15.0.0git.dylib:arm64+0x3ef4a4) #1 lldb_private::IOHandlerStack::PrintAsync(lldb_private::Stream*, char const*, unsigned long) IOHandler.cpp:129 (liblldb.15.0.0git.dylib:arm64+0x3ec84c) #2 lldb_private::Debugger::PrintAsync(char const*, unsigned long, bool) Debugger.cpp:1074 (liblldb.15.0.0git.dylib:arm64+0x3cb03c) #3 lldb_private::StreamAsynchronousIO::~StreamAsynchronousIO() StreamAsynchronousIO.cpp:21 (liblldb.15.0.0git.dylib:arm64+0x44749c) #4 std::__1::__shared_ptr_emplace<lldb_private::StreamAsynchronousIO, std::__1::allocator<lldb_private::StreamAsynchronousIO> >::__on_zero_shared() shared_ptr.h:315 (liblldb.15.0.0git.dylib:arm64+0x3d0f80) #5 lldb_private::Debugger::HandleThreadEvent(std::__1::shared_ptr<lldb_private::Event> const&) Debugger.cpp:1591 (liblldb.15.0.0git.dylib:arm64+0x3ce2cc) #6 lldb_private::Debugger::DefaultEventHandler() Debugger.cpp:1659 (liblldb.15.0.0git.dylib:arm64+0x3ce720) #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+0x3d145c) #8 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) HostNativeThreadBase.cpp:62 (liblldb.15.0.0git.dylib:arm64+0x4c67a4) #9 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) HostThreadMacOSX.mm:18 (liblldb.15.0.0git.dylib:arm64+0x29cebac) Previous write of size 8 at 0x000106c01bc0 by main thread (mutexes: write M2754): #0 lldb_private::CommandInterpreter::PrintCommandOutput(lldb_private::Stream&, llvm::StringRef) CommandInterpreter.cpp:2992 (liblldb.15.0.0git.dylib:arm64+0x502888) #1 lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&) CommandInterpreter.cpp:3060 (liblldb.15.0.0git.dylib:arm64+0x502fec) #2 non-virtual thunk to lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&) CommandInterpreter.cpp (liblldb.15.0.0git.dylib:arm64+0x503410) #3 lldb_private::IOHandlerEditline::Run() IOHandler.cpp (liblldb.15.0.0git.dylib:arm64+0x3ef2f4) #4 lldb_private::Debugger::RunIOHandlerSync(std::__1::shared_ptr<lldb_private::IOHandler> const&) Debugger.cpp:1037 (liblldb.15.0.0git.dylib:arm64+0x3cabc0) #5 lldb_private::CommandInterpreter::HandleCommandsFromFile(lldb_private::FileSpec&, lldb_private::CommandInterpreterRunOptions const&, lldb_private::CommandReturnObject&) CommandInterpreter.cpp:2748 (liblldb.15.0.0git.dylib:arm64+0x5000d4) #6 CommandObjectCommandsSource::DoExecute(lldb_private::Args&, lldb_private::CommandReturnObject&) CommandObjectCommands.cpp:187 (liblldb.15.0.0git.dylib:arm64+0x284d0b4) #7 lldb_private::CommandObjectParsed::Execute(char const*, lldb_private::CommandReturnObject&) CommandObject.cpp:998 (liblldb.15.0.0git.dylib:arm64+0x50bf04) #8 lldb_private::CommandInterpreter::HandleCommand(char const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&) CommandInterpreter.cpp:1981 (liblldb.15.0.0git.dylib:arm64+0x4feaf0) #9 lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&) CommandInterpreter.cpp:3049 (liblldb.15.0.0git.dylib:arm64+0x502ee0) #10 non-virtual thunk to lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&) CommandInterpreter.cpp (liblldb.15.0.0git.dylib:arm64+0x503410) #11 lldb_private::IOHandlerEditline::Run() IOHandler.cpp (liblldb.15.0.0git.dylib:arm64+0x3ef2f4) #12 lldb_private::Debugger::RunIOHandlers() Debugger.cpp:1008 (liblldb.15.0.0git.dylib:arm64+0x3ca8f8) #13 lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&) CommandInterpreter.cpp:3299 (liblldb.15.0.0git.dylib:arm64+0x5048a4) #14 lldb::SBDebugger::RunCommandInterpreter(lldb::SBCommandInterpreterRunOptions const&) SBDebugger.cpp:1203 (liblldb.15.0.0git.dylib:arm64+0x523f8) #15 Driver::MainLoop() Driver.cpp:575 (lldb:arm64+0x10000606c) #16 main Driver.cpp:851 (lldb:arm64+0x100007388) https://reviews.llvm.org/D121500 Files: lldb/include/lldb/Utility/Stream.h lldb/source/Core/Debugger.cpp Index: lldb/source/Core/Debugger.cpp =================================================================== --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -733,8 +733,10 @@ : UserID(g_unique_id++), Properties(std::make_shared<OptionValueProperties>()), m_input_file_sp(std::make_shared<NativeFile>(stdin, false)), - m_output_stream_sp(std::make_shared<StreamFile>(stdout, false)), - m_error_stream_sp(std::make_shared<StreamFile>(stderr, false)), + m_output_stream_sp( + std::make_shared<LockableStream<StreamFile>>(stdout, false)), + m_error_stream_sp( + std::make_shared<LockableStream<StreamFile>>(stderr, false)), m_input_recorder(nullptr), m_broadcaster_manager_sp(BroadcasterManager::MakeBroadcasterManager()), m_terminal_state(), m_target_list(*this), m_platform_list(), Index: lldb/include/lldb/Utility/Stream.h =================================================================== --- lldb/include/lldb/Utility/Stream.h +++ lldb/include/lldb/Utility/Stream.h @@ -19,6 +19,7 @@ #include <cstdarg> #include <cstddef> #include <cstdint> +#include <mutex> #include <type_traits> namespace lldb_private { @@ -44,7 +45,6 @@ Stream *m_stream; /// Bytes we have written so far when ByteDelta was created. size_t m_start; - public: ByteDelta(Stream &s) : m_stream(&s), m_start(s.GetWrittenBytes()) {} /// Returns the number of bytes written to the given Stream since this @@ -412,6 +412,26 @@ RawOstreamForward m_forwarder; }; +template <typename T, typename M = std::mutex> class LockableStream : public T { +public: + using T::T; + ~LockableStream() = default; + + void Flush() override { + std::lock_guard<M> guard(m_mutex); + T::Flush(); + } + +protected: + size_t WriteImpl(const void *s, size_t length) override { + std::lock_guard<M> guard(m_mutex); + return T::WriteImpl(s, length); + } + +private: + M m_mutex; +}; + /// Output an address value to this stream. /// /// Put an address \a addr out to the stream with optional \a prefix and \a
Index: lldb/source/Core/Debugger.cpp =================================================================== --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -733,8 +733,10 @@ : UserID(g_unique_id++), Properties(std::make_shared<OptionValueProperties>()), m_input_file_sp(std::make_shared<NativeFile>(stdin, false)), - m_output_stream_sp(std::make_shared<StreamFile>(stdout, false)), - m_error_stream_sp(std::make_shared<StreamFile>(stderr, false)), + m_output_stream_sp( + std::make_shared<LockableStream<StreamFile>>(stdout, false)), + m_error_stream_sp( + std::make_shared<LockableStream<StreamFile>>(stderr, false)), m_input_recorder(nullptr), m_broadcaster_manager_sp(BroadcasterManager::MakeBroadcasterManager()), m_terminal_state(), m_target_list(*this), m_platform_list(), Index: lldb/include/lldb/Utility/Stream.h =================================================================== --- lldb/include/lldb/Utility/Stream.h +++ lldb/include/lldb/Utility/Stream.h @@ -19,6 +19,7 @@ #include <cstdarg> #include <cstddef> #include <cstdint> +#include <mutex> #include <type_traits> namespace lldb_private { @@ -44,7 +45,6 @@ Stream *m_stream; /// Bytes we have written so far when ByteDelta was created. size_t m_start; - public: ByteDelta(Stream &s) : m_stream(&s), m_start(s.GetWrittenBytes()) {} /// Returns the number of bytes written to the given Stream since this @@ -412,6 +412,26 @@ RawOstreamForward m_forwarder; }; +template <typename T, typename M = std::mutex> class LockableStream : public T { +public: + using T::T; + ~LockableStream() = default; + + void Flush() override { + std::lock_guard<M> guard(m_mutex); + T::Flush(); + } + +protected: + size_t WriteImpl(const void *s, size_t length) override { + std::lock_guard<M> guard(m_mutex); + return T::WriteImpl(s, length); + } + +private: + M m_mutex; +}; + /// Output an address value to this stream. /// /// Put an address \a addr out to the stream with optional \a prefix and \a
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits