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

Reply via email to