[Lldb-commits] [PATCH] Add logging plugin for Winodows

[Adrian McCarthy]

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.cpp:41
@@ +40,3 @@
+    static ConstString g_name("windows");
+    static std::once_flag g_once_flag;
+
----------------
zturner wrote:
> Can you make this an llvm::ManagedStatic<std::once_flag> once_flag on Windows is not linker-initialized, and Windows doesn't have thread-safe function local static initialization.  This is probably benign in this case, as this is only called from SystemInitialzierCommon::Initialize(), but either way, as a general principle once_flags should be ManagedStatics.
Done, but I thought VC++2013 had finally added thread-safe initialization of function statics.  Maybe I'm misreading the table:  http://blogs.msdn.com/b/vcblog/archive/2013/11/18/announcing-the-visual-c-compiler-november-2013-ctp.aspx

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.h:18
@@ +17,3 @@
+#define WINDOWS_LOG_THREAD                   (1u << 2)
+#define WINDOWS_LOG_PACKETS                  (1u << 3)
+#define WINDOWS_LOG_MEMORY                   (1u << 4)    // Log memory reads/writes calls
----------------
zturner wrote:
> This flag doesn't make much sense currently, I think this must have to do with remote ddebugging.
Done.  I wasn't sure if it was important to keep the flag numbering consistent.

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.h:23
@@ +22,3 @@
+#define WINDOWS_LOG_BREAKPOINTS              (1u << 7)
+#define WINDOWS_LOG_WATCHPOINTS              (1u << 8)
+#define WINDOWS_LOG_STEP                     (1u << 9)
----------------
zturner wrote:
> Can delete this one too, as Windows doesn't support watchpoints yet.
Done.

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.h:25
@@ +24,3 @@
+#define WINDOWS_LOG_STEP                     (1u << 9)
+#define WINDOWS_LOG_COMM                     (1u << 10)
+#define WINDOWS_LOG_ASYNC                    (1u << 11)
----------------
zturner wrote:
> Not sure what this one is, but check what they're using it for on the Linux side, and if it doesn't apply to Windows (yet / ever) then remove this one too
It's for "communication," but I'm not sure what that means in this context.  Removed.

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.h:27
@@ +26,3 @@
+#define WINDOWS_LOG_ASYNC                    (1u << 11)
+#define WINDOWS_LOG_PTRACE                   (1u << 12)
+#define WINDOWS_LOG_REGISTERS                (1u << 13)
----------------
zturner wrote:
> Another one that can go.
Done.

================
Comment at: source/Plugins/Process/Windows/ProcessWindowsLog.h:75-95
@@ +74,23 @@
+
+    // The following functions can be used to enable the client to limit
+    // logging to only the top level function calls.  This is useful for
+    // recursive functions.  FIXME: not thread safe!
+    static bool
+    AtTopNestLevel()
+    {
+        return m_nestinglevel == 1;
+    }
+
+    static void
+    IncNestLevel()
+    {
+        ++m_nestinglevel;
+    }
+
+    static void
+    DecNestLevel()
+    {
+        --m_nestinglevel;
+        assert(m_nestinglevel >= 0);
+    }
+};
----------------
zturner wrote:
> I don't think we need any of this nesting stuff.  If it becomes useful later, we can always introduce it back.
Removed.

http://reviews.llvm.org/D8937

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/