[lldb] Introduce the concept of a log handler (NFC)

[JDevlieghere]

Summary

This patch introduces the concept of a log handlers. Log handlers allow customizing the way log output is emitted. The StreamCallback class tried to do something conceptually similar. The benefit of the log handler interface is that you don't need to conform to llvm's raw_ostream interface.

This patch is in preparation for a new kind of a new type of log handler.

[JDevlieghere]

Update unit tests

[clayborg]

lldb/source/Utility/Log.cpp

321–322

Since we are improving stuff here, should we add a mutex to each handler and use the handler specific mutex if needed?

321–322

Will this copy the string? The Emit() calls take a "std::string", maybe we should switch it to "const std::string &" to avoid any copies?

321–322

Ditto

[JDevlieghere]

lldb/source/Utility/Log.cpp

321–322

Yeah, was planning on std::move-ing everything but I already found a few cases where that doesn't work. This should indeed take a reference.

321–322

Yup, I think that makes sense

[mib]

lldb/source/Utility/Log.cpp

321–322

+1

[clayborg]

lldb/source/Utility/Log.cpp

321–322

The other option is to have the Emit functions use llvm::StringRef?:

virtual void Emit(llvm::StringRef s) = 0;

[JDevlieghere]

• Make Emit take a StringRef
• Move mutex in the log handler

[clayborg]

lldb/include/lldb/Utility/Log.h

54

Maybe we don't expose this mutex at all and we have a EmitThreadSafe(...) method?:

void EmitThreadSafe(llvm::StringRef message) {
  std::lock_guard<std::mutex> guard(m_mutex);
  Emit(message);
}

lldb/source/Utility/Log.cpp

321–322

If we like the idea of EmitThreadSafe then the code can look like this

322

Do we ever not call Flush() after a Emit() call? If so do we even need the flush call, or should we just leave it up to the specific implementation to do what it wants in Emit(...)?

323

Do we ever not call Flush() after a Emit() call? If so do we even need the flush call, or should we just leave it up to the specific implementation to do what it wants in Emit(...)?

[JDevlieghere]

• Add EmitThreadSafe
• Remove Flush

[clayborg]

LGTM!

[labath]

I think we should talk about the threadsafe flag. The reason that the non-threadsafe mode is safe now is because we're using unbuffered streams, where each stream operation will map to a single write syscall (which will then be serialized by the OS somehow). And since we pre-format the log messages into a temporary buffer, we shouldn't even get overlapping messages in the output. In fact, I would say that, before this, the threadsafe mode was mostly useless.

All of that goes out the window once you start doing arbitrary logic in place of log writing. As they're implemented now, neither BufferedLogHandler nor RotatingLogHandler are thread-safe. (Making the rotating handler thread safe (without locks) should be relatively easy -- assuming we're willing to accept races between printing messages, and new messages coming in -- but I don't think that the Buffered handler can be made safe without locking at least the flushing part). An it's not just that the messages may be slightly wonky. I think it wouldn't be hard to crash the process by spewing log messages from multiple threads (and we're pretty good at spewing :P).

So, what I'd recommend is to drop the thread-safe flag, and make the locking strategy the responsibility of the individual log handler. When writing to an unbuffered stream, we wouldn't need any locks at all. The circular handler could either lock into that or, if you're into that sort of thing, use atomics to make enqueueing lock-free. The buffered handler could either use a buffered ostream, wrapped in a mutex, or do something similar to the circular one, while locking only the flushing code.

[JDevlieghere]

In D127922#3595530, @labath wrote:

I think we should talk about the threadsafe flag. The reason that the non-threadsafe mode is safe now is because we're using unbuffered streams, where each stream operation will map to a single write syscall (which will then be serialized by the OS somehow). And since we pre-format the log messages into a temporary buffer, we shouldn't even get overlapping messages in the output. In fact, I would say that, before this, the threadsafe mode was mostly useless.

All of that goes out the window once you start doing arbitrary logic in place of log writing. As they're implemented now, neither BufferedLogHandler nor RotatingLogHandler are thread-safe. (Making the rotating handler thread safe (without locks) should be relatively easy -- assuming we're willing to accept races between printing messages, and new messages coming in -- but I don't think that the Buffered handler can be made safe without locking at least the flushing part). An it's not just that the messages may be slightly wonky. I think it wouldn't be hard to crash the process by spewing log messages from multiple threads (and we're pretty good at spewing :P).

So, what I'd recommend is to drop the thread-safe flag, and make the locking strategy the responsibility of the individual log handler. When writing to an unbuffered stream, we wouldn't need any locks at all. The circular handler could either lock into that or, if you're into that sort of thing, use atomics to make enqueueing lock-free. The buffered handler could either use a buffered ostream, wrapped in a mutex, or do something similar to the circular one, while locking only the flushing code.

Yup that makes sense to me. I'll put up a patch for that tomorrow.

lldb/include/lldb/Core/Debugger.h

 
 namespace llvm {
 class raw_ostream;
 class ThreadPool;
 }
 
 namespace lldb_private {
 class Address;
+class CallbackLogHandler;
 class CommandInterpreter;
+class LogHandler;
 class Process;
 class Stream;
 class SymbolContext;
 class Target;
 
 namespace repro {
 class DataRecorder;
 }
protected:
   std::array<lldb::ScriptInterpreterSP, lldb::eScriptLanguageUnknown>
       m_script_interpreters;
 
   IOHandlerStack m_io_handler_stack;
   std::recursive_mutex m_io_handler_synchronous_mutex;
 
   llvm::Optional<uint64_t> m_current_event_id;
 
-  llvm::StringMap<std::weak_ptr<llvm::raw_ostream>> m_log_streams;
-  std::shared_ptr<llvm::raw_ostream> m_log_callback_stream_sp;
+  llvm::StringMap<std::weak_ptr<LogHandler>> m_stream_handlers;
+  std::shared_ptr<CallbackLogHandler> m_callback_handler_sp;
   ConstString m_instance_name;
   static LoadPluginCallbackType g_load_plugin_callback;
   typedef std::vector<llvm::sys::DynamicLibrary> LoadedPluginsList;
   LoadedPluginsList m_loaded_plugins;
   HostThread m_event_handler_thread;
   HostThread m_io_handler_thread;
   Broadcaster m_sync_broadcaster; ///< Private debugger synchronization.
   Broadcaster m_broadcaster;      ///< Public Debugger event broadcaster.

lldb/include/lldb/Utility/Log.h

 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/RWMutex.h"
 
 #include <atomic>
 #include <cstdarg>
 #include <cstdint>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <type_traits>
 
 namespace llvm {
 class raw_ostream;
 }
 // Logging Options
 #define LLDB_LOG_OPTION_THREADSAFE (1u << 0)
 #define LLDB_LOG_OPTION_VERBOSE (1u << 1)
 #define LLDB_LOG_OPTION_PREPEND_SEQUENCE (1u << 3)
 #define LLDB_LOG_OPTION_PREPEND_TIMESTAMP (1u << 4)
 #define LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD (1u << 5)
 #define LLDB_LOG_OPTION_PREPEND_THREAD_NAME (1U << 6)
 #define LLDB_LOG_OPTION_BACKTRACE (1U << 7)
 #define LLDB_LOG_OPTION_APPEND (1U << 8)
 #define LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION (1U << 9)
 
 // Logging Functions
 namespace lldb_private {
 
+class LogHandler {
+public:
+  virtual ~LogHandler() = default;
+  virtual void Emit(llvm::StringRef message) = 0;
+  void EmitThreadSafe(llvm::StringRef message);
+
+private:
+  std::mutex m_mutex;
+};
+
+class StreamLogHandler : public LogHandler {
+public:
+  StreamLogHandler(int fd, bool should_close, bool unbuffered);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr<StreamLogHandler> Create(int fd, bool unbuffered);
+
+private:
+  llvm::raw_fd_ostream m_stream;
+};
+
+class CallbackLogHandler : public LogHandler {
+public:
+  CallbackLogHandler(lldb::LogOutputCallback callback, void *baton);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr<CallbackLogHandler>
+  Create(lldb::LogOutputCallback callback, void *baton);
+
+private:
+  lldb::LogOutputCallback m_callback;
+  void *m_baton;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
   /// enum class MyLog : MaskType {
   ///   Channel0 = Log::ChannelFlag<0>,
   ///   Channel1 = Log::ChannelFlag<1>,
   ///   ...,
   ///   LLVM_MARK_AS_BITMASK_ENUM(LastChannel),
public:
   };
 
 
   // Static accessors for logging channels
   static void Register(llvm::StringRef name, Channel &channel);
   static void Unregister(llvm::StringRef name);
 
   static bool
-  EnableLogChannel(const std::shared_ptr<llvm::raw_ostream> &log_stream_sp,
+  EnableLogChannel(const std::shared_ptr<LogHandler> &log_handler_sp,
                    uint32_t log_options, llvm::StringRef channel,
                    llvm::ArrayRef<const char *> categories,
                    llvm::raw_ostream &error_stream);
 
   static bool DisableLogChannel(llvm::StringRef channel,
                                 llvm::ArrayRef<const char *> categories,
                                 llvm::raw_ostream &error_stream);
 
private:
   Channel &m_channel;
 
   // The mutex makes sure enable/disable operations are thread-safe. The
   // options and mask variables are atomic to enable their reading in
   // Channel::GetLogIfAny without taking the mutex to speed up the fast path.
   // Their modification however, is still protected by this mutex.
   llvm::sys::RWMutex m_mutex;
 
-  std::shared_ptr<llvm::raw_ostream> m_stream_sp;
+  std::shared_ptr<LogHandler> m_handler;
   std::atomic<uint32_t> m_options{0};
   std::atomic<MaskType> m_mask{0};
 
   void WriteHeader(llvm::raw_ostream &OS, llvm::StringRef file,
                    llvm::StringRef function);
   void WriteMessage(const std::string &message);
 
   void Format(llvm::StringRef file, llvm::StringRef function,
               const llvm::formatv_object_base &payload);
 
-  std::shared_ptr<llvm::raw_ostream> GetStream() {
+  std::shared_ptr<LogHandler> GetHandler() {
     llvm::sys::ScopedReader lock(m_mutex);
-    return m_stream_sp;
+    return m_handler;
   }
 
-  void Enable(const std::shared_ptr<llvm::raw_ostream> &stream_sp,
-              uint32_t options, uint32_t flags);
+  void Enable(const std::shared_ptr<LogHandler> &handler_sp, uint32_t options,
+              uint32_t flags);
 
   void Disable(uint32_t flags);
 
   typedef llvm::StringMap<Log> ChannelMap;
   static llvm::ManagedStatic<ChannelMap> g_channel_map;
 
   static void ForEachCategory(
       const Log::ChannelMap::value_type &entry,

lldb/include/lldb/Utility/StreamCallback.h

-//===-- StreamCallback.h -----------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_UTILITY_STREAMCALLBACK_H
-#define LLDB_UTILITY_STREAMCALLBACK_H
-
-#include "lldb/lldb-types.h"
-#include "llvm/Support/raw_ostream.h"
-
-#include <cstddef>
-#include <cstdint>
-
-namespace lldb_private {
-
-class StreamCallback : public llvm::raw_ostream {
-public:
-  StreamCallback(lldb::LogOutputCallback callback, void *baton);
-  ~StreamCallback() override = default;
-
-private:
-  lldb::LogOutputCallback m_callback;
-  void *m_baton;
-
-  void write_impl(const char *Ptr, size_t Size) override;
-  uint64_t current_pos() const override;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_UTILITY_STREAMCALLBACK_H

lldb/source/Core/Debugger.cpp

 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
-#include "lldb/Utility/StreamCallback.h"
 #include "lldb/Utility/StreamString.h"
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h"
 #include "lldb/Host/windows/windows.h"
 #endif
 
 #include "llvm/ADT/None.h"
: UserID(g_unique_id++),
       m_io_handler_stack(), m_instance_name(), m_loaded_plugins(),
       m_event_handler_thread(), m_io_handler_thread(),
       m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
       m_broadcaster(m_broadcaster_manager_sp,
                     GetStaticBroadcasterClass().AsCString()),
       m_forward_listener_sp(), m_clear_once() {
   m_instance_name.SetString(llvm::formatv("debugger_{0}", GetID()).str());
   if (log_callback)
-    m_log_callback_stream_sp =
-        std::make_shared<StreamCallback>(log_callback, baton);
+    m_callback_handler_sp = CallbackLogHandler::Create(log_callback, baton);
   m_command_interpreter_up->Initialize();
   // Always add our default platform to the platform list
   PlatformSP default_platform_sp(Platform::GetHostPlatform());
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
   // Create the dummy target.
   {
return FormatEntity::Format(*format, s, sc, exe_ctx, addr, nullptr,
                               function_changed, initial_function);
 }
 
 void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
                                   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
   // open logging streams, I just redirect everything from here on out to the
   // callback.
-  m_log_callback_stream_sp =
-      std::make_shared<StreamCallback>(log_callback, baton);
+  m_callback_handler_sp = CallbackLogHandler::Create(log_callback, baton);
 }
 
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
                                   const std::string &message,
                                   uint64_t completed, uint64_t total,
                                   bool is_debugger_specific) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
ReportDiagnosticImpl(DiagnosticEventData::Type::Error, std::move(message),
                        debugger_id, once);
 }
 
 bool Debugger::EnableLog(llvm::StringRef channel,
                          llvm::ArrayRef<const char *> categories,
                          llvm::StringRef log_file, uint32_t log_options,
                          llvm::raw_ostream &error_stream) {
   const bool should_close = true;
-  const bool unbuffered = true;
 
-  std::shared_ptr<llvm::raw_ostream> log_stream_sp;
-  if (m_log_callback_stream_sp) {
-    log_stream_sp = m_log_callback_stream_sp;
+  std::shared_ptr<LogHandler> log_handler_sp;
+  if (m_callback_handler_sp) {
+    log_handler_sp = m_callback_handler_sp;
     // For now when using the callback mode you always get thread & timestamp.
     log_options |=
         LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-    log_stream_sp = std::make_shared<llvm::raw_fd_ostream>(
-        GetOutputFile().GetDescriptor(), !should_close, unbuffered);
+    log_handler_sp = StreamLogHandler::Create(GetOutputFile().GetDescriptor(),
+                                              !should_close);
   } else {
-    auto pos = m_log_streams.find(log_file);
-    if (pos != m_log_streams.end())
-      log_stream_sp = pos->second.lock();
-    if (!log_stream_sp) {
+    auto pos = m_stream_handlers.find(log_file);
+    if (pos != m_stream_handlers.end())
+      log_handler_sp = pos->second.lock();
+    if (!log_handler_sp) {
       File::OpenOptions flags =
           File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate;
       if (log_options & LLDB_LOG_OPTION_APPEND)
         flags |= File::eOpenOptionAppend;
       else
         flags |= File::eOpenOptionTruncate;
       llvm::Expected<FileUP> file = FileSystem::Instance().Open(
           FileSpec(log_file), flags, lldb::eFilePermissionsFileDefault, false);
       if (!file) {
         error_stream << "Unable to open log file '" << log_file
                      << "': " << llvm::toString(file.takeError()) << "\n";
         return false;
       }
 
-      log_stream_sp = std::make_shared<llvm::raw_fd_ostream>(
-          (*file)->GetDescriptor(), should_close, unbuffered);
-      m_log_streams[log_file] = log_stream_sp;
+      log_handler_sp =
+          StreamLogHandler::Create((*file)->GetDescriptor(), should_close);
+      m_stream_handlers[log_file] = log_handler_sp;
     }
   }
-  assert(log_stream_sp);
+  assert(log_handler_sp);
 
   if (log_options == 0)
     log_options =
         LLDB_LOG_OPTION_PREPEND_THREAD_NAME | LLDB_LOG_OPTION_THREADSAFE;
 
-  return Log::EnableLogChannel(log_stream_sp, log_options, channel, categories,
+  return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
                                error_stream);
 }
 
 ScriptInterpreter *
 Debugger::GetScriptInterpreter(bool can_create,
                                llvm::Optional<lldb::ScriptLanguage> language) {
   std::lock_guard<std::recursive_mutex> locker(m_script_interpreter_mutex);
   lldb::ScriptLanguage script_language =

lldb/source/Utility/CMakeLists.txt

add_lldb_library(lldbUtility
   Reproducer.cpp
   Instrumentation.cpp
   ReproducerProvider.cpp
   Scalar.cpp
   SelectHelper.cpp
   State.cpp
   Status.cpp
   Stream.cpp
-  StreamCallback.cpp
   StreamString.cpp
   StringExtractor.cpp
   StringExtractorGDBRemote.cpp
   StringLexer.cpp
   StringList.cpp
   StructuredData.cpp
   TildeExpressionResolver.cpp
   Timer.cpp

lldb/source/Utility/Log.cpp

stream << llvm::formatv("error: unrecognized log category '{0}'\n",
                             category);
     list_categories = true;
   }
   if (list_categories)
     ListCategories(stream, entry);
   return flags;
 }
 
-void Log::Enable(const std::shared_ptr<llvm::raw_ostream> &stream_sp,
+void Log::Enable(const std::shared_ptr<LogHandler> &handler_sp,
                  uint32_t options, uint32_t flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_or(flags, std::memory_order_relaxed);
   if (mask | flags) {
     m_options.store(options, std::memory_order_relaxed);
-    m_stream_sp = stream_sp;
+    m_handler = handler_sp;
     m_channel.log_ptr.store(this, std::memory_order_relaxed);
   }
 }
 
 void Log::Disable(uint32_t flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
   if (!(mask & ~flags)) {
-    m_stream_sp.reset();
+    m_handler.reset();
     m_channel.log_ptr.store(nullptr, std::memory_order_relaxed);
   }
 }
 
 const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
 

 
 void Log::Unregister(llvm::StringRef name) {
   auto iter = g_channel_map->find(name);
   assert(iter != g_channel_map->end());
   iter->second.Disable(UINT32_MAX);
   g_channel_map->erase(iter);
 }
 
-bool Log::EnableLogChannel(
-    const std::shared_ptr<llvm::raw_ostream> &log_stream_sp,
+bool Log::EnableLogChannel(const std::shared_ptr<LogHandler> &log_handler_sp,
                            uint32_t log_options, llvm::StringRef channel,
-    llvm::ArrayRef<const char *> categories, llvm::raw_ostream &error_stream) {
+                           llvm::ArrayRef<const char *> categories,
+                           llvm::raw_ostream &error_stream) {
   auto iter = g_channel_map->find(channel);
   if (iter == g_channel_map->end()) {
     error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
     return false;
   }
   uint32_t flags = categories.empty()
                        ? iter->second.m_channel.default_flags
                        : GetFlags(error_stream, *iter, categories);
-  iter->second.Enable(log_stream_sp, log_options, flags);
+  iter->second.Enable(log_handler_sp, log_options, flags);
   return true;
 }
 
 bool Log::DisableLogChannel(llvm::StringRef channel,
                             llvm::ArrayRef<const char *> categories,
                             llvm::raw_ostream &error_stream) {
   auto iter = g_channel_map->find(channel);
   if (iter == g_channel_map->end()) {
if (options.Test(LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION) &&
     function = function.take_front(40);
     OS << llvm::formatv("{0,-60:60} ", (file + ":" + function).str());
   }
 }
 
 void Log::WriteMessage(const std::string &message) {
   // Make a copy of our stream shared pointer in case someone disables our log
   // while we are logging and releases the stream
-  auto stream_sp = GetStream();
-  if (!stream_sp)
+  auto handler_sp = GetHandler();
+  if (!handler_sp)
     return;
 
   Flags options = GetOptions();
-  if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) {
-    static std::recursive_mutex g_LogThreadedMutex;
-    std::lock_guard<std::recursive_mutex> guard(g_LogThreadedMutex);
-    *stream_sp << message;
-    stream_sp->flush();
-  } else {
-    *stream_sp << message;
-    stream_sp->flush();
-  }
+  if (options.Test(LLDB_LOG_OPTION_THREADSAFE))
+    handler_sp->EmitThreadSafe(message);
+  else
+    handler_sp->Emit(message);
 }
 
 void Log::Format(llvm::StringRef file, llvm::StringRef function,
                  const llvm::formatv_object_base &payload) {
   std::string message_string;
   llvm::raw_string_ostream message(message_string);
   WriteHeader(message, file, function);
   message << payload << "\n";
   WriteMessage(message.str());
 }
+
+void LogHandler::EmitThreadSafe(llvm::StringRef message) {
+  std::lock_guard<std::mutex> guard(m_mutex);
+  Emit(message);
+}
+
+StreamLogHandler::StreamLogHandler(int fd, bool should_close, bool unbuffered)
+    : m_stream(fd, should_close, unbuffered) {}
+
+void StreamLogHandler::Emit(llvm::StringRef message) {
+  m_stream << message;
+  m_stream.flush();
+}
+
+std::shared_ptr<StreamLogHandler> StreamLogHandler::Create(int fd,
+                                                           bool should_close) {
+  constexpr const bool unbuffered = true;
+  return std::make_shared<StreamLogHandler>(fd, should_close, unbuffered);
+}
+
+CallbackLogHandler::CallbackLogHandler(lldb::LogOutputCallback callback,
+                                       void *baton)
+    : m_callback(callback), m_baton(baton) {}
+
+void CallbackLogHandler::Emit(llvm::StringRef message) {
+  m_callback(message.data(), m_baton);
+}
+
+std::shared_ptr<CallbackLogHandler>
+CallbackLogHandler::Create(lldb::LogOutputCallback callback, void *baton) {
+  return std::make_shared<CallbackLogHandler>(callback, baton);
+}

lldb/source/Utility/StreamCallback.cpp

-//===-- StreamCallback.cpp ------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Utility/StreamCallback.h"
-
-#include <string>
-
-using namespace lldb_private;
-
-StreamCallback::StreamCallback(lldb::LogOutputCallback callback, void *baton)
-    : llvm::raw_ostream(true), m_callback(callback), m_baton(baton) {}
-
-void StreamCallback::write_impl(const char *Ptr, size_t Size) {
-  m_callback(std::string(Ptr, Size).c_str(), m_baton);
-}
-
-uint64_t StreamCallback::current_pos() const { return 0; }

lldb/tools/lldb-server/LLDBServerUtilities.cpp

 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
 using namespace lldb_private::lldb_server;
+using namespace lldb_private;
 using namespace llvm;
 
-static std::shared_ptr<raw_ostream> GetLogStream(StringRef log_file) {
+class TestLogHandler : public LogHandler {
+public:
+  TestLogHandler(std::shared_ptr<llvm::raw_ostream> stream_sp)
+      : m_stream_sp(stream_sp) {}
+
+  void Emit(llvm::StringRef message) override {
+    (*m_stream_sp) << message;
+    m_stream_sp->flush();
+  }
+
+private:
+  std::shared_ptr<raw_ostream> m_stream_sp;
+};
+
+static std::shared_ptr<TestLogHandler> GetLogStream(StringRef log_file) {
   if (!log_file.empty()) {
     std::error_code EC;
-    std::shared_ptr<raw_ostream> stream_sp = std::make_shared<raw_fd_ostream>(
+    auto stream_sp = std::make_shared<raw_fd_ostream>(
         log_file, EC, sys::fs::OF_TextWithCRLF | sys::fs::OF_Append);
     if (!EC)
-      return stream_sp;
+      return std::make_shared<TestLogHandler>(stream_sp);
     errs() << llvm::formatv(
         "Failed to open log file `{0}`: {1}\nWill log to stderr instead.\n",
         log_file, EC.message());
   }
   // No need to delete the stderr stream.
-  return std::shared_ptr<raw_ostream>(&errs(), [](raw_ostream *) {});
+  return std::make_shared<TestLogHandler>(
+      std::shared_ptr<raw_ostream>(&errs(), [](raw_ostream *) {}));
 }
 
 bool LLDBServerUtilities::SetupLogging(const std::string &log_file,
                                        const StringRef &log_channels,
                                        uint32_t log_options) {
 
   auto log_stream_sp = GetLogStream(log_file);
 

lldb/unittests/Core/CMakeLists.txt

 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
   DiagnosticEventTest.cpp
   DumpDataExtractorTest.cpp
   FormatEntityTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
   RichManglingContextTest.cpp
   SourceLocationSpecTest.cpp
   SourceManagerTest.cpp
-  StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbPluginObjectFileELF
     lldbPluginObjectFileMachO
     lldbPluginObjectFilePECOFF
     lldbPluginPlatformMacOSX
     lldbPluginSymbolFileSymtab
     lldbSymbol
     lldbUtilityHelpers
     LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )

lldb/unittests/Core/StreamCallbackTest.cpp

-//===-- StreamCallbackTest.cpp --------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Utility/StreamCallback.h"
-#include "gtest/gtest.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-static char test_baton;
-static size_t callback_count = 0;
-static void TestCallback(const char *data, void *baton) {
-  EXPECT_STREQ("Foobar", data);
-  EXPECT_EQ(&test_baton, baton);
-  ++callback_count;
-}
-
-TEST(StreamCallbackTest, Callback) {
-  StreamCallback stream(TestCallback, &test_baton);
-  stream << "Foobar";
-  EXPECT_EQ(1u, callback_count);
-}

lldb/unittests/Utility/LogTest.cpp

 
 static Log::Channel test_channel(test_categories, TestChannel::FOO);
 
 namespace lldb_private {
 template <> Log::Channel &LogChannelFor<TestChannel>() { return test_channel; }
 } // namespace lldb_private
 
 // Wrap enable, disable and list functions to make them easier to test.
-static bool EnableChannel(std::shared_ptr<llvm::raw_ostream> stream_sp,
+static bool EnableChannel(std::shared_ptr<LogHandler> log_handler_sp,
                           uint32_t log_options, llvm::StringRef channel,
                           llvm::ArrayRef<const char *> categories,
                           std::string &error) {
   error.clear();
   llvm::raw_string_ostream error_stream(error);
-  return Log::EnableLogChannel(stream_sp, log_options, channel, categories,
+  return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
                                error_stream);
 }
 
 static bool DisableChannel(llvm::StringRef channel,
                            llvm::ArrayRef<const char *> categories,
                            std::string &error) {
   error.clear();
   llvm::raw_string_ostream error_stream(error);
   return Log::DisableLogChannel(channel, categories, error_stream);
 }
 
 static bool ListCategories(llvm::StringRef channel, std::string &result) {
   result.clear();
   llvm::raw_string_ostream result_stream(result);
   return Log::ListChannelCategories(channel, result_stream);
 }
 
 namespace {
 // A test fixture which provides tests with a pre-registered channel.
 struct LogChannelTest : public ::testing::Test {
   void TearDown() override { Log::DisableAllLogChannels(); }
 
-  static void SetUpTestCase() {
-    Log::Register("chan", test_channel);
-  }
+  static void SetUpTestCase() { Log::Register("chan", test_channel); }
 
   static void TearDownTestCase() {
     Log::Unregister("chan");
     llvm::llvm_shutdown();
   }
 };
 
+class TestLogHandler : public LogHandler {
+public:
+  TestLogHandler() : m_messages(), m_stream(m_messages) {}
+
+  void Emit(llvm::StringRef message) override { m_stream << message; }
+
+  llvm::SmallString<0> m_messages;
+  llvm::raw_svector_ostream m_stream;
+};
+
 // A test fixture which provides tests with a pre-registered and pre-enabled
 // channel. Additionally, the messages written to that channel are captured and
 // made available via getMessage().
 class LogChannelEnabledTest : public LogChannelTest {
-  llvm::SmallString<0> m_messages;
-  std::shared_ptr<llvm::raw_svector_ostream> m_stream_sp =
-      std::make_shared<llvm::raw_svector_ostream>(m_messages);
+  std::shared_ptr<TestLogHandler> m_log_handler_sp =
+      std::make_shared<TestLogHandler>();
   Log *m_log;
   size_t m_consumed_bytes = 0;
 
 protected:
-  std::shared_ptr<llvm::raw_ostream> getStream() { return m_stream_sp; }
+  std::shared_ptr<LogHandler> getLogHandler() { return m_log_handler_sp; }
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
 
 public:
   void SetUp() override;
 };
 } // end anonymous namespace
 
 void LogChannelEnabledTest::SetUp() {
   LogChannelTest::SetUp();
 
   std::string error;
-  ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
+  ASSERT_TRUE(EnableChannel(m_log_handler_sp, 0, "chan", {}, error));
 
   m_log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, m_log);
 }
 
 llvm::StringRef LogChannelEnabledTest::takeOutput() {
-  llvm::StringRef result = m_stream_sp->str().drop_front(m_consumed_bytes);
-  m_consumed_bytes+= result.size();
+  llvm::StringRef result =
+      m_log_handler_sp->m_stream.str().drop_front(m_consumed_bytes);
+  m_consumed_bytes += result.size();
   return result;
 }
 
-llvm::StringRef LogChannelEnabledTest::logAndTakeOutput(llvm::StringRef Message) {
+llvm::StringRef
+LogChannelEnabledTest::logAndTakeOutput(llvm::StringRef Message) {
   LLDB_LOG(m_log, "{0}", Message);
   return takeOutput();
 }
 
 TEST(LogTest, LLDB_LOG_nullptr) {
   Log *log = nullptr;
   LLDB_LOG(log, "{0}", 0); // Shouldn't crash
 }
 
 TEST(LogTest, Register) {
   llvm::llvm_shutdown_obj obj;
   Log::Register("chan", test_channel);
   Log::Unregister("chan");
   Log::Register("chan", test_channel);
   Log::Unregister("chan");
 }
 
 TEST(LogTest, Unregister) {
   llvm::llvm_shutdown_obj obj;
   Log::Register("chan", test_channel);
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr<llvm::raw_string_ostream> stream_sp(
-      new llvm::raw_string_ostream(message));
-  EXPECT_TRUE(Log::EnableLogChannel(stream_sp, 0, "chan", {"foo"}, llvm::nulls()));
+  auto log_handler_sp = std::make_shared<TestLogHandler>();
+  EXPECT_TRUE(
+      Log::EnableLogChannel(log_handler_sp, 0, "chan", {"foo"}, llvm::nulls()));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   Log::Unregister("chan");
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
 }
 
+namespace {
+static char test_baton;
+static size_t callback_count = 0;
+static void TestCallback(const char *data, void *baton) {
+  EXPECT_STREQ("Foobar", data);
+  EXPECT_EQ(&test_baton, baton);
+  ++callback_count;
+}
+} // namespace
+
+TEST(LogTest, CallbackLogHandler) {
+  CallbackLogHandler handler(TestCallback, &test_baton);
+  handler.Emit("Foobar");
+  EXPECT_EQ(1u, callback_count);
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr<llvm::raw_string_ostream> stream_sp(
-      new llvm::raw_string_ostream(message));
+  auto log_handler_sp = std::make_shared<TestLogHandler>();
   std::string error;
-  ASSERT_FALSE(EnableChannel(stream_sp, 0, "chanchan", {}, error));
+  ASSERT_FALSE(EnableChannel(log_handler_sp, 0, "chanchan", {}, error));
   EXPECT_EQ("Invalid log channel 'chanchan'.\n", error);
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_EQ(nullptr, GetLog(TestChannel::BAR));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO | TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"bar"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"bar"}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_NE(nullptr, GetLog(TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"baz"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"baz"}, error));
   EXPECT_NE(std::string::npos, error.find("unrecognized log category 'baz'"))
       << "error: " << error;
 }
 
 TEST_F(LogChannelTest, EnableOptions) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr<llvm::raw_string_ostream> stream_sp(
-      new llvm::raw_string_ostream(message));
+  auto log_handler_sp = std::make_shared<TestLogHandler>();
   std::string error;
-  EXPECT_TRUE(
-      EnableChannel(stream_sp, LLDB_LOG_OPTION_VERBOSE, "chan", {}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, LLDB_LOG_OPTION_VERBOSE, "chan", {},
+                            error));
 
   Log *log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, log);
   EXPECT_TRUE(log->GetVerbose());
 }
 
 TEST_F(LogChannelTest, Disable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr<llvm::raw_string_ostream> stream_sp(
-      new llvm::raw_string_ostream(message));
+  auto log_handler_sp = std::make_shared<TestLogHandler>();
   std::string error;
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"foo", "bar"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"foo", "bar"}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_NE(nullptr, GetLog(TestChannel::BAR));
 
   EXPECT_TRUE(DisableChannel("chan", {"bar"}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_EQ(nullptr, GetLog(TestChannel::BAR));
 
   EXPECT_TRUE(DisableChannel("chan", {"baz"}, error));
)";
 
   EXPECT_FALSE(ListCategories("chanchan", list));
   EXPECT_EQ("Invalid log channel 'chanchan'.\n", list);
 }
 
 TEST_F(LogChannelEnabledTest, log_options) {
   std::string Err;
   EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
-  EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_THREADSAFE, "chan", {},
-                            Err));
+  EXPECT_TRUE(EnableChannel(getLogHandler(), LLDB_LOG_OPTION_THREADSAFE, "chan",
+                            {}, Err));
   EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
 
   {
-    EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_SEQUENCE,
+    EXPECT_TRUE(EnableChannel(getLogHandler(), LLDB_LOG_OPTION_PREPEND_SEQUENCE,
                               "chan", {}, Err));
     llvm::StringRef Msg = logAndTakeOutput("Hello World");
     int seq_no;
     EXPECT_EQ(1, sscanf(Msg.str().c_str(), "%d Hello World", &seq_no));
   }
 
   {
-    EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION,
-                              "chan", {}, Err));
+    EXPECT_TRUE(EnableChannel(getLogHandler(),
+                              LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "chan", {},
+                              Err));
     llvm::StringRef Msg = logAndTakeOutput("Hello World");
     char File[12];
     char Function[17];
-      
-    sscanf(Msg.str().c_str(), "%[^:]:%s                                 Hello World", File, Function);
+    sscanf(Msg.str().c_str(),
+           "%[^:]:%s                                 Hello World", File,
+           Function);
     EXPECT_STRCASEEQ("LogTest.cpp", File);
     EXPECT_STREQ("logAndTakeOutput", Function);
   }
 
-  EXPECT_TRUE(EnableChannel(
-      getStream(), LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD, "chan", {}, Err));
+  EXPECT_TRUE(EnableChannel(getLogHandler(),
+                            LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD, "chan", {},
+                            Err));
   EXPECT_EQ(llvm::formatv("[{0,0+4}/{1,0+4}] Hello World\n", ::getpid(),
                           llvm::get_threadid())
                 .str(),
             logAndTakeOutput("Hello World"));
 }
 
 TEST_F(LogChannelEnabledTest, LLDB_LOG_ERROR) {
   LLDB_LOG_ERROR(getLog(), llvm::Error::success(), "Foo failed: {0}");
TEST_F(LogChannelEnabledTest, LogVerboseThread) {
   // Test that we are able to concurrently check the verbose flag of a log
   // channel and enable it.
   std::string err;
 
   // Start logging on one thread. Concurrently, try enabling the log channel
   // (with different log options).
   std::thread log_thread([this] { LLDB_LOGV(getLog(), "Hello World"); });
   EXPECT_TRUE(
-      EnableChannel(getStream(), LLDB_LOG_OPTION_VERBOSE, "chan", {}, err));
+      EnableChannel(getLogHandler(), LLDB_LOG_OPTION_VERBOSE, "chan", {}, err));
   log_thread.join();
 
   // The log thread either managed to write to the log, or it didn't. In either
   // case, we should not trip any undefined behavior (run the test under TSAN to
   // verify this).
   EXPECT_THAT(takeOutput(), testing::AnyOf("", "Hello World\n"));
 }