From c561a6a920dbf0a53203fafcc6e0c470c840c41a Mon Sep 17 00:00:00 2001
From: Pavel Labath <labath@google.com>
Date: Tue, 30 Jan 2018 12:19:34 +0000
Subject: [PATCH] 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

llvm-svn: 323753
---
 lldb/include/lldb/Utility/Log.h               | 23 +++++++++++++++++++
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 11 ++++-----
 lldb/unittests/Utility/LogTest.cpp            | 17 ++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 81028381b0eb..6e401e132055 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -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
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 433a2297aa97..614c76b2e2fb 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -3491,9 +3491,9 @@ size_t ObjectFileELF::ReadSectionData(Section *section,
        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(Section *section,
   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);
diff --git a/lldb/unittests/Utility/LogTest.cpp b/lldb/unittests/Utility/LogTest.cpp
index f4c2ba90189a..81fdce6efa36 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -240,6 +240,23 @@ TEST_F(LogChannelEnabledTest, log_options) {
             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.