Fix CrashHandler & make async-signal-safe

Source/CommandLine.cpp

@@ -17,7 +17,6 @@
 #include "CrashHandler.h"
 
 #if JUCE_MAC
- #include <signal.h>
  #include <sys/types.h>
  #include <unistd.h>
 #endif
@@ -56,37 +55,10 @@ inline void logAndFlush (const juce::String& m)
     std::cout << m << std::flush;
 }
 
-//==============================================================================
-#if JUCE_MAC
-static void kill9WithSomeMercy (int signal)
-{
-    juce::Logger::writeToLog ("pluginval received " + juce::String(::strsignal(signal)) + ", exiting immediately");
-
-    // Use std::_Exit here instead of kill as kill doesn't seem to set the exit code of the process so is picked up as a "pass" in the host process
-    std::_Exit (SIGKILL);
-}
-
-// Avoid showing the macOS crash dialog, which can cause the process to hang
-static void setupSignalHandling()
-{
-    const int signals[] = { SIGFPE, SIGILL, SIGSEGV, SIGBUS, SIGABRT };
-
-    for (int i = 0; i < juce::numElementsInArray (signals); ++i)
-    {
-        ::signal (signals[i], kill9WithSomeMercy);
-        ::siginterrupt (signals[i], 1);
-    }
-}
-#endif
-
-
 //==============================================================================
 //==============================================================================
 CommandLineValidator::CommandLineValidator()
 {
-   #if JUCE_MAC
-    setupSignalHandling();
-   #endif
 }
 
 CommandLineValidator::~CommandLineValidator()

Source/CrashHandler.cpp

@@ -15,57 +15,134 @@
 #include "juce_core/juce_core.h"
 #include "CrashHandler.h"
 
-#if JUCE_MAC
+#if !JUCE_WINDOWS
  #include <dlfcn.h>
+ #include <execinfo.h>
+ #include <fcntl.h>
+ #include <stdarg.h>
+ #include <stdio.h>
+ #include <unistd.h>
+ #define CRASH_LOG "/tmp/pluginval_crash.txt"
 #endif
 
 namespace
 {
-    juce::String getExtraPlatformSpecificInfo()
-    {
-       #if JUCE_MAC
-        juce::StringArray imagesDone;
-        juce::StringArray output;
-
-        for (auto& l : juce::StringArray::fromLines (juce::SystemStats::getStackBacktrace()))
-        {
-            const juce::String imageName = l.upToFirstOccurrenceOf ("0x", false, false).fromFirstOccurrenceOf (" ", false, false).trim();
-            const juce::String addressString = l.fromFirstOccurrenceOf ("0x", true, false).upToFirstOccurrenceOf (" ", false, false).trim();
-
-            if (imagesDone.contains (imageName))
-                continue;
-
-            imagesDone.add (imageName);
-
-            Dl_info info;
-            const size_t address = static_cast<size_t> (addressString.getHexValue64());
-
-            if (dladdr (reinterpret_cast<void*> (address), &info) != 0)
-                output.add (juce::String ("0x") + juce::String::toHexString (static_cast<juce::pointer_sized_int> (reinterpret_cast<size_t> (info.dli_fbase))) + " " + imageName);
-        }
-
-        return "Binary Images:\n" + output.joinIntoString ("\n");
-       #else
-        return {};
-       #endif
-    }
-
     juce::String getCrashLogContents()
     {
-        return "\n" + juce::SystemStats::getStackBacktrace() + "\n" + getExtraPlatformSpecificInfo();
+        return "\n" + juce::SystemStats::getStackBacktrace();
     }
 
     static juce::File getCrashTraceFile()
     {
+      #if JUCE_WINDOWS
         return juce::File::getSpecialLocation (juce::File::tempDirectory).getChildFile ("pluginval_crash.txt");
+      #else
+        return juce::File(CRASH_LOG);
+      #endif
     }
 
-    static void handleCrash (void*)
+  #if JUCE_WINDOWS
+    static void handleCrash(void*)
     {
         const auto log = getCrashLogContents();
         std::cout << "\n*** FAILED: VALIDATION CRASHED\n" << log << std::endl;
         getCrashTraceFile().replaceWithText (log);
     }
+  #else
+    #ifdef __printflike
+      __printflike(2, 3)
+    #endif
+    static void writeToCrashLog(int fd, const char* fmt, ...)
+    {
+        char buf[1024];
+        va_list args;
+        va_start(args, fmt);
+        // Warning: printf is not 100% async-signal-safe, but should be ok for locale-independent arguments like
+        // integers, strings, hex... floating point is locale-dependent and not safe to use here.
+        vsnprintf(buf, sizeof(buf), fmt, args);
+        va_end(args);
+        auto len = strlen(buf);
+        write(STDERR_FILENO, buf, len);
+        if (fd != -1)
+            write(fd, buf, len);
+    }
+
+    static void handleCrash(void*)
+    {
+        // On Linux & Mac this is a signal handler, and therefore only "async-signal-safe" functions should be used.
+        // This means nothing that uses malloc (juce::File, juce::String, std::string, std::vector etc.) or buffered I/O.
+
+        int fd = open(CRASH_LOG, O_RDWR | O_CREAT | O_TRUNC);
+
+        const char *message = "\n*** FAILED: VALIDATION CRASHED\n";
+        write(STDERR_FILENO, message, strlen(message));
+
+        // Recreate the output of backtrace_symbols(), which cannot be used in a signal handler because it uses malloc
+        static const int kMaxStacks = 128;
+        void *stacktrace[kMaxStacks] {};
+        int stackCount = backtrace(stacktrace, kMaxStacks);
+
+        static const int kMaxImages = 64;
+        const void *imageAddresses[kMaxImages] {};
+        const char *imageNames[kMaxImages] {};
+        int imageCount = 0;
+
+        int skip = 2; // Skip handleCrash and juce::handleCrash)
+        for (int i = skip; i < stackCount; i++)
+        {
+            Dl_info info {};
+            // Warning: dladdr can deadlock under rare conditions on macOS - if dyld is adding an image to its list
+            if (!dladdr(stacktrace[i], &info))
+            {
+                writeToCrashLog(fd, "%-3d %-35s %p\n", i - skip, "", stacktrace[i]);
+                continue;
+            }
+
+            const char *imageName = info.dli_fname ? strrchr(info.dli_fname, '/') : nullptr;
+            if (imageName)
+            {
+                imageName++;
+
+                auto it = std::find(std::begin(imageAddresses), std::end(imageAddresses), info.dli_fbase);
+                if (it == std::end(imageAddresses) && imageCount < kMaxImages)
+                {
+                    imageAddresses[imageCount] = info.dli_fbase;
+                    imageNames[imageCount] = imageName;
+                    imageCount++;
+                }
+            }
+
+            if (info.dli_saddr)
+            {
+                ptrdiff_t offset = (char *)stacktrace[i] - (char *)info.dli_saddr;
+                writeToCrashLog(fd, "%-3d %-35s %p %s + %ld\n", i - skip, imageName, stacktrace[i], info.dli_sname, offset);
+            }
+            else
+            {
+                writeToCrashLog(fd, "%-3d %-35s %p\n", i - skip, imageName, stacktrace[i]);
+            }
+        }
+
+        if (imageCount)
+        {
+            writeToCrashLog(fd, "\nBinary Images:");
+            for (int i = 0; i < imageCount; i++)
+                writeToCrashLog(fd, "\n%p %s", imageAddresses[i], imageNames[i]);
+            writeToCrashLog(fd, "\n");
+        }
+
+        if (fd != -1)
+            close(fd);
+
+        // Terminate normally to work around a bug in juce::ChildProcess::ActiveProcess::getExitStatus()
+        // which returns 0 (a "pass" in the host process) if the child process terminates abnormally.
+        // - https://github.com/Tracktion/pluginval/issues/125
+        // - https://forum.juce.com/t/killed-childprocess-activeprocess-exit-code/61645/3
+        // Use _Exit() instead of exit() so that static destructors don't run (they may not be async-signal-safe).
+        // FIXME: exiting here prevents Apple's Crash Reporter from creating reports.
+        std::_Exit(SIGKILL);
+    }
+  #endif
 }
 
 //==============================================================================

Source/Validator.cpp

@@ -32,6 +32,9 @@ struct PluginsUnitTestRunner    : public juce::UnitTestRunner,
         jassert (callback);
         resetTimeout();
 
+        // Initialise the crash handler to clear any previous crash logs
+        initialiseCrashHandler();
+
         if (timeoutInMs > 0)
             startThread (juce::Thread::Priority::low);
     }