Make the signal handler thing work and add test case

Signed-off-by: Akshita Mavurapu <[email protected]>

api/bag.cpp

@@ -20,7 +20,6 @@
 #include "bag_vrrefinementsdescriptor.h"
 #include "bag_vrtrackinglist.h"
 #include <array>
-#include <csignal>
 #include <cstdio>
 #include <unistd.h>
 
@@ -36,136 +35,13 @@
 #include <sstream>
 #include <string>
 #include <string.h>
-#include <signal.h>
+#include "signal_hook.h"
 
 #ifdef _MSC_VER
 #pragma warning(pop)
 #endif
 
 
-namespace {
-
-static void (*cleanup_functions[128])(int);
-static int cleanup_functions_count = 0;
-
-// Returns 0 on success, 1 if too many functions have been set
-// This is not thread safe, only call it from a single thread
-int add_cleanup_function(void (*f)(int)) {
-    if (cleanup_functions_count >= 128) {
-        return 1;
-    }
-
-    cleanup_functions[cleanup_functions_count] = f;
-    cleanup_functions_count++;
-
-    return 0;
-}
-
-static void bag_handler(int signal);
-
-struct BagAbortHook {
-    //void (*old_handler[255])(int);
-    //void (*current_handler[255])(int);
-    struct sigaction old_handlers[255];
-    struct sigaction current_handlers[255];
-
-    // returns after done printing the exit message
-    void print_error_msg(int signal) {
-        // if the handlers are the same, don't call "bag handler" twice
-        char* signal_name;
-
-        #define TERM_STRING "a critical error occurred within BAG or a dependency (probably libxml or HDF5), exiting to prevent corruption or unexpected behavior\n"\
-                            "the signal that caused this error was: SIG"
-
-#ifdef __USE_GNU
-        const char* signal_abbrev = sigabbrev_np(signal);
-#else
-        const char* signal_abbrev = "<unknown>";
-#endif
-        if (!signal_abbrev) {
-            signal_abbrev = "<bad signal number>";
-        }
-
-        int stderr_fd = stderr->_fileno;
-
-        // strlen is not async signal safe, so it can't be used here
-        int signal_abbrev_length = 0;
-        for(;;signal_abbrev_length++) {
-            if (signal_abbrev[signal_abbrev_length] == 0) {
-                break;
-            }
-        }
-
-        //fputs(", FILE *__restrict stream);
-        write(stderr_fd, TERM_STRING, sizeof (TERM_STRING) - 1);
-        write(stderr_fd, signal_abbrev, signal_abbrev_length);
-    }
-
-    void call_cleanup_functions(int signal) {
-        for(int i = 0; i < cleanup_functions_count; i++) {
-            cleanup_functions[i](signal);
-        }
-    }
-
-    void call_previous_handler(int signal) {
-        if (this->old_handlers[signal].sa_handler != nullptr && this->old_handlers[signal].sa_handler != bag_handler) {
-            this->old_handlers[signal].sa_handler(signal);
-        }
-    }
-
-    void handle(int signal) {
-        this->print_error_msg(signal);
-
-        // if anyone set a cleanup function, call those
-        this->call_cleanup_functions(signal);
-
-        // if there was a handler before this, that isn't the BAG handler,
-        // call that one
-        this->call_previous_handler(signal);
-
-        // if nothing has exited in the previous handlers, exit here
-        _exit(-1);
-    }
-
-    std::array<int, 4> handled_signals() {
-        return { SIGABRT, SIGILL, SIGBUS, SIGFPE };
-    }
-
-    BagAbortHook() {
-        // set these all to null so that when restoring them
-        // we know which ones were added
-        for (int i = 0; i < 255; i++) {
-            this->old_handlers[i].sa_handler = nullptr;
-        }
-
-        struct sigaction old_action;
-        struct sigaction new_action;
-        for (auto sn : this->handled_signals()) {
-            new_action.sa_handler = bag_handler;
-            sigaction(sn, &new_action, &old_action);
-
-            this->old_handlers[sn] = old_action;
-        }
-
-    }
-
-    ~BagAbortHook() {
-        // reset the handlers back to what they were
-        for (auto sn : this->handled_signals()) {
-            struct sigaction old_action = this->old_handlers[sn];
-            sigaction(sn, &old_action, nullptr);
-        }
-    }
-};
-
-static BagAbortHook* current_hook;
-
-static void bag_handler(int signal) {
-    if (current_hook) {
-        current_hook->handle(signal);
-    }
-}
-
 //! Convert a BAG::CompoundDataType (C++) into a BagCompoundDataType (C).
 /*!
 \param field
@@ -237,8 +113,6 @@ BAG::CompoundDataType getValue(
     }
 }
 
-}  // namespace
-
 //! Open the specified BAG.
 /*!
 \param handle

api/signal_hook.h

@@ -0,0 +1,179 @@
+#ifndef BAG_SIGNAL_HOOK
+#define BAG_SIGNAL_HOOK
+
+#include <iostream>
+#include <ostream>
+#include <string>
+#include <string.h>
+#include <signal.h>
+#include <array>
+#include <csignal>
+#include <cstdio>
+#include <unistd.h>
+
+static void (*cleanup_functions[128])(int);
+static int cleanup_functions_count = 0;
+
+// Returns 0 on success, 1 if too many functions have been set
+// This is not thread safe, only call it from a single thread
+int add_cleanup_function(void (*f)(int)) {
+    if (cleanup_functions_count >= 128) {
+        return 1;
+    }
+
+    cleanup_functions[cleanup_functions_count] = f;
+    cleanup_functions_count++;
+
+    return 0;
+}
+
+static void bag_handler(int signal);
+
+struct BagAbortHook;
+
+static BagAbortHook* current_hook;
+
+struct BagAbortHook {
+    BagAbortHook* previous;
+    //void (*old_handler[255])(int);
+    //void (*current_handler[255])(int);
+    struct sigaction old_handlers[255];
+    //void (*old_handlers[255])(int);
+
+    // returns after done printing the exit message
+    void print_error_msg(int signal) {
+        // if the handlers are the same, don't call "bag handler" twice
+        char* signal_name;
+
+        #define TERM_STRING "a critical error occurred within BAG or a dependency (probably libxml or HDF5), exiting to prevent corruption or unexpected behavior\n"\
+                            "the signal that caused this error was: SIG"
+
+#ifdef __USE_GNU
+        const char* signal_abbrev = sigabbrev_np(signal);
+#else
+        const char* signal_abbrev = "<unknown>";
+#endif
+        if (!signal_abbrev) {
+            signal_abbrev = "<bad signal number>";
+        }
+
+        int stderr_fd = stderr->_fileno;
+
+        // strlen is not async signal safe, so it can't be used here
+        int signal_abbrev_length = 0;
+        for(;;signal_abbrev_length++) {
+            if (signal_abbrev[signal_abbrev_length] == 0) {
+                break;
+            }
+        }
+
+        //fputs(", FILE *__restrict stream);
+        write(stderr_fd, TERM_STRING, sizeof (TERM_STRING) - 1);
+        write(stderr_fd, signal_abbrev, signal_abbrev_length);
+        write(stderr_fd, "\n", 1);
+    }
+
+    void call_cleanup_functions(int signal) {
+        //std::cout << "calling cleanup functions" << std::endl;
+        for(int i = 0; i < cleanup_functions_count; i++) {
+            //std::cout << "calling cleanup function:" << i << std::endl;
+            cleanup_functions[i](signal);
+        }
+    }
+
+    void call_previous_handler(int signal) {
+        if (this->old_handlers[signal].sa_handler != nullptr && this->old_handlers[signal].sa_handler != bag_handler) {
+            this->old_handlers[signal].sa_handler(signal);
+        }
+    }
+
+    void handle(int signal) {
+        this->print_error_msg(signal);
+
+        // if anyone set a cleanup function, call those
+        this->call_cleanup_functions(signal);
+
+        // if there was a handler before this, that isn't the BAG handler,
+        // call that one
+
+        this->call_previous_handler(signal);
+
+        // if nothing has exited in the previous handlers, exit here
+        //std::cout << "exiting" << std::endl;
+        _exit(-1);
+    }
+
+    std::array<int, 5> handled_signals() {
+        return { SIGABRT, SIGILL, SIGBUS, SIGFPE, SIGSEGV };
+    }
+
+    BagAbortHook() {
+        std::cout << "made hook" << std::endl;
+        // set these all to null so that when restoring them
+        // we know which ones were added
+        for (int i = 0; i < 255; i++) {
+            this->old_handlers[i].sa_handler = nullptr;
+        }
+
+        struct sigaction old_action;
+        for (auto sn : this->handled_signals()) {
+            //std::cout << "setting action for signal " << sn << std::endl;
+            struct sigaction new_action;
+            new_action.sa_flags = 0;
+            new_action.sa_handler = bag_handler;
+
+            // I was going to use https://en.cppreference.com/w/c/program/signal here
+            // but I needed to get what the old action was so that I could
+            // put the original handler back
+            // https://linux.die.net/man/2/sigaction seems to give me what it used to be
+            // in old action, but I'm not sure if I need to do anything special with the
+            // other things in the sigaction struct?
+            //
+            // Actually, I think it does support it but apparently signal is
+            // pretty outdated: https://stackoverflow.com/questions/231912/what-is-the-difference-between-sigaction-and-signal
+            auto r = sigaction(sn, &new_action, &old_action);
+            //auto old_handler = signal(sn, bag_handler);
+
+            /*if (old_handler == SIG_ERR) {
+                std::cout << "got SIG_ERR setting signal handler?" << std::endl;
+            } else if (old_handler == SIG_DFL) {
+                std::cout << "old handler was DFL" << std::endl;
+            } else if (old_handler == SIG_IGN) {
+                std::cout << "old handler was IGN" << std::endl;
+            }*/
+
+            //std::cout << "setting action returned:" << old_handler << std::endl;
+
+            this->old_handlers[sn] = old_action;
+        }
+        this->previous = current_hook;
+        current_hook = this;
+    }
+
+    ~BagAbortHook() {
+        std::cout << "resetting to original handlers" << std::endl;
+        // reset the handlers back to what they were
+        for (auto sn : this->handled_signals()) {
+            struct sigaction old_action = this->old_handlers[sn];
+            //auto old_action = this->old_handlers[sn];
+            //signal(sn, old_action);
+            sigaction(sn, &old_action, nullptr);
+        }
+
+        current_hook = this->previous;
+    }
+};
+
+static void bag_handler(int signal) {
+    //std::cout << "handler" << std::endl;
+    if (current_hook) {
+        current_hook->handle(signal);
+    } else {
+        // something broke I think?
+        // I don't know if there's a way to deal with this
+        // so I'll just exit here
+        _exit(-2);
+    }
+}
+
+#endif

examples/driver.cpp

@@ -1,7 +1,8 @@
+
+#include <string>
 #include <cstring>
 #include <iostream>
 #include <fstream>
-#include <string>
 #include <ios>
 #include <vector>
 #include <stdint.h>
@@ -40,4 +41,4 @@ int main(int argc, char** argv)//argv, which is an array of pointers to strings
     //strcpy(filename_cstr, filename.c_str());
 
     LLVMFuzzerTestOneInputByFile(filename.c_str());
-}
+}

examples/test_signal_hook.cpp

@@ -0,0 +1,33 @@
+#include "../api/signal_hook.h"
+#include <csignal>
+#include <cstdlib>
+#include <iostream>
+#include <ostream>
+
+void cleanuper(int signal) {
+    std::cout << "cleanuper" << std::endl;
+}
+
+void existing_handler(int signal) {
+    std::cout << "existing handler" << std::endl;
+}
+
+int main(int argc, char** argv) {
+    signal(SIGSEGV, existing_handler);
+
+    BagAbortHook hook;
+
+    add_cleanup_function(cleanuper);
+
+    std::cout << "signal going to happen" << std::endl;
+
+    // this causes the segfault, but not sure if compiler might do something else?
+    // this is just a test, so could maybe make it do sigabrt instead?
+    volatile int* p = nullptr;
+
+    int v = *p;
+
+    std::cout << "v is:" << v << std::endl;
+
+    std::cout << "this shouldn't print" << std::endl;
+}