Index: lldb/include/lldb/Core/Log.h =================================================================== --- lldb/include/lldb/Core/Log.h +++ lldb/include/lldb/Core/Log.h @@ -15,6 +15,7 @@ #include "lldb/Core/Logging.h" #include "lldb/Core/PluginInterface.h" #include "lldb/Utility/ConstString.h" +#include "lldb/Utility/Error.h" #include "lldb/lldb-private.h" // Other libraries and framework includes @@ -200,4 +201,15 @@ log_private->Format(__FILE__, __FUNCTION__, __VA_ARGS__); \ } while (0) +#define LLDB_LOG_ERROR(log, error, ...) \ + do { \ + ::lldb_private::Log *log_private = (log); \ + if (log_private) { \ + log_private->Format( \ + __FILE__, __FUNCTION__, "{0}", \ + fmt_error_context(std::forward(error), \ + __VA_ARGS__)); \ + } \ + } while (0) + #endif // liblldb_Log_h_ Index: lldb/include/lldb/Utility/Error.h =================================================================== --- lldb/include/lldb/Utility/Error.h +++ lldb/include/lldb/Utility/Error.h @@ -11,7 +11,10 @@ #define __DCError_h__ #if defined(__cplusplus) +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/Twine.h" #include "llvm/Support/DataTypes.h" +#include "llvm/Support/FormatAdapters.h" #include "llvm/Support/FormatVariadic.h" #include @@ -24,8 +27,6 @@ namespace lldb_private { -class Log; - //---------------------------------------------------------------------- /// @class Error Error.h "lldb/Utility/Error.h" /// @brief An error handling class. @@ -148,48 +149,6 @@ lldb::ErrorType GetType() const; //------------------------------------------------------------------ - /// Log an error to Log(). - /// - /// Log the error given a formatted string \a format. If the this - /// object contains an error code, update the error string to - /// contain the prefix "error: ", followed by the formatted string, - /// followed by the error value and any string that describes the - /// error value. This allows more context to be given to an error - /// string that remains cached in this object. Logging always occurs - /// even when the error code contains a non-error value. - /// - /// @param[in] format - /// A printf style format string. - /// - /// @param[in] ... - /// Variable arguments that are needed for the printf style - /// format string \a format. - //------------------------------------------------------------------ - void PutToLog(Log *log, const char *format, ...) - __attribute__((format(printf, 3, 4))); - - //------------------------------------------------------------------ - /// Log an error to Log() if the error value is an error. - /// - /// Log the error given a formatted string \a format only if the - /// error value in this object describes an error condition. If the - /// this object contains an error, update the error string to - /// contain the prefix "error: " followed by the formatted string, - /// followed by the error value and any string that describes the - /// error value. This allows more context to be given to an error - /// string that remains cached in this object. - /// - /// @param[in] format - /// A printf style format string. - /// - /// @param[in] ... - /// Variable arguments that are needed for the printf style - /// format string \a format. - //------------------------------------------------------------------ - void LogIfError(Log *log, const char *format, ...) - __attribute__((format(printf, 3, 4))); - - //------------------------------------------------------------------ /// Set accessor from a kern_return_t. /// /// Set accesssor for the error value to \a err and the error type @@ -304,12 +263,44 @@ namespace llvm { template <> struct format_provider { static void format(const lldb_private::Error &error, llvm::raw_ostream &OS, - llvm::StringRef Options) { - llvm::format_provider::format(error.AsCString(), OS, - Options); + llvm::StringRef Options); +}; +} + +namespace lldb_private { +namespace detail { +class ErrorWithContextAdapter final : public llvm::FormatAdapter { + std::string Context; + +public: + template + ErrorWithContextAdapter(Error &&Err, const char *Context, Ts &&... Args) + : FormatAdapter(std::forward(Err)) { + this->Context = llvm::formatv(Context, std::forward(Args)...).str(); + } + + void format(llvm::raw_ostream &Stream, llvm::StringRef Style) { + if (Item.Fail()) { + const char *err_str = Item.AsCString(); + if (err_str == nullptr) + err_str = "???"; + + Stream << llvm::formatv("error: {0} err = {1} ({2:x})", Context, err_str, + Item.GetError()); + } else { + Stream << llvm::formatv("{0} err = {1:x}", Context, Item.GetError()); + } } }; } +template +inline detail::ErrorWithContextAdapter +fmt_error_context(Error &&Err, const char *Context, Ts &&... Args) { + return detail::ErrorWithContextAdapter(std::forward(Err), Context, + std::forward(Args)...); +} +} + #endif // #if defined(__cplusplus) #endif // #ifndef __DCError_h__ Index: lldb/source/Core/Communication.cpp =================================================================== --- lldb/source/Core/Communication.cpp +++ lldb/source/Core/Communication.cpp @@ -322,10 +322,9 @@ comm->Disconnect(); done = true; } - if (log) - error.LogIfError( - log, "%p Communication::ReadFromConnection () => status = %s", p, - Communication::ConnectionStatusAsCString(status)); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, "status = {0}", + Communication::ConnectionStatusAsCString(status)); break; case eConnectionStatusInterrupted: // Synchronization signal from // SynchronizeWithReadThread() @@ -340,10 +339,9 @@ done = true; LLVM_FALLTHROUGH; case eConnectionStatusTimedOut: // Request timed out - if (log) - error.LogIfError( - log, "%p Communication::ReadFromConnection () => status = %s", p, - Communication::ConnectionStatusAsCString(status)); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, "status = {0}", + Communication::ConnectionStatusAsCString(status)); break; } } Index: lldb/source/Host/common/Host.cpp =================================================================== --- lldb/source/Host/common/Host.cpp +++ lldb/source/Host/common/Host.cpp @@ -685,10 +685,10 @@ posix_spawnattr_t attr; error.SetError(::posix_spawnattr_init(&attr), eErrorTypePOSIX); - if (error.Fail() || log) - error.PutToLog(log, "::posix_spawnattr_init ( &attr )"); - if (error.Fail()) + if (error.Fail()) { + LLDB_LOG_ERROR(log, error, "::posix_spawnattr_init ( &attr )"); return error; + } // Make a quick class that will cleanup the posix spawn attributes in case // we return in the middle of this function. @@ -709,11 +709,10 @@ short flags = GetPosixspawnFlags(launch_info); error.SetError(::posix_spawnattr_setflags(&attr, flags), eErrorTypePOSIX); - if (error.Fail() || log) - error.PutToLog(log, "::posix_spawnattr_setflags ( &attr, flags=0x%8.8x )", - flags); - if (error.Fail()) + if (error.Fail()) { + LLDB_LOG_ERROR(log, error, "::posix_spawnattr_setflags ( &attr, flags={0:x} )", flags); return error; + } // posix_spawnattr_setbinpref_np appears to be an Apple extension per: // http://www.unix.com/man-page/OSX/3/posix_spawnattr_setbinpref_np/ @@ -740,10 +739,9 @@ size_t ocount = 0; error.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu, &ocount), eErrorTypePOSIX); - if (error.Fail() || log) - error.PutToLog(log, "::posix_spawnattr_setbinpref_np ( &attr, 1, " - "cpu_type = 0x%8.8x, count => %llu )", - cpu, (uint64_t)ocount); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, "::posix_spawnattr_setbinpref_np ( &attr, 1, " + "cpu_type = {0:x}, count => {1} )", cpu, ocount); if (error.Fail() || ocount != 1) return error; @@ -813,10 +811,10 @@ posix_spawn_file_actions_t file_actions; error.SetError(::posix_spawn_file_actions_init(&file_actions), eErrorTypePOSIX); - if (error.Fail() || log) - error.PutToLog(log, "::posix_spawn_file_actions_init ( &file_actions )"); - if (error.Fail()) + if (error.Fail()) { + LLDB_LOG_ERROR(log, error, "::posix_spawn_file_actions_init ( &file_actions )"); return error; + } // Make a quick class that will cleanup the posix spawn attributes in case // we return in the middle of this function. @@ -838,16 +836,13 @@ ::posix_spawnp(&result_pid, exe_path, &file_actions, &attr, argv, envp), eErrorTypePOSIX); - if (error.Fail() || log) { - error.PutToLog( - log, "::posix_spawnp ( pid => %i, path = '%s', file_actions = %p, " - "attr = %p, argv = %p, envp = %p )", - result_pid, exe_path, static_cast(&file_actions), - static_cast(&attr), reinterpret_cast(argv), - reinterpret_cast(envp)); + if (error.Fail()) { + LLDB_LOG_ERROR(log, error, "::posix_spawnp(pid => {0}, path = '{1}', file_actions = {2}, " + "attr = {3}, argv = {4}, envp = {5} )", + result_pid, exe_path, &file_actions, &attr, argv, envp); if (log) { for (int ii = 0; argv[ii]; ++ii) - log->Printf("argv[%i] = '%s'", ii, argv[ii]); + LLDB_LOG(log, "argv[{0}] = '{1}'", ii, argv[ii]); } } @@ -856,16 +851,13 @@ ::posix_spawnp(&result_pid, exe_path, NULL, &attr, argv, envp), eErrorTypePOSIX); - if (error.Fail() || log) { - error.PutToLog(log, "::posix_spawnp ( pid => %i, path = '%s', " - "file_actions = NULL, attr = %p, argv = %p, envp = " - "%p )", - result_pid, exe_path, static_cast(&attr), - reinterpret_cast(argv), - reinterpret_cast(envp)); + if (error.Fail()) { + LLDB_LOG_ERROR(log, error, "::posix_spawnp ( pid => {0}, path = '{1}', " + "file_actions = NULL, attr = {2}, argv = {3}, envp = {4} )", + result_pid, exe_path, &attr, argv, envp); if (log) { for (int ii = 0; argv[ii]; ++ii) - log->Printf("argv[%i] = '%s'", ii, argv[ii]); + LLDB_LOG("argv[{0}] = '{1}'", ii, argv[ii]); } } } @@ -908,10 +900,9 @@ error.SetError( ::posix_spawn_file_actions_addclose(file_actions, info->GetFD()), eErrorTypePOSIX); - if (log && (error.Fail() || log)) - error.PutToLog(log, - "posix_spawn_file_actions_addclose (action=%p, fd=%i)", - static_cast(file_actions), info->GetFD()); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, "posix_spawn_file_actions_addclose (action={0}, fd={1})", + file_actions, info->GetFD()); } break; @@ -927,12 +918,10 @@ ::posix_spawn_file_actions_adddup2(file_actions, info->GetFD(), info->GetActionArgument()), eErrorTypePOSIX); - if (log && (error.Fail() || log)) - error.PutToLog( - log, - "posix_spawn_file_actions_adddup2 (action=%p, fd=%i, dup_fd=%i)", - static_cast(file_actions), info->GetFD(), - info->GetActionArgument()); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, + "posix_spawn_file_actions_adddup2 (action={0}, fd={1}, dup_fd={2})", + file_actions, info->GetFD(), info->GetActionArgument()); } break; @@ -952,11 +941,10 @@ file_actions, info->GetFD(), info->GetPath().str().c_str(), oflag, mode), eErrorTypePOSIX); - if (error.Fail() || log) - error.PutToLog(log, "posix_spawn_file_actions_addopen (action=%p, " - "fd=%i, path='%s', oflag=%i, mode=%i)", - static_cast(file_actions), info->GetFD(), - info->GetPath().str().c_str(), oflag, mode); + if (error.Fail()) + LLDB_LOG_ERROR(log, error, "posix_spawn_file_actions_addopen (action={0}, " + "fd={1}, path='{2}', oflag={3}, mode={4})", + file_actions, info->GetFD(), info->GetPath(), oflag, mode); } break; } Index: lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp =================================================================== --- lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp +++ lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp @@ -319,7 +319,7 @@ for (uint64_t i = 0; i < allglen; ++i) { goroutines.push_back(CreateGoroutineAtIndex(i, err)); if (err.Fail()) { - err.PutToLog(log, "OperatingSystemGo::UpdateThreadList"); + LLDB_LOG_ERROR(log, err, "OperatingSystemGo::UpdateThreadList"); return new_thread_list.GetSize(false) > 0; } } Index: lldb/source/Utility/Error.cpp =================================================================== --- lldb/source/Utility/Error.cpp +++ lldb/source/Utility/Error.cpp @@ -130,72 +130,6 @@ bool Error::Fail() const { return m_code != 0; } //---------------------------------------------------------------------- -// Log the error given a string with format. If the this object -// contains an error code, update the error string to contain the -// "error: " followed by the formatted string, followed by the error -// value and any string that describes the current error. This -// allows more context to be given to an error string that remains -// cached in this object. Logging always occurs even when the error -// code contains a non-error value. -//---------------------------------------------------------------------- -void Error::PutToLog(Log *log, const char *format, ...) { - char *arg_msg = nullptr; - va_list args; - va_start(args, format); - ::vasprintf(&arg_msg, format, args); - va_end(args); - - if (arg_msg != nullptr) { - if (Fail()) { - const char *err_str = AsCString(); - if (err_str == nullptr) - err_str = "???"; - - SetErrorStringWithFormat("error: %s err = %s (0x%8.8x)", arg_msg, err_str, - m_code); - if (log != nullptr) - log->Error("%s", m_string.c_str()); - } else { - if (log != nullptr) - log->Printf("%s err = 0x%8.8x", arg_msg, m_code); - } - ::free(arg_msg); - } -} - -//---------------------------------------------------------------------- -// Log the error given a string with format. If the this object -// contains an error code, update the error string to contain the -// "error: " followed by the formatted string, followed by the error -// value and any string that describes the current error. This -// allows more context to be given to an error string that remains -// cached in this object. Logging only occurs even when the error -// code contains a error value. -//---------------------------------------------------------------------- -void Error::LogIfError(Log *log, const char *format, ...) { - if (Fail()) { - char *arg_msg = nullptr; - va_list args; - va_start(args, format); - ::vasprintf(&arg_msg, format, args); - va_end(args); - - if (arg_msg != nullptr) { - const char *err_str = AsCString(); - if (err_str == nullptr) - err_str = "???"; - - SetErrorStringWithFormat("%s err = %s (0x%8.8x)", arg_msg, err_str, - m_code); - if (log != nullptr) - log->Error("%s", m_string.c_str()); - - ::free(arg_msg); - } - } -} - -//---------------------------------------------------------------------- // Set accesssor for the error value to "err" and the type to // "eErrorTypeMachKernel" //---------------------------------------------------------------------- @@ -334,3 +268,10 @@ bool Error::WasInterrupted() const { return (m_type == eErrorTypePOSIX && m_code == EINTR); } + +void llvm::format_provider::format( + const lldb_private::Error &error, llvm::raw_ostream &OS, + llvm::StringRef Options) { + llvm::format_provider::format(error.AsCString(), OS, + Options); +} Index: llvm/include/llvm/Support/FormatProviders.h =================================================================== --- llvm/include/llvm/Support/FormatProviders.h +++ llvm/include/llvm/Support/FormatProviders.h @@ -18,6 +18,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/ADT/Twine.h" #include "llvm/Support/FormatVariadicDetails.h" #include "llvm/Support/NativeFormatting.h" @@ -262,6 +263,12 @@ } }; +template <> struct format_provider { + static void format(const Twine &T, raw_ostream &Stream, StringRef Style) { + Stream << T; + } +}; + /// Implementation of format_provider for floating point types. /// /// The options string of a floating point type has the format: