[Libreoffice-commits] online.git: common/Log.cpp common/SigUtil.cpp

[Ashod Nakashian (via logerrit)]

 common/Log.cpp     |    3 ++-
 common/SigUtil.cpp |   41 +++++++++--------------------------------
 2 files changed, 11 insertions(+), 33 deletions(-)

New commits:
commit 3ebbc6213b6e398bfc4a71b3eb5f03f421d0ffe6
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Mon Nov 18 22:13:00 2019 -0500
Commit:     Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Tue Nov 19 16:13:33 2019 +0100

    wsd: avoid malloc in signal handler
    
    malloc is not signal safe, and must not be called
    from signal-safe functions. If malloc itself signals,
    calling it in the signal handler can deadlock.
    
    Luckily, we only needed malloc for getting the
    backtrace strings. Now we just write directly to
    stderr, which is faster, cleaner, and safer.
    
    Change-Id: I54093f45e05f2a0fd3c5cde0cc2104ffe6d81d2a
    Reviewed-on: https://gerrit.libreoffice.org/83151
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/Log.cpp b/common/Log.cpp
index e08287bb9..ef2b03038 100644
--- a/common/Log.cpp
+++ b/common/Log.cpp
@@ -83,7 +83,7 @@ namespace Log
         while (true)
         {
             const int length = std::strlen(message);
-            const int written = write (STDERR_FILENO, message, length);
+            const int written = write(STDERR_FILENO, message, length);
             if (written < 0)
             {
                 if (errno == EINTR)
@@ -119,6 +119,7 @@ namespace Log
         Poco::DateTime time;
 #ifdef __linux
         const long osTid = Util::getThreadId();
+        // Note that snprintf is deemed signal-safe in most common implementations.
         snprintf(buffer, len, "%s-%.05lu %.4u-%.2u-%.2u %.2u:%.2u:%.2u.%.6u [ %s ] %s  ",
                     (Source.getInited() ? Source.getId().c_str() : "<shutdown>"),
                     osTid,
diff --git a/common/SigUtil.cpp b/common/SigUtil.cpp
index e5dabcb6f..c24ffb9d5 100644
--- a/common/SigUtil.cpp
+++ b/common/SigUtil.cpp
@@ -251,13 +251,6 @@ namespace SigUtil
         Log::signalLog(signalName(signal));
         Log::signalLog("\n");
 
-        if (std::getenv("LOOL_DEBUG"))
-        {
-            Log::signalLog(FatalGdbString);
-            LOG_ERR("Sleeping 30s to allow debugging.");
-            sleep(30);
-        }
-
         struct sigaction action;
 
         sigemptyset(&action.sa_mask);
@@ -274,34 +267,17 @@ namespace SigUtil
 
     void dumpBacktrace()
     {
-        char header[32];
-        sprintf(header, "Backtrace %d:\n", getpid());
-
 #if !defined(__ANDROID__)
+        Log::signalLog("\nBacktrace ");
+        Log::signalLogNumber(getpid());
+        Log::signalLog(":\n");
+
         const int maxSlots = 50;
         void *backtraceBuffer[maxSlots];
-        int numSlots = backtrace(backtraceBuffer, maxSlots);
+        const int numSlots = backtrace(backtraceBuffer, maxSlots);
         if (numSlots > 0)
         {
-            char **symbols = backtrace_symbols(backtraceBuffer, numSlots);
-            if (symbols != nullptr)
-            {
-                struct iovec ioVector[maxSlots*2+1];
-                ioVector[0].iov_base = static_cast<void*>(header);
-                ioVector[0].iov_len = std::strlen(static_cast<const char*>(ioVector[0].iov_base));
-                for (int i = 0; i < numSlots; i++)
-                {
-                    ioVector[1+i*2+0].iov_base = symbols[i];
-                    ioVector[1+i*2+0].iov_len = std::strlen(static_cast<const char *>(ioVector[1+i*2+0].iov_base));
-                    ioVector[1+i*2+1].iov_base = const_cast<void*>(static_cast<const void*>("\n"));
-                    ioVector[1+i*2+1].iov_len = 1;
-                }
-
-                if (writev(STDERR_FILENO, ioVector, numSlots*2+1) == -1)
-                {
-                    LOG_SYS("Failed to dump backtrace to stderr.");
-                }
-            }
+            backtrace_symbols_fd(backtraceBuffer, numSlots, STDERR_FILENO);
         }
 #else
         LOG_SYS("Backtrace not available on Android.");
@@ -309,6 +285,7 @@ namespace SigUtil
 
         if (std::getenv("LOOL_DEBUG"))
         {
+            Log::signalLog(FatalGdbString);
             LOG_ERR("Sleeping 30s to allow debugging.");
             sleep(30);
         }
@@ -328,9 +305,9 @@ namespace SigUtil
         sigaction(SIGILL, &action, nullptr);
         sigaction(SIGFPE, &action, nullptr);
 
-        // prepare this in advance just in case.
+        // Prepare this in advance just in case.
         std::ostringstream stream;
-        stream << "\nFatal signal! Attach debugger with:\n"
+        stream << "\nERROR: Fatal signal! Attach debugger with:\n"
                << "sudo gdb --pid=" << getpid() << "\n or \n"
                << "sudo gdb --q --n --ex 'thread apply all backtrace full' --batch --pid="
                << getpid() << "\n";