From f94070748c46199faf601e45be0c4643ce468da2 Mon Sep 17 00:00:00 2001 From: maierlars Date: Mon, 23 Aug 2021 17:54:12 +0200 Subject: [PATCH 1/8] First version for TRI_ASSERT logger stream. --- .../Replication2/ReplicatedLog/LogLeader.cpp | 3 +- lib/Basics/CrashHandler.cpp | 6 ++- lib/Basics/CrashHandler.h | 2 +- lib/Basics/debugging.h | 47 ++++++++++++++----- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/arangod/Replication2/ReplicatedLog/LogLeader.cpp b/arangod/Replication2/ReplicatedLog/LogLeader.cpp index d2905f5a65f0..0fc0ada8daf2 100644 --- a/arangod/Replication2/ReplicatedLog/LogLeader.cpp +++ b/arangod/Replication2/ReplicatedLog/LogLeader.cpp @@ -481,7 +481,8 @@ auto replicated_log::LogLeader::GuardedLeaderData::updateCommitIndexLeader( << "updating commit index to " << newIndex << "with quorum " << quorum->quorum; auto oldIndex = _commitIndex; - TRI_ASSERT(_commitIndex < newIndex); + TRI_ASSERT(_commitIndex < newIndex) + << "_commitIndex == " << _commitIndex << ", newIndex == " << newIndex; _commitIndex = newIndex; _lastQuorum = quorum; diff --git a/lib/Basics/CrashHandler.cpp b/lib/Basics/CrashHandler.cpp index ef2f5dc21470..6a14c901fac4 100644 --- a/lib/Basics/CrashHandler.cpp +++ b/lib/Basics/CrashHandler.cpp @@ -584,7 +584,7 @@ void CrashHandler::crash(char const* context) { } /// @brief logs an assertion failure and crashes the program -void CrashHandler::assertionFailure(char const* file, int line, char const* func, char const* context) { +void CrashHandler::assertionFailure(char const* file, int line, char const* func, char const* context, const char *message) { // assemble an "assertion failured in file:line: message" string char buffer[4096]; memset(&buffer[0], 0, sizeof(buffer)); @@ -601,6 +601,10 @@ void CrashHandler::assertionFailure(char const* file, int line, char const* func } appendNullTerminatedString(": ", p); appendNullTerminatedString(context, 256, p); + if (message != nullptr) { + appendNullTerminatedString(" - ", p); + appendNullTerminatedString(message, p); + } crash(&buffer[0]); } diff --git a/lib/Basics/CrashHandler.h b/lib/Basics/CrashHandler.h index 7c5117f9a9d4..d81c41732898 100644 --- a/lib/Basics/CrashHandler.h +++ b/lib/Basics/CrashHandler.h @@ -33,7 +33,7 @@ class CrashHandler { [[noreturn]] static void crash(char const* context); /// @brief logs an assertion failure and crashes the program - [[noreturn]] static void assertionFailure(char const* file, int line, char const* func, char const* context); + [[noreturn]] static void assertionFailure(char const* file, int line, char const* func, char const* context, const char* message); /// @brief set flag to kill process hard using SIGKILL, in order to circumvent core /// file generation etc. diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index b987a0c4b3eb..8dfe1751d6ff 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -199,6 +200,33 @@ enable_if_t::value, std::ostream&> operator<<(std::ostream& o, T return o; } +namespace debug { + +struct NoOpStream { + template + auto operator<<(T const&) -> NoOpStream& { + return *this; + } +}; + +struct AssertionLogger { + void operator&(std::stringstream const& str) { + std::string message = str.str(); + arangodb::CrashHandler::assertionFailure(file, line, function, expr, + message.empty() ? nullptr : message.c_str()); + } + void operator&(std::ostream const& os) { + operator&(static_cast(os)); + } + + const char* file; + int line; + const char *function; + const char* expr; +}; + +} // namespace debug + } // namespace arangodb /// @brief assert @@ -206,24 +234,17 @@ enable_if_t::value, std::ostream&> operator<<(std::ostream& o, T #ifdef ARANGODB_ENABLE_MAINTAINER_MODE -#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - do { \ - if (!(ADB_LIKELY(expr))) { \ - arangodb::CrashHandler::assertionFailure(__FILE__, __LINE__, __FUNCTION__, #expr); \ - } else { \ - } \ - } while (false) +#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ + (!(ADB_LIKELY(expr))) \ + ? (void)nullptr \ + : arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ + (std::stringstream{}) #else #define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - do { \ - if (false) { \ - (void)(expr); \ - } \ - } while (false) + (false) ? (void)(expr) : arangodb::debug::AssertionLogger{} & (NoOpStream{}) #endif // #ifdef ARANGODB_ENABLE_MAINTAINER_MODE #endif // #ifndef TRI_ASSERT - From b0815e4193d8bec29ad89d647ff52fae9e382ae0 Mon Sep 17 00:00:00 2001 From: maierlars Date: Mon, 23 Aug 2021 21:34:50 +0200 Subject: [PATCH 2/8] Fixing compilation and wrong bool expression. --- lib/Basics/CrashHandler.cpp | 2 +- lib/Basics/debugging.h | 34 +++++++++++++++++++++------------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/Basics/CrashHandler.cpp b/lib/Basics/CrashHandler.cpp index 6a14c901fac4..92c29ec694c0 100644 --- a/lib/Basics/CrashHandler.cpp +++ b/lib/Basics/CrashHandler.cpp @@ -602,7 +602,7 @@ void CrashHandler::assertionFailure(char const* file, int line, char const* func appendNullTerminatedString(": ", p); appendNullTerminatedString(context, 256, p); if (message != nullptr) { - appendNullTerminatedString(" - ", p); + appendNullTerminatedString(" ; ", p); appendNullTerminatedString(message, p); } diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index 8dfe1751d6ff..cf4d203505bc 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -209,19 +209,26 @@ struct NoOpStream { } }; +struct StringStreamWrapper { + template + auto operator<<(T const& v) -> StringStreamWrapper& { + _stream << v; + return *this; + } + + std::stringstream _stream; +}; + struct AssertionLogger { - void operator&(std::stringstream const& str) { - std::string message = str.str(); + void operator&(StringStreamWrapper const& wrapper) { + std::string message = wrapper._stream.str(); arangodb::CrashHandler::assertionFailure(file, line, function, expr, message.empty() ? nullptr : message.c_str()); } - void operator&(std::ostream const& os) { - operator&(static_cast(os)); - } - + void operator&(NoOpStream const&) {} const char* file; int line; - const char *function; + const char* function; const char* expr; }; @@ -234,16 +241,17 @@ struct AssertionLogger { #ifdef ARANGODB_ENABLE_MAINTAINER_MODE -#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - (!(ADB_LIKELY(expr))) \ - ? (void)nullptr \ - : arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ - (std::stringstream{}) +#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ + (ADB_LIKELY(expr)) \ + ? (void)nullptr \ + : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ + (::arangodb::debug::StringStreamWrapper{}) #else #define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - (false) ? (void)(expr) : arangodb::debug::AssertionLogger{} & (NoOpStream{}) + (false) ? (void)(expr) \ + : ::arangodb::debug::AssertionLogger{} & (::arangodb::debug::NoOpStream{}) #endif // #ifdef ARANGODB_ENABLE_MAINTAINER_MODE From a04948fb224a047d5374d06f1bae917f6b0c543e Mon Sep 17 00:00:00 2001 From: maierlars Date: Tue, 24 Aug 2021 09:56:37 +0200 Subject: [PATCH 3/8] Remove stream wrapper. --- lib/Basics/debugging.h | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index cf4d203505bc..6ab6f43caf3c 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -209,22 +209,16 @@ struct NoOpStream { } }; -struct StringStreamWrapper { - template - auto operator<<(T const& v) -> StringStreamWrapper& { - _stream << v; - return *this; - } - - std::stringstream _stream; -}; - struct AssertionLogger { - void operator&(StringStreamWrapper const& wrapper) { - std::string message = wrapper._stream.str(); + void operator&(std::ostringstream const& stream) const { + std::string message = stream.str(); arangodb::CrashHandler::assertionFailure(file, line, function, expr, message.empty() ? nullptr : message.c_str()); } + // can be removed in C++20 because of LWG 1203 + void operator&(std::ostream const& stream) const { + operator&(static_cast(stream)); + } void operator&(NoOpStream const&) {} const char* file; int line; @@ -245,7 +239,7 @@ struct AssertionLogger { (ADB_LIKELY(expr)) \ ? (void)nullptr \ : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ - (::arangodb::debug::StringStreamWrapper{}) + std::ostringstream{} #else From f2b9bba67b30c1734f76b51154d2cc641c02db7d Mon Sep 17 00:00:00 2001 From: maierlars Date: Tue, 24 Aug 2021 11:33:27 +0200 Subject: [PATCH 4/8] Fix nonmaintainer compile. --- lib/Basics/debugging.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index 6ab6f43caf3c..3be1dbbef1bc 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -201,7 +201,6 @@ enable_if_t::value, std::ostream&> operator<<(std::ostream& o, T } namespace debug { - struct NoOpStream { template auto operator<<(T const&) -> NoOpStream& { @@ -210,6 +209,7 @@ struct NoOpStream { }; struct AssertionLogger { +#ifdef ARANGODB_ENABLE_MAINTAINER_MODE void operator&(std::ostringstream const& stream) const { std::string message = stream.str(); arangodb::CrashHandler::assertionFailure(file, line, function, expr, @@ -219,6 +219,7 @@ struct AssertionLogger { void operator&(std::ostream const& stream) const { operator&(static_cast(stream)); } +#endif void operator&(NoOpStream const&) {} const char* file; int line; @@ -239,13 +240,13 @@ struct AssertionLogger { (ADB_LIKELY(expr)) \ ? (void)nullptr \ : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ - std::ostringstream{} + std::ostringstream {} #else #define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ (false) ? (void)(expr) \ - : ::arangodb::debug::AssertionLogger{} & (::arangodb::debug::NoOpStream{}) + : ::arangodb::debug::AssertionLogger{} & ::arangodb::debug::NoOpStream {} #endif // #ifdef ARANGODB_ENABLE_MAINTAINER_MODE From cbedc261419b2c563468c1cb125a2114a4058956 Mon Sep 17 00:00:00 2001 From: maierlars Date: Tue, 24 Aug 2021 11:42:47 +0200 Subject: [PATCH 5/8] Fix non-maintainer compile for real. --- lib/Basics/debugging.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index 3be1dbbef1bc..e870b7151c05 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -244,9 +244,9 @@ struct AssertionLogger { #else -#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - (false) ? (void)(expr) \ - : ::arangodb::debug::AssertionLogger{} & ::arangodb::debug::NoOpStream {} +#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ + (true) ? ((false) ? (void)(expr) : (void)nullptr) \ + : ::arangodb::debug::AssertionLogger{} & ::arangodb::debug::NoOpStream {} #endif // #ifdef ARANGODB_ENABLE_MAINTAINER_MODE From 4c59a2091e266e105a130406c96d2b6f1d692744 Mon Sep 17 00:00:00 2001 From: maierlars Date: Tue, 24 Aug 2021 14:13:02 +0200 Subject: [PATCH 6/8] Made noop operator& noexcept and const. --- lib/Basics/debugging.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index e870b7151c05..2ace115523e0 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -219,12 +219,13 @@ struct AssertionLogger { void operator&(std::ostream const& stream) const { operator&(static_cast(stream)); } -#endif - void operator&(NoOpStream const&) {} + const char* file; int line; const char* function; const char* expr; +#endif + void operator&(NoOpStream const&) const noexcept {} }; } // namespace debug From b0db1f77c4185db17ca8d97cb5c6848962c374f5 Mon Sep 17 00:00:00 2001 From: maierlars Date: Tue, 24 Aug 2021 16:44:17 +0200 Subject: [PATCH 7/8] Added pretty function macros. --- lib/Basics/debugging.h | 11 ++++++----- lib/Basics/system-compiler.h | 8 ++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index 2ace115523e0..a2adf8d11856 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -203,7 +203,7 @@ enable_if_t::value, std::ostream&> operator<<(std::ostream& o, T namespace debug { struct NoOpStream { template - auto operator<<(T const&) -> NoOpStream& { + auto operator<<(T const&) noexcept -> NoOpStream& { return *this; } }; @@ -237,10 +237,11 @@ struct AssertionLogger { #ifdef ARANGODB_ENABLE_MAINTAINER_MODE -#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ - (ADB_LIKELY(expr)) \ - ? (void)nullptr \ - : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, __FUNCTION__, #expr} & \ +#define TRI_ASSERT(expr) /*GCOVR_EXCL_LINE*/ \ + (ADB_LIKELY(expr)) \ + ? (void)nullptr \ + : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, \ + ARANGODB_PRETTY_FUNCTION, #expr} & \ std::ostringstream {} #else diff --git a/lib/Basics/system-compiler.h b/lib/Basics/system-compiler.h index 090c2dd0fd5b..c586a3bcccc0 100644 --- a/lib/Basics/system-compiler.h +++ b/lib/Basics/system-compiler.h @@ -74,3 +74,11 @@ #endif #endif +// pretty function name macro +#if defined(__clang__) || defined(__GNUC__) +#define ARANGODB_PRETTY_FUNCTION __PRETTY_FUNCTION__ +#elif defined(_MSC_VER) +#define ARANGODB_PRETTY_FUNCTION __FUNCSIG__ +#else +#define ARANGODB_PRETTY_FUNCTION __func__ +#endif From 83121f8282aba4bc9a1c07260050e534a22a49ef Mon Sep 17 00:00:00 2001 From: maierlars Date: Thu, 26 Aug 2021 11:10:03 +0200 Subject: [PATCH 8/8] Do not allocate std::ostringstream on the stack. --- lib/Basics/debugging.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Basics/debugging.h b/lib/Basics/debugging.h index a2adf8d11856..a5557fe5c1c5 100644 --- a/lib/Basics/debugging.h +++ b/lib/Basics/debugging.h @@ -226,6 +226,10 @@ struct AssertionLogger { const char* expr; #endif void operator&(NoOpStream const&) const noexcept {} + static auto getOutputStream() -> std::ostringstream&& { + static thread_local std::ostringstream stream; + return std::move(stream); + } }; } // namespace debug @@ -242,7 +246,7 @@ struct AssertionLogger { ? (void)nullptr \ : ::arangodb::debug::AssertionLogger{__FILE__, __LINE__, \ ARANGODB_PRETTY_FUNCTION, #expr} & \ - std::ostringstream {} + ::arangodb::debug::AssertionLogger::getOutputStream() #else