[lldb] Make logging machinery type-safe

This patch makes use of c++ type checking and scoped enums to make
logging statements shorter and harder to misuse.

Defines like LIBLLDB_LOG_PROCESS are replaces with LLDBLog::Process.
Because it now carries type information we do not need to worry about
matching a specific enum value with the right getter function -- the
compiler will now do that for us.

The main entry point for the logging machinery becomes the GetLog
(template) function, which will obtain the correct Log object based on
the enum type. It achieves this through another template function
(LogChannelFor<T>), which must be specialized for each type, and should
return the appropriate channel object.

This patch also removes the ability to log a message if multiple
categories are enabled simultaneously as it was unused and confusing.

This patch does not actually remove any of the existing interfaces. The
defines and log retrieval functions are left around as wrappers around
the new interfaces. They will be removed in follow-up patch.

Differential Revision: https://reviews.llvm.org/D117490

lldb/include/lldb/Interpreter/ScriptedInterface.h

@@ -33,7 +33,7 @@ class ScriptedInterface {
   template <typename Ret>
   static Ret ErrorWithMessage(llvm::StringRef caller_name,
                               llvm::StringRef error_msg, Status &error,
-                              uint32_t log_caterogy = LIBLLDB_LOG_PROCESS) {
+                              LLDBLog log_caterogy = LLDBLog::Process) {
     LLDB_LOGF(GetLogIfAllCategoriesSet(log_caterogy), "%s ERROR = %s",
               caller_name.data(), error_msg.data());
     error.SetErrorString(llvm::Twine(caller_name + llvm::Twine(" ERROR = ") +

lldb/include/lldb/Utility/Log.h

@@ -10,7 +10,6 @@
 #define LLDB_UTILITY_LOG_H
 
 #include "lldb/Utility/Flags.h"
-#include "lldb/Utility/Logging.h"
 #include "lldb/lldb-defines.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -48,11 +47,31 @@ namespace lldb_private {
 
 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),
+  /// };
+  using MaskType = uint64_t;
+
+  template <MaskType Bit>
+  static constexpr MaskType ChannelFlag = MaskType(1) << Bit;
+
   // Description of a log channel category.
   struct Category {
     llvm::StringLiteral name;
     llvm::StringLiteral description;
-    uint32_t flag;
+    MaskType flag;
+
+    template <typename Cat>
+    constexpr Category(llvm::StringLiteral name,
+                       llvm::StringLiteral description, Cat mask)
+        : name(name), description(description), flag(MaskType(mask)) {
+      static_assert(
+          std::is_same<Log::MaskType, std::underlying_type_t<Cat>>::value, "");
+    }
   };
 
   // This class describes a log channel. It also encapsulates the behavior
@@ -63,18 +82,22 @@ class Log final {
 
   public:
     const llvm::ArrayRef<Category> categories;
-    const uint32_t default_flags;
+    const MaskType default_flags;
 
+    template <typename Cat>
     constexpr Channel(llvm::ArrayRef<Log::Category> categories,
-                      uint32_t default_flags)
+                      Cat default_flags)
         : log_ptr(nullptr), categories(categories),
-          default_flags(default_flags) {}
+          default_flags(MaskType(default_flags)) {
+      static_assert(
+          std::is_same<Log::MaskType, std::underlying_type_t<Cat>>::value, "");
+    }
 
     // This function is safe to call at any time. If the channel is disabled
     // after (or concurrently with) this function returning a non-null Log
     // pointer, it is still safe to attempt to write to the Log object -- the
     // output will be discarded.
-    Log *GetLogIfAll(uint32_t mask) {
+    Log *GetLogIfAll(MaskType mask) {
       Log *log = log_ptr.load(std::memory_order_relaxed);
       if (log && log->GetMask().AllSet(mask))
         return log;
@@ -85,7 +108,7 @@ class Log final {
     // after (or concurrently with) this function returning a non-null Log
     // pointer, it is still safe to attempt to write to the Log object -- the
     // output will be discarded.
-    Log *GetLogIfAny(uint32_t mask) {
+    Log *GetLogIfAny(MaskType mask) {
       Log *log = log_ptr.load(std::memory_order_relaxed);
       if (log && log->GetMask().AnySet(mask))
         return log;
@@ -180,7 +203,7 @@ class Log final {
 
   std::shared_ptr<llvm::raw_ostream> m_stream_sp;
   std::atomic<uint32_t> m_options{0};
-  std::atomic<uint32_t> m_mask{0};
+  std::atomic<MaskType> m_mask{0};
 
   void WriteHeader(llvm::raw_ostream &OS, llvm::StringRef file,
                    llvm::StringRef function);
@@ -215,6 +238,19 @@ class Log final {
   void operator=(const Log &) = delete;
 };
 
+// Must be specialized for a particular log type.
+template <typename Cat> Log::Channel &LogChannelFor() = delete;
+
+/// Retrieve the Log object for the channel associated with the given log enum.
+///
+/// Returns a valid Log object if any of the provided categories are enabled.
+/// Otherwise, returns nullptr.
+template <typename Cat> Log *GetLog(Cat mask) {
+  static_assert(std::is_same<Log::MaskType, std::underlying_type_t<Cat>>::value,
+                "");
+  return LogChannelFor<Cat>().GetLogIfAny(Log::MaskType(mask));
+}
+
 } // namespace lldb_private
 
 /// The LLDB_LOG* macros defined below are the way to emit log messages.
@@ -272,3 +308,6 @@ class Log final {
   } while (0)
 
 #endif // LLDB_UTILITY_LOG_H
+
+// TODO: Remove this and fix includes everywhere.
+#include "lldb/Utility/Logging.h"

lldb/include/lldb/Utility/Logging.h

@@ -9,57 +9,89 @@
 #ifndef LLDB_UTILITY_LOGGING_H
 #define LLDB_UTILITY_LOGGING_H
 
+#include "lldb/Utility/Log.h"
+#include "llvm/ADT/BitmaskEnum.h"
 #include <cstdint>
 
-// Log Bits specific to logging in lldb
-#define LIBLLDB_LOG_PROCESS (1u << 1)
-#define LIBLLDB_LOG_THREAD (1u << 2)
-#define LIBLLDB_LOG_DYNAMIC_LOADER (1u << 3)
-#define LIBLLDB_LOG_EVENTS (1u << 4)
-#define LIBLLDB_LOG_BREAKPOINTS (1u << 5)
-#define LIBLLDB_LOG_WATCHPOINTS (1u << 6)
-#define LIBLLDB_LOG_STEP (1u << 7)
-#define LIBLLDB_LOG_EXPRESSIONS (1u << 8)
-#define LIBLLDB_LOG_TEMPORARY (1u << 9)
-#define LIBLLDB_LOG_STATE (1u << 10)
-#define LIBLLDB_LOG_OBJECT (1u << 11)
-#define LIBLLDB_LOG_COMMUNICATION (1u << 12)
-#define LIBLLDB_LOG_CONNECTION (1u << 13)
-#define LIBLLDB_LOG_HOST (1u << 14)
-#define LIBLLDB_LOG_UNWIND (1u << 15)
-#define LIBLLDB_LOG_API (1u << 16)
-#define LIBLLDB_LOG_SCRIPT (1u << 17)
-#define LIBLLDB_LOG_COMMANDS (1U << 18)
-#define LIBLLDB_LOG_TYPES (1u << 19)
-#define LIBLLDB_LOG_SYMBOLS (1u << 20)
-#define LIBLLDB_LOG_MODULES (1u << 21)
-#define LIBLLDB_LOG_TARGET (1u << 22)
-#define LIBLLDB_LOG_MMAP (1u << 23)
-#define LIBLLDB_LOG_OS (1u << 24)
-#define LIBLLDB_LOG_PLATFORM (1u << 25)
-#define LIBLLDB_LOG_SYSTEM_RUNTIME (1u << 26)
-#define LIBLLDB_LOG_JIT_LOADER (1u << 27)
-#define LIBLLDB_LOG_LANGUAGE (1u << 28)
-#define LIBLLDB_LOG_DATAFORMATTERS (1u << 29)
-#define LIBLLDB_LOG_DEMANGLE (1u << 30)
-#define LIBLLDB_LOG_AST (1u << 31)
-#define LIBLLDB_LOG_ALL (UINT32_MAX)
-#define LIBLLDB_LOG_DEFAULT                                                    \
-  (LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD | LIBLLDB_LOG_DYNAMIC_LOADER |     \
-   LIBLLDB_LOG_BREAKPOINTS | LIBLLDB_LOG_WATCHPOINTS | LIBLLDB_LOG_STEP |      \
-   LIBLLDB_LOG_STATE | LIBLLDB_LOG_SYMBOLS | LIBLLDB_LOG_TARGET |              \
-   LIBLLDB_LOG_COMMANDS)
-
 namespace lldb_private {
 
-class Log;
+enum class LLDBLog : Log::MaskType {
+  API = Log::ChannelFlag<0>,
+  AST = Log::ChannelFlag<1>,
+  Breakpoints = Log::ChannelFlag<2>,
+  Commands = Log::ChannelFlag<3>,
+  Communication = Log::ChannelFlag<4>,
+  Connection = Log::ChannelFlag<5>,
+  DataFormatters = Log::ChannelFlag<6>,
+  Demangle = Log::ChannelFlag<7>,
+  DynamicLoader = Log::ChannelFlag<8>,
+  Events = Log::ChannelFlag<9>,
+  Expressions = Log::ChannelFlag<10>,
+  Host = Log::ChannelFlag<11>,
+  JITLoader = Log::ChannelFlag<12>,
+  Language = Log::ChannelFlag<13>,
+  MMap = Log::ChannelFlag<14>,
+  Modules = Log::ChannelFlag<15>,
+  Object = Log::ChannelFlag<16>,
+  OS = Log::ChannelFlag<17>,
+  Platform = Log::ChannelFlag<18>,
+  Process = Log::ChannelFlag<19>,
+  Script = Log::ChannelFlag<20>,
+  State = Log::ChannelFlag<21>,
+  Step = Log::ChannelFlag<22>,
+  Symbols = Log::ChannelFlag<23>,
+  SystemRuntime = Log::ChannelFlag<24>,
+  Target = Log::ChannelFlag<25>,
+  Temporary = Log::ChannelFlag<26>,
+  Thread = Log::ChannelFlag<27>,
+  Types = Log::ChannelFlag<28>,
+  Unwind = Log::ChannelFlag<29>,
+  Watchpoints = Log::ChannelFlag<30>,
+  LLVM_MARK_AS_BITMASK_ENUM(Watchpoints),
+};
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+// Log Bits specific to logging in lldb
+#define LIBLLDB_LOG_PROCESS ::lldb_private::LLDBLog::Process
+#define LIBLLDB_LOG_THREAD ::lldb_private::LLDBLog::Thread
+#define LIBLLDB_LOG_DYNAMIC_LOADER ::lldb_private::LLDBLog::DynamicLoader
+#define LIBLLDB_LOG_EVENTS ::lldb_private::LLDBLog::Events
+#define LIBLLDB_LOG_BREAKPOINTS ::lldb_private::LLDBLog::Breakpoints
+#define LIBLLDB_LOG_WATCHPOINTS ::lldb_private::LLDBLog::Watchpoints
+#define LIBLLDB_LOG_STEP ::lldb_private::LLDBLog::Step
+#define LIBLLDB_LOG_EXPRESSIONS ::lldb_private::LLDBLog::Expressions
+#define LIBLLDB_LOG_TEMPORARY ::lldb_private::LLDBLog::Temporary
+#define LIBLLDB_LOG_STATE ::lldb_private::LLDBLog::State
+#define LIBLLDB_LOG_OBJECT ::lldb_private::LLDBLog::Object
+#define LIBLLDB_LOG_COMMUNICATION ::lldb_private::LLDBLog::Communication
+#define LIBLLDB_LOG_CONNECTION ::lldb_private::LLDBLog::Connection
+#define LIBLLDB_LOG_HOST ::lldb_private::LLDBLog::Host
+#define LIBLLDB_LOG_UNWIND ::lldb_private::LLDBLog::Unwind
+#define LIBLLDB_LOG_API ::lldb_private::LLDBLog::API
+#define LIBLLDB_LOG_SCRIPT ::lldb_private::LLDBLog::Script
+#define LIBLLDB_LOG_COMMANDS ::lldb_private::LLDBLog::Commands
+#define LIBLLDB_LOG_TYPES ::lldb_private::LLDBLog::Types
+#define LIBLLDB_LOG_SYMBOLS ::lldb_private::LLDBLog::Symbols
+#define LIBLLDB_LOG_MODULES ::lldb_private::LLDBLog::Modules
+#define LIBLLDB_LOG_TARGET ::lldb_private::LLDBLog::Target
+#define LIBLLDB_LOG_MMAP ::lldb_private::LLDBLog::MMap
+#define LIBLLDB_LOG_OS ::lldb_private::LLDBLog::OS
+#define LIBLLDB_LOG_PLATFORM ::lldb_private::LLDBLog::Platform
+#define LIBLLDB_LOG_SYSTEM_RUNTIME ::lldb_private::LLDBLog::SystemRuntime
+#define LIBLLDB_LOG_JIT_LOADER ::lldb_private::LLDBLog::JITLoader
+#define LIBLLDB_LOG_LANGUAGE ::lldb_private::LLDBLog::Language
+#define LIBLLDB_LOG_DATAFORMATTERS ::lldb_private::LLDBLog::DataFormatters
+#define LIBLLDB_LOG_DEMANGLE ::lldb_private::LLDBLog::Demangle
+#define LIBLLDB_LOG_AST ::lldb_private::LLDBLog::AST
 
-Log *GetLogIfAllCategoriesSet(uint32_t mask);
+Log *GetLogIfAllCategoriesSet(LLDBLog mask);
 
-Log *GetLogIfAnyCategoriesSet(uint32_t mask);
+Log *GetLogIfAnyCategoriesSet(LLDBLog mask);
 
 void InitializeLldbChannel();
 
+template <> Log::Channel &LogChannelFor<LLDBLog>();
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_LOGGING_H

lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp

@@ -13,16 +13,20 @@
 using namespace lldb_private;
 
 static constexpr Log::Category g_categories[] = {
-  {{"break"}, {"log breakpoints"}, POSIX_LOG_BREAKPOINTS},
-  {{"memory"}, {"log memory reads and writes"}, POSIX_LOG_MEMORY},
-  {{"process"}, {"log process events and activities"}, POSIX_LOG_PROCESS},
-  {{"ptrace"}, {"log all calls to ptrace"}, POSIX_LOG_PTRACE},
-  {{"registers"}, {"log register read/writes"}, POSIX_LOG_REGISTERS},
-  {{"thread"}, {"log thread events and activities"}, POSIX_LOG_THREAD},
-  {{"watch"}, {"log watchpoint related activities"}, POSIX_LOG_WATCHPOINTS},
+    {{"break"}, {"log breakpoints"}, POSIXLog::Breakpoints},
+    {{"memory"}, {"log memory reads and writes"}, POSIXLog::Memory},
+    {{"process"}, {"log process events and activities"}, POSIXLog::Process},
+    {{"ptrace"}, {"log all calls to ptrace"}, POSIXLog::Ptrace},
+    {{"registers"}, {"log register read/writes"}, POSIXLog::Registers},
+    {{"thread"}, {"log thread events and activities"}, POSIXLog::Thread},
+    {{"watch"}, {"log watchpoint related activities"}, POSIXLog::Watchpoints},
 };
 
-Log::Channel ProcessPOSIXLog::g_channel(g_categories, POSIX_LOG_DEFAULT);
+static Log::Channel g_channel(g_categories, POSIXLog::Process);
+
+template <> Log::Channel &lldb_private::LogChannelFor<POSIXLog>() {
+  return g_channel;
+}
 
 void ProcessPOSIXLog::Initialize() {
   static llvm::once_flag g_once_flag;

lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.h

@@ -13,27 +13,35 @@
 
 #include "lldb/Utility/Log.h"
 
-#define POSIX_LOG_PROCESS (1u << 1)
-#define POSIX_LOG_THREAD (1u << 2)
-#define POSIX_LOG_MEMORY (1u << 4) // Log memory reads/writes calls
-#define POSIX_LOG_PTRACE (1u << 5)
-#define POSIX_LOG_REGISTERS (1u << 6)
-#define POSIX_LOG_BREAKPOINTS (1u << 7)
-#define POSIX_LOG_WATCHPOINTS (1u << 8)
-#define POSIX_LOG_ALL (UINT32_MAX)
-#define POSIX_LOG_DEFAULT POSIX_LOG_PROCESS
-
 namespace lldb_private {
-class ProcessPOSIXLog {
-  static Log::Channel g_channel;
 
+enum class POSIXLog : Log::MaskType {
+  Breakpoints = Log::ChannelFlag<0>,
+  Memory = Log::ChannelFlag<1>,
+  Process = Log::ChannelFlag<2>,
+  Ptrace = Log::ChannelFlag<3>,
+  Registers = Log::ChannelFlag<4>,
+  Thread = Log::ChannelFlag<5>,
+  Watchpoints = Log::ChannelFlag<6>,
+  LLVM_MARK_AS_BITMASK_ENUM(Watchpoints)
+};
+
+#define POSIX_LOG_PROCESS ::lldb_private::POSIXLog::Process
+#define POSIX_LOG_THREAD ::lldb_private::POSIXLog::Thread
+#define POSIX_LOG_MEMORY ::lldb_private::POSIXLog::Memory
+#define POSIX_LOG_PTRACE ::lldb_private::POSIXLog::Ptrace
+#define POSIX_LOG_REGISTERS ::lldb_private::POSIXLog::Registers
+#define POSIX_LOG_BREAKPOINTS ::lldb_private::POSIXLog::Breakpoints
+#define POSIX_LOG_WATCHPOINTS ::lldb_private::POSIXLog::Watchpoints
+
+class ProcessPOSIXLog {
 public:
   static void Initialize();
 
-  static Log *GetLogIfAllCategoriesSet(uint32_t mask) {
-    return g_channel.GetLogIfAll(mask);
-  }
+  static Log *GetLogIfAllCategoriesSet(POSIXLog mask) { return GetLog(mask); }
 };
-}
+
+template <> Log::Channel &LogChannelFor<POSIXLog>();
+} // namespace lldb_private
 
 #endif // liblldb_ProcessPOSIXLog_h_

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

@@ -1086,7 +1086,7 @@ void GDBRemoteCommunicationServerLLGS::NewSubprocess(
 }
 
 void GDBRemoteCommunicationServerLLGS::DataAvailableCallback() {
-  Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_COMM));
+  Log *log = GetLog(GDBRLog::Comm);
 
   bool interrupt = false;
   bool done = false;

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -529,7 +529,7 @@ Status ProcessGDBRemote::WillAttachToProcessWithName(const char *process_name,
 }
 
 Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
-  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+  Log *log = GetLog(GDBRLog::Process);
 
   Status error(WillLaunchOrAttach());
   if (error.Fail())
@@ -606,8 +606,7 @@ Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
                 ReadModuleFromMemory(FileSpec(namebuf), standalone_value);
           }
 
-          Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(
-              LIBLLDB_LOG_DYNAMIC_LOADER));
+          Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
           if (module_sp.get()) {
             target.GetImages().AppendIfNeeded(module_sp, false);