[Lldb-commits] [lldb] r323753 - Add LLDB_LOG_ERROR macro

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jan 30 04:19:34 2018
New Revision: 323753

URL: http://llvm.org/viewvc/llvm-project?rev=323753&view=rev
Log:
Add LLDB_LOG_ERROR macro

Summary:
The difference between this and regular LLDB_LOG is that this one clears
the error object unconditionally.  This was inspired by the
ObjectFileELF bug (r322664), where the error object was being cleared
only if logging was enabled.

Reviewers: davide, zturner, jingham, clayborg

Subscribers: lldb-commits, emaste

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

Modified:
    lldb/trunk/include/lldb/Utility/Log.h
    lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
    lldb/trunk/unittests/Utility/LogTest.cpp

Modified: lldb/trunk/include/lldb/Utility/Log.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Log.h?rev=323753&r1=323752&r2=323753&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/Log.h (original)
+++ lldb/trunk/include/lldb/Utility/Log.h Tue Jan 30 04:19:34 2018
@@ -17,6 +17,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h" // for StringMap
 #include "llvm/ADT/StringRef.h" // for StringRef, StringLiteral
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/RWMutex.h"
@@ -140,6 +141,15 @@ public:
     Format(file, function, llvm::formatv(format, std::forward<Args>(args)...));
   }
 
+  template <typename... Args>
+  void FormatError(llvm::Error error, llvm::StringRef file,
+                   llvm::StringRef function, const char *format,
+                   Args &&... args) {
+    Format(file, function,
+           llvm::formatv(format, llvm::toString(std::move(error)),
+                         std::forward<Args>(args)...));
+  }
+
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
   void VAPrintf(const char *format, va_list args);
@@ -218,4 +228,17 @@ private:
       log_private->Format(__FILE__, __func__, __VA_ARGS__);                    \
   } while (0)
 
+// Write message to log, if error is set. In the log message refer to the error
+// with {0}. Error is cleared regardless of whether logging is enabled.
+#define LLDB_LOG_ERROR(log, error, ...)                                        \
+  do {                                                                         \
+    ::lldb_private::Log *log_private = (log);                                  \
+    ::llvm::Error error_private = (error);                                     \
+    if (log_private && error_private) {                                        \
+      log_private->FormatError(::std::move(error_private), __FILE__, __func__, \
+                               __VA_ARGS__);                                   \
+    } else                                                                     \
+      ::llvm::consumeError(::std::move(error_private));                        \
+  } while (0)
+
 #endif // LLDB_UTILITY_LOG_H

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=323753&r1=323752&r2=323753&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Tue Jan 30 04:19:34 2018
@@ -3491,9 +3491,9 @@ size_t ObjectFileELF::ReadSectionData(Se
        size_t(section_data.GetByteSize())},
       GetByteOrder() == eByteOrderLittle, GetAddressByteSize() == 8);
   if (!Decompressor) {
-    LLDB_LOG(log, "Unable to initialize decompressor for section {0}: {1}",
-             section->GetName(), llvm::toString(Decompressor.takeError()));
-    consumeError(Decompressor.takeError());
+    LLDB_LOG_ERROR(log, Decompressor.takeError(),
+                   "Unable to initialize decompressor for section {0}",
+                   section->GetName());
     return result;
   }
   auto buffer_sp =
@@ -3501,9 +3501,8 @@ size_t ObjectFileELF::ReadSectionData(Se
   if (auto Error = Decompressor->decompress(
           {reinterpret_cast<char *>(buffer_sp->GetBytes()),
            size_t(buffer_sp->GetByteSize())})) {
-    LLDB_LOG(log, "Decompression of section {0} failed: {1}",
-             section->GetName(), llvm::toString(std::move(Error)));
-    consumeError(std::move(Error));
+    LLDB_LOG_ERROR(log, std::move(Error), "Decompression of section {0} failed",
+                   section->GetName());
     return result;
   }
   section_data.SetData(buffer_sp);

Modified: lldb/trunk/unittests/Utility/LogTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/LogTest.cpp?rev=323753&r1=323752&r2=323753&view=diff
==============================================================================
--- lldb/trunk/unittests/Utility/LogTest.cpp (original)
+++ lldb/trunk/unittests/Utility/LogTest.cpp Tue Jan 30 04:19:34 2018
@@ -240,6 +240,23 @@ TEST_F(LogChannelEnabledTest, log_option
             logAndTakeOutput("Hello World"));
 }
 
+TEST_F(LogChannelEnabledTest, LLDB_LOG_ERROR) {
+  LLDB_LOG_ERROR(getLog(), llvm::Error::success(), "Foo failed: {0}");
+  ASSERT_EQ("", takeOutput());
+
+  LLDB_LOG_ERROR(getLog(),
+                 llvm::make_error<llvm::StringError>(
+                     "My Error", llvm::inconvertibleErrorCode()),
+                 "Foo failed: {0}");
+  ASSERT_EQ("Foo failed: My Error\n", takeOutput());
+
+  // Doesn't log, but doesn't assert either
+  LLDB_LOG_ERROR(nullptr,
+                 llvm::make_error<llvm::StringError>(
+                     "My Error", llvm::inconvertibleErrorCode()),
+                 "Foo failed: {0}");
+}
+
 TEST_F(LogChannelEnabledTest, LogThread) {
   // Test that we are able to concurrently write to a log channel and disable
   // it.