diff --git a/src/glog/logging.h b/src/glog/logging.h index 6e3ea2615..a0759df0c 100644 --- a/src/glog/logging.h +++ b/src/glog/logging.h @@ -521,8 +521,10 @@ using PrefixFormatterCallback = void (*)(std::ostream&, const LogMessage&, GLOG_EXPORT void InstallPrefixFormatter(PrefixFormatterCallback callback, void* data = nullptr); -// Install a function which will be called after LOG(FATAL). -GLOG_EXPORT void InstallFailureFunction(logging_fail_func_t fail_func); +// Install a function which will be called after LOG(FATAL). Returns the +// previously set function. +GLOG_EXPORT logging_fail_func_t +InstallFailureFunction(logging_fail_func_t fail_func); [[deprecated( "Use the type-safe std::chrono::minutes EnableLogCleaner overload " @@ -606,13 +608,11 @@ namespace internal { // A container for a string pointer which can be evaluated to a bool - // true iff the pointer is nullptr. struct CheckOpString { - CheckOpString(std::string* str) : str_(str) {} - // No destructor: if str_ is non-nullptr, we're about to LOG(FATAL), - // so there's no point in cleaning up str_. + CheckOpString(std::unique_ptr str) : str_(std::move(str)) {} explicit operator bool() const noexcept { return GOOGLE_PREDICT_BRANCH_NOT_TAKEN(str_ != nullptr); } - std::string* str_; + std::unique_ptr str_; }; // Function is overloaded for integral types to allow static const @@ -671,7 +671,8 @@ GLOG_EXPORT void MakeCheckOpValueString(std::ostream* os, // Build the error message string. Specify no inlining for code size. template -std::string* MakeCheckOpString(const T1& v1, const T2& v2, const char* exprtext) +std::unique_ptr MakeCheckOpString(const T1& v1, const T2& v2, + const char* exprtext) #if defined(__has_attribute) # if __has_attribute(used) __attribute__((noinline)) @@ -696,15 +697,15 @@ class GLOG_EXPORT CheckOpMessageBuilder { // For inserting the second variable (adds an intermediate " vs. "). std::ostream* ForVar2(); // Get the result (inserts the closing ")"). - std::string* NewString(); + std::unique_ptr NewString(); private: std::ostringstream* stream_; }; template -std::string* MakeCheckOpString(const T1& v1, const T2& v2, - const char* exprtext) { +std::unique_ptr MakeCheckOpString(const T1& v1, const T2& v2, + const char* exprtext) { CheckOpMessageBuilder comb(exprtext); MakeCheckOpValueString(comb.ForVar1(), v1); MakeCheckOpValueString(comb.ForVar2(), v2); @@ -715,17 +716,18 @@ std::string* MakeCheckOpString(const T1& v1, const T2& v2, // The (int, int) specialization works around the issue that the compiler // will not instantiate the template version of the function on values of // unnamed enum type - see comment below. -#define DEFINE_CHECK_OP_IMPL(name, op) \ - template \ - inline std::string* name##Impl(const T1& v1, const T2& v2, \ - const char* exprtext) { \ - if (GOOGLE_PREDICT_TRUE(v1 op v2)) \ - return nullptr; \ - else \ - return MakeCheckOpString(v1, v2, exprtext); \ - } \ - inline std::string* name##Impl(int v1, int v2, const char* exprtext) { \ - return name##Impl(v1, v2, exprtext); \ +#define DEFINE_CHECK_OP_IMPL(name, op) \ + template \ + inline std::unique_ptr name##Impl(const T1& v1, const T2& v2, \ + const char* exprtext) { \ + if (GOOGLE_PREDICT_TRUE(v1 op v2)) { \ + return nullptr; \ + } \ + return MakeCheckOpString(v1, v2, exprtext); \ + } \ + inline std::unique_ptr name##Impl(int v1, int v2, \ + const char* exprtext) { \ + return name##Impl(v1, v2, exprtext); \ } // We use the full name Check_EQ, Check_NE, etc. in case the file including @@ -757,14 +759,15 @@ DEFINE_CHECK_OP_IMPL(Check_GT, >) // with other string implementations that get defined after this // file is included). Save the current meaning now and use it // in the macro. -typedef std::string _Check_string; +using _Check_string = std::string; # define CHECK_OP_LOG(name, op, val1, val2, log) \ - while (google::logging::internal::_Check_string* _result = \ + while (std::unique_ptr _result = \ google::logging::internal::Check##name##Impl( \ google::logging::internal::GetReferenceableValue(val1), \ google::logging::internal::GetReferenceableValue(val2), \ #val1 " " #op " " #val2)) \ - log(__FILE__, __LINE__, google::logging::internal::CheckOpString(_result)) \ + log(__FILE__, __LINE__, \ + google::logging::internal::CheckOpString(std::move(_result))) \ .stream() #else // In optimized mode, use CheckOpString to hint to compiler that @@ -820,8 +823,8 @@ typedef std::string _Check_string; // Helper functions for string comparisons. // To avoid bloat, the definitions are in logging.cc. -#define DECLARE_CHECK_STROP_IMPL(func, expected) \ - GLOG_EXPORT std::string* Check##func##expected##Impl( \ +#define DECLARE_CHECK_STROP_IMPL(func, expected) \ + GLOG_EXPORT std::unique_ptr Check##func##expected##Impl( \ const char* s1, const char* s2, const char* names); DECLARE_CHECK_STROP_IMPL(strcmp, true) @@ -840,7 +843,7 @@ DECLARE_CHECK_STROP_IMPL(strcasecmp, false) while (google::logging::internal::CheckOpString _result = \ google::logging::internal::Check##func##expected##Impl( \ (s1), (s2), #s1 " " #op " " #s2)) \ - LOG(FATAL) << *_result.str_ + LOG(FATAL) << (*_result.str_) // String (char*) equality/inequality checks. // CASE versions are case-insensitive. @@ -1332,7 +1335,7 @@ class GLOG_EXPORT LogMessage { LogMessage(const char* file, int line, const logging::internal::CheckOpString& result); - ~LogMessage(); + ~LogMessage() noexcept(false); // Flush a buffered message to the sink set in the constructor. Always // called by the destructor, it may also be called from elsewhere if @@ -1409,7 +1412,7 @@ class GLOG_EXPORT LogMessageFatal : public LogMessage { LogMessageFatal(const char* file, int line); LogMessageFatal(const char* file, int line, const logging::internal::CheckOpString& result); - [[noreturn]] ~LogMessageFatal(); + [[noreturn]] ~LogMessageFatal() noexcept(false); }; // A non-macro interface to the log facility; (useful @@ -1462,7 +1465,7 @@ namespace internal { template T CheckNotNull(const char* file, int line, const char* names, T&& t) { if (t == nullptr) { - LogMessageFatal(file, line, new std::string(names)); + LogMessageFatal(file, line, std::make_unique(names)); } return std::forward(t); } diff --git a/src/googletest.h b/src/googletest.h index 971c5b401..33af3feb6 100644 --- a/src/googletest.h +++ b/src/googletest.h @@ -196,8 +196,19 @@ void InitGoogleTest(int*, char**) {} } \ } while (0) -vector g_testlist; // the tests to run +# define EXPECT_THROW(statement, exception) \ + do { \ + try { \ + statement; \ + } catch (const exception&) { \ + printf("ok\n"); \ + } catch (...) { \ + fprintf(stderr, "%s\n", "Unexpected exception thrown"); \ + exit(EXIT_FAILURE); \ + } \ + } while (0) +vector g_testlist; // the tests to run # define TEST(a, b) \ struct Test_##a##_##b { \ Test_##a##_##b() { g_testlist.push_back(&Run); } \ diff --git a/src/logging.cc b/src/logging.cc index f95fb617a..fb283fd8a 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -1723,8 +1723,9 @@ const char* LogMessage::fullname() const noexcept { return data_->fullname_; } const char* LogMessage::basename() const noexcept { return data_->basename_; } const LogMessageTime& LogMessage::time() const noexcept { return time_; } -LogMessage::~LogMessage() { +LogMessage::~LogMessage() noexcept(false) { Flush(); + bool fail = data_->severity_ == GLOG_FATAL && exit_on_dfatal; #ifdef GLOG_THREAD_LOCAL_STORAGE if (data_ == static_cast(&thread_msg_data)) { data_->~LogMessageData(); @@ -1735,6 +1736,26 @@ LogMessage::~LogMessage() { #else // !defined(GLOG_THREAD_LOCAL_STORAGE) delete allocated_; #endif // defined(GLOG_THREAD_LOCAL_STORAGE) + // + + if (fail) { + const char* message = "*** Check failure stack trace: ***\n"; + if (write(fileno(stderr), message, strlen(message)) < 0) { + // Ignore errors. + } + AlsoErrorWrite(GLOG_FATAL, + glog_internal_namespace_::ProgramInvocationShortName(), + message); +#if defined(__cpp_lib_uncaught_exceptions) && \ + (__cpp_lib_uncaught_exceptions >= 201411L) + if (std::uncaught_exceptions() == 0) +#else + if (!std::uncaught_exception()) +#endif + { + Fail(); + } + } } int LogMessage::preserved_errno() const { return data_->preserved_errno_; } @@ -1894,22 +1915,7 @@ void LogMessage::SendToLog() EXCLUSIVE_LOCKS_REQUIRED(log_mutex) { } } - // release the lock that our caller (directly or indirectly) - // LogMessage::~LogMessage() grabbed so that signal handlers - // can use the logging facility. Alternately, we could add - // an entire unsafe logging interface to bypass locking - // for signal handlers but this seems simpler. - log_mutex.unlock(); LogDestination::WaitForSinks(data_); - - const char* message = "*** Check failure stack trace: ***\n"; - if (write(fileno(stderr), message, strlen(message)) < 0) { - // Ignore errors. - } - AlsoErrorWrite(GLOG_FATAL, - glog_internal_namespace_::ProgramInvocationShortName(), - message); - Fail(); } } @@ -1941,8 +1947,8 @@ NullStreamFatal::~NullStreamFatal() { std::abort(); } -void InstallFailureFunction(logging_fail_func_t fail_func) { - g_logging_fail_func = fail_func; +logging_fail_func_t InstallFailureFunction(logging_fail_func_t fail_func) { + return std::exchange(g_logging_fail_func, fail_func); } void LogMessage::Fail() { g_logging_fail_func(); } @@ -2533,17 +2539,17 @@ namespace logging { namespace internal { // Helper functions for string comparisons. #define DEFINE_CHECK_STROP_IMPL(name, func, expected) \ - string* Check##func##expected##Impl(const char* s1, const char* s2, \ - const char* names) { \ + std::unique_ptr Check##func##expected##Impl( \ + const char* s1, const char* s2, const char* names) { \ bool equal = s1 == s2 || (s1 && s2 && !func(s1, s2)); \ - if (equal == expected) \ + if (equal == (expected)) \ return nullptr; \ else { \ ostringstream ss; \ if (!s1) s1 = ""; \ if (!s2) s2 = ""; \ ss << #name " failed: " << names << " (" << s1 << " vs. " << s2 << ")"; \ - return new string(ss.str()); \ + return std::make_unique(ss.str()); \ } \ } DEFINE_CHECK_STROP_IMPL(CHECK_STREQ, strcmp, true) @@ -2635,7 +2641,7 @@ LogMessageFatal::LogMessageFatal(const char* file, int line, const logging::internal::CheckOpString& result) : LogMessage(file, line, result) {} -LogMessageFatal::~LogMessageFatal() { +LogMessageFatal::~LogMessageFatal() noexcept(false) { Flush(); LogMessage::Fail(); } @@ -2655,9 +2661,9 @@ ostream* CheckOpMessageBuilder::ForVar2() { return stream_; } -string* CheckOpMessageBuilder::NewString() { +std::unique_ptr CheckOpMessageBuilder::NewString() { *stream_ << ")"; - return new string(stream_->str()); + return std::make_unique(stream_->str()); } template <> diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc index 1c5153b0c..321f38fa3 100644 --- a/src/logging_unittest.cc +++ b/src/logging_unittest.cc @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -1571,3 +1572,17 @@ TEST(EmailLogging, MaliciousAddress) { EXPECT_FALSE( SendEmail("!/bin/true@example.com", "Example subject", "Example body")); } + +TEST(Logging, FatalThrow) { + auto const fail_func = + InstallFailureFunction(+[]() +#if defined(__has_attribute) +# if __has_attribute(noreturn) + __attribute__((noreturn)) +# endif // __has_attribute(noreturn) +#endif // defined(__has_attribute) + { throw std::logic_error{"fail"}; }); + auto restore_fail = [fail_func] { InstallFailureFunction(fail_func); }; + ScopedExit restore{restore_fail}; + EXPECT_THROW({ LOG(FATAL) << "must throw to fail"; }, std::logic_error); +}