Allow making SAL_LOG based output fatal

This introduces the [+-]FATAL marker for SAL_LOG. This way you can
set part of the matching SAL_LOG string to std::abort on match.

The example "SAL_LOG=+FATAL+WARN.vcl.scheduler-FATAL+INFO" will
abort for any "+WARN.vcl.scheduler" match, but will just print all
additional "+INFO" logs.

Change-Id: Ib77f194a78f5165e6c885c82374ae41293815ee9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97651
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>

3 files changed, 78 insertions, 25 deletions

diff --git a/compilerplugins/clang/stringconcatliterals.cxx b/compilerplugins/clang/stringconcatliterals.cxx
index 0f26f4f553bc..0b52bd0c9b4f 100644
--- a/compilerplugins/clang/stringconcatliterals.cxx
+++ b/compilerplugins/clang/stringconcatliterals.cxx
@@ -109,7 +109,8 @@ bool StringConcatLiterals::VisitCallExpr(CallExpr const * expr) {
                 compiler.getSourceManager().getSpellingLoc(
                     compiler.getSourceManager().getImmediateMacroCallerLoc(
                         compiler.getSourceManager().getImmediateMacroCallerLoc(
-                            compat::getBeginLoc(expr))))),
+                            compiler.getSourceManager().getImmediateMacroCallerLoc(
+                                compat::getBeginLoc(expr)))))),
             SRCDIR "/include/tools/diagnose_ex.h"))
         return true;
 
diff --git a/include/sal/log.hxx b/include/sal/log.hxx
index 00d533ab5495..6bb0d1b43d3d 100644
--- a/include/sal/log.hxx
+++ b/include/sal/log.hxx
@@ -26,11 +26,20 @@
 
 /// @cond INTERNAL
 
+enum sal_detail_LogAction
+{
+    SAL_DETAIL_LOG_ACTION_IGNORE,
+    SAL_DETAIL_LOG_ACTION_LOG,
+    SAL_DETAIL_LOG_ACTION_FATAL
+};
+
 extern "C" SAL_DLLPUBLIC void SAL_CALL sal_detail_log(
     sal_detail_LogLevel level, char const * area, char const * where,
     char const * message, sal_uInt32 backtraceDepth);
 
-extern "C" SAL_DLLPUBLIC sal_Bool SAL_CALL sal_detail_log_report(
+// the return value is actually "enum sal_detail_LogAction", but due to ABI
+// compatibility, it's left as the original "sal_Bool" / "unsigned char".
+extern "C" SAL_DLLPUBLIC unsigned char SAL_CALL sal_detail_log_report(
     sal_detail_LogLevel level, char const * area);
 
 namespace sal { namespace detail {
@@ -113,22 +122,38 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER StreamIgnore const &) {
 
 } }
 
+// to prevent using a local variable, which can eventually shadow,
+// resulting in compiler warnings (or even errors with -Werror)
+#define SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream) \
+    if (sizeof ::sal::detail::getResult( \
+            ::sal::detail::StreamStart() << stream) == 1) \
+    { \
+        ::sal_detail_log( \
+            (level), (area), (where), \
+            ::sal::detail::unwrapStream( \
+                ::sal::detail::StreamStart() << stream), \
+            0); \
+    } else { \
+        ::std::ostringstream sal_detail_stream; \
+        sal_detail_stream << stream; \
+        ::sal::detail::log( \
+            (level), (area), (where), sal_detail_stream, 0); \
+    }
+
 #define SAL_DETAIL_LOG_STREAM(condition, level, area, where, stream) \
     do { \
-        if ((condition) && sal_detail_log_report(level, area)) { \
-            if (sizeof ::sal::detail::getResult( \
-                    ::sal::detail::StreamStart() << stream) == 1) \
+        if (condition) \
+        { \
+            switch (sal_detail_log_report(level, area)) \
             { \
-                ::sal_detail_log( \
-                    (level), (area), (where), \
-                    ::sal::detail::unwrapStream( \
-                        ::sal::detail::StreamStart() << stream), \
-                    0); \
-            } else { \
-                ::std::ostringstream sal_detail_stream; \
-                sal_detail_stream << stream; \
-                ::sal::detail::log( \
-                    (level), (area), (where), sal_detail_stream, 0); \
+            case SAL_DETAIL_LOG_ACTION_IGNORE: break; \
+            case SAL_DETAIL_LOG_ACTION_LOG: \
+                SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream); \
+                break; \
+            case SAL_DETAIL_LOG_ACTION_FATAL: \
+                SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream); \
+                std::abort(); \
+                break; \
             } \
         } \
     } while (false)
@@ -235,7 +260,7 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER StreamIgnore const &) {
       <switch> ::= <sense><item>
       <sense> ::= "+"|"-"
       <item> ::= <flag>|<level>("."<area>)?
-      <flag> ::= "TIMESTAMP"|"RELATIVETIMER"
+      <flag> ::= "TIMESTAMP"|"RELATIVETIMER"|"FATAL"
       <level> ::= "INFO"|"WARN"
     @endverbatim
 
@@ -252,6 +277,12 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER StreamIgnore const &) {
     the level switch(es)) to be prefixed by a relative timestamp in
     seconds since the first output line like 1.312.
 
+    The "+FATAL" flag will cause later matching rules to log and call
+    std::abort. This can be disabled at some later point by using the
+    "-FATAL" flag before specifying additional rules. The flag will just
+    abort on positive rules, as it doesn't seem to make sense to abort
+    on ignored output.
+
     If both +TIMESTAMP and +RELATIVETIMER are specified, they are
     output in that order.
 
diff --git a/sal/osl/all/log.cxx b/sal/osl/all/log.cxx
index ed663076b8d2..927e78b97064 100644
--- a/sal/osl/all/log.cxx
+++ b/sal/osl/all/log.cxx
@@ -344,7 +344,9 @@ void sal_detail_logFormat(
     sal_detail_LogLevel level, char const * area, char const * where,
     char const * format, ...)
 {
-    if (!sal_detail_log_report(level, area))
+    const sal_detail_LogAction eAction
+        = static_cast<sal_detail_LogAction>(sal_detail_log_report(level, area));
+    if (eAction == SAL_DETAIL_LOG_ACTION_IGNORE)
         return;
 
     std::va_list args;
@@ -359,11 +361,15 @@ void sal_detail_logFormat(
     }
     sal_detail_log(level, area, where, buf, 0);
     va_end(args);
+
+    if (eAction == SAL_DETAIL_LOG_ACTION_FATAL)
+        std::abort();
 }
 
-sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
+unsigned char sal_detail_log_report(sal_detail_LogLevel level, char const * area)
+{
     if (level == SAL_DETAIL_LOG_LEVEL_DEBUG) {
-        return true;
+        return SAL_DETAIL_LOG_ACTION_LOG;
     }
     assert(area != nullptr);
     static char const* const env = [] {
@@ -379,17 +385,26 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
         // no matching switches at all, the result will be negative (and
         // initializing with 1 is safe as the length of a valid switch, even
         // without the "+"/"-" prefix, will always be > 1)
+    bool senseFatal[2] = { false, false };
     bool seenWarn = false;
+    bool bFlagFatal = false;
     for (char const * p = env;;) {
         Sense sense;
         switch (*p++) {
         case '\0':
+        {
             if (level == SAL_DETAIL_LOG_LEVEL_WARN && !seenWarn)
                 return sal_detail_log_report(SAL_DETAIL_LOG_LEVEL_INFO, area);
-            return senseLen[POSITIVE] >= senseLen[NEGATIVE];
-                // if a specific item is both positive and negative
-                // (senseLen[POSITIVE] == senseLen[NEGATIVE]), default to
-                // positive
+
+            sal_detail_LogAction eAction = SAL_DETAIL_LOG_ACTION_IGNORE;
+            // if a specific item is positive and negative (==), default to positive
+            if (senseLen[POSITIVE] >= senseLen[NEGATIVE])
+            {
+                if (senseFatal[POSITIVE]) eAction = SAL_DETAIL_LOG_ACTION_FATAL;
+                else eAction = SAL_DETAIL_LOG_ACTION_LOG;
+            }
+            return eAction;
+        }
         case '+':
             sense = POSITIVE;
             break;
@@ -397,7 +412,7 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
             sense = NEGATIVE;
             break;
         default:
-            return true; // upon an illegal SAL_LOG value, enable everything
+            return SAL_DETAIL_LOG_ACTION_LOG; // upon an illegal SAL_LOG value, enable everything
         }
         char const * p1 = p;
         while (*p1 != '.' && *p1 != '+' && *p1 != '-' && *p1 != '\0') {
@@ -410,13 +425,17 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
         {
             match = level == SAL_DETAIL_LOG_LEVEL_WARN;
             seenWarn = true;
+        } else if (equalStrings(p, p1 - p, RTL_CONSTASCII_STRINGPARAM("FATAL")))
+        {
+            bFlagFatal = (sense == POSITIVE);
+            match = false;
         } else if (equalStrings(p, p1 - p, RTL_CONSTASCII_STRINGPARAM("TIMESTAMP")) ||
                    equalStrings(p, p1 - p, RTL_CONSTASCII_STRINGPARAM("RELATIVETIMER")))
         {
             // handled later
             match = false;
         } else {
-            return true;
+            return SAL_DETAIL_LOG_ACTION_LOG;
                 // upon an illegal SAL_LOG value, everything is considered
                 // positive
         }
@@ -433,9 +452,11 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
                         && equalStrings(p1, n, area, n)))
                 {
                     senseLen[sense] = p2 - p;
+                    senseFatal[sense] = bFlagFatal;
                 }
             } else {
                 senseLen[sense] = p1 - p;
+                senseFatal[sense] = bFlagFatal;
             }
         }
         p = p2;