EH: correctly deregister EH frames and cleanup the code

- previously we never deregistered the EH frame from libunwind (which exposed a bug)
- I removed the calls to register the EH info with gcc since we have our own unwinder which does not need this
- cleaned up the code and fixed some small memory leaks

src/codegen/baseline_jit.cpp

@@ -104,19 +104,12 @@ JitCodeBlock::JitCodeBlock(llvm::StringRef name)
     entry_offset = a.bytesWritten();
 
     // generate the eh frame...
-    const int size = sizeof(eh_info);
+    const int eh_frame_size = sizeof(eh_info);
     void* eh_frame_addr = memory.get();
-    memcpy(eh_frame_addr, eh_info, size);
+    memcpy(eh_frame_addr, eh_info, eh_frame_size);
 
-    int32_t* offset_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x20);
-    int32_t* size_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x24);
-    int64_t offset = (int8_t*)code - (int8_t*)offset_ptr;
-    assert(offset >= INT_MIN && offset <= INT_MAX);
-    *offset_ptr = offset;
-    *size_ptr = code_size;
-
-    registerDynamicEhFrame((uint64_t)code, code_size, (uint64_t)eh_frame_addr, size - 4);
-    registerEHFrames((uint8_t*)eh_frame_addr, (uint64_t)eh_frame_addr, size);
+    register_eh_info.updateAndRegisterFrameFromTemplate((uint64_t)code, code_size, (uint64_t)eh_frame_addr,
+                                                        eh_frame_size);
 
     static int num_block = 0;
     auto unique_name = ("bjit_" + name + "_" + llvm::Twine(num_block++)).str();

src/codegen/baseline_jit.h

@@ -177,6 +177,7 @@ private:
     // this contains all the decref infos the bjit generated inside the memory block,
     // this allows us to deregister them when we release the code
     std::vector<DecrefInfo> decref_infos;
+    RegisterEHFrame register_eh_info;
 
 public:
     JitCodeBlock(llvm::StringRef name);

src/codegen/memmgr.cpp

@@ -241,14 +241,4 @@ PystonMemoryManager::~PystonMemoryManager() {
 std::unique_ptr<llvm::RTDyldMemoryManager> createMemoryManager() {
     return std::unique_ptr<llvm::RTDyldMemoryManager>(new PystonMemoryManager());
 }
-
-// These functions exist as instance methods of the RTDyldMemoryManager class,
-// but it's tricky to access them since the class has pure-virtual methods.
-void registerEHFrames(uint8_t* addr, uint64_t load_addr, size_t size) {
-    PystonMemoryManager().registerEHFrames(addr, load_addr, size);
-}
-
-void deregisterEHFrames(uint8_t* addr, uint64_t load_addr, size_t size) {
-    PystonMemoryManager().deregisterEHFrames(addr, load_addr, size);
-}
 }

src/codegen/memmgr.h

@@ -26,8 +26,6 @@ class RTDyldMemoryManager;
 namespace pyston {
 
 std::unique_ptr<llvm::RTDyldMemoryManager> createMemoryManager();
-void registerEHFrames(uint8_t* addr, uint64_t load_addr, size_t size);
-void deregisterEHFrames(uint8_t* addr, uint64_t load_addr, size_t size);
 }
 
 #endif

src/codegen/unwinding.cpp

@@ -94,7 +94,7 @@ void parseEhFrame(uint64_t start_addr, uint64_t size, uint64_t func_addr, uint64
     *out_len = nentries;
 }
 
-void registerDynamicEhFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr, size_t eh_frame_size) {
+void* registerDynamicEhFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr, size_t eh_frame_size) {
     unw_dyn_info_t* dyn_info = new unw_dyn_info_t();
     dyn_info->start_ip = code_addr;
     dyn_info->end_ip = code_addr + code_size;
@@ -116,6 +116,42 @@ void registerDynamicEhFrame(uint64_t code_addr, size_t code_size, uint64_t eh_fr
     // as opposed to the binary search it can do within a dyn_info.
     // If we're registering a lot of dyn_info's, it might make sense to coalesce them into a single
     // dyn_info that contains a binary search table.
+    return dyn_info;
+}
+
+void deregisterDynamicEhFrame(void* _dyn_info) {
+    auto dyn_info = (unw_dyn_info_t*)_dyn_info;
+    _U_dyn_cancel(dyn_info);
+    delete (uw_table_entry*)dyn_info->u.rti.table_data;
+    delete dyn_info;
+}
+
+void RegisterEHFrame::updateAndRegisterFrameFromTemplate(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr,
+                                                         size_t eh_frame_size) {
+    assert(eh_frame_size > 0x24);
+    int32_t* offset_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x20);
+    int32_t* size_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x24);
+    int64_t offset = (int8_t*)code_addr - (int8_t*)offset_ptr;
+    assert(offset >= INT_MIN && offset <= INT_MAX);
+    *offset_ptr = offset;
+    *size_ptr = code_size;
+
+    // (EH_FRAME_SIZE - 4) to omit the 4-byte null terminator, otherwise we trip an assert in parseEhFrame.
+    // TODO: can we omit the terminator in general?
+    registerFrame(code_addr, code_size, eh_frame_addr, eh_frame_size - 4);
+}
+
+void RegisterEHFrame::registerFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr,
+                                    size_t eh_frame_size) {
+    assert(!dyn_info);
+    dyn_info = registerDynamicEhFrame(code_addr, code_size, eh_frame_addr, eh_frame_size);
+}
+
+void RegisterEHFrame::deregisterFrame() {
+    if (dyn_info) {
+        deregisterDynamicEhFrame(dyn_info);
+        dyn_info = NULL;
+    }
 }
 
 struct compare_cf {

src/codegen/unwinding.h

@@ -30,7 +30,24 @@ class BoxedDict;
 class BoxedModule;
 struct FrameInfo;
 
-void registerDynamicEhFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr, size_t eh_frame_size);
+class RegisterEHFrame {
+private:
+    void* dyn_info;
+
+public:
+    RegisterEHFrame() : dyn_info(NULL) {}
+    ~RegisterEHFrame() { deregisterFrame(); }
+
+    // updates the EH info at eh_frame_addr to reference the passed code addr and code size and registers it
+    void updateAndRegisterFrameFromTemplate(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr,
+                                            size_t eh_frame_size);
+
+    void registerFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr, size_t eh_frame_size);
+    void deregisterFrame();
+};
+
+void* registerDynamicEhFrame(uint64_t code_addr, size_t code_size, uint64_t eh_frame_addr, size_t eh_frame_size);
+void deregisterDynamicEhFrame(void* dyn_info);
 uint64_t getCXXUnwindSymbolAddress(llvm::StringRef sym);
 
 // use this instead of std::uncaught_exception.

src/runtime/ics.cpp

@@ -22,7 +22,6 @@
 #include "codegen/memmgr.h"
 #include "codegen/patchpoints.h"
 #include "codegen/stackmaps.h"
-#include "codegen/unwinding.h" // registerDynamicEhFrame
 #include "core/common.h"
 #include "core/options.h"
 #include "core/stats.h"
@@ -161,23 +160,6 @@ static constexpr int _eh_frame_template_fp_size = sizeof(_eh_frame_template_fp)
 
 static_assert(sizeof("") == 1, "strings are null-terminated");
 
-static void writeTrivialEhFrame(void* eh_frame_addr, void* func_addr, uint64_t func_size, bool omit_frame_pointer) {
-    if (omit_frame_pointer)
-        memcpy(eh_frame_addr, _eh_frame_template_ofp, _eh_frame_template_ofp_size);
-    else
-        memcpy(eh_frame_addr, _eh_frame_template_fp, _eh_frame_template_fp_size);
-
-    int32_t* offset_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x20);
-    int32_t* size_ptr = (int32_t*)((uint8_t*)eh_frame_addr + 0x24);
-
-    int64_t offset = (int8_t*)func_addr - (int8_t*)offset_ptr;
-    RELEASE_ASSERT(offset >= INT_MIN && offset <= INT_MAX, "");
-    *offset_ptr = offset;
-
-    assert(func_size <= UINT_MAX);
-    *size_ptr = func_size;
-}
-
 #if RUNTIMEICS_OMIT_FRAME_PTR
 // If you change this, you *must* update the value in _eh_frame_template
 // (set the -9'th byte to this value plus 8)
@@ -309,11 +291,13 @@ RuntimeIC::RuntimeIC(void* func_addr, int total_size) {
         assert(!epilogue_assem.hasFailed());
         assert(epilogue_assem.isExactlyFull());
 
-        writeTrivialEhFrame(eh_frame_addr, addr, total_code_size, RUNTIMEICS_OMIT_FRAME_PTR);
-        // (EH_FRAME_SIZE - 4) to omit the 4-byte null terminator, otherwise we trip an assert in parseEhFrame.
-        // TODO: can we omit the terminator in general?
-        registerDynamicEhFrame((uint64_t)addr, total_code_size, (uint64_t)eh_frame_addr, EH_FRAME_SIZE - 4);
-        registerEHFrames((uint8_t*)eh_frame_addr, (uint64_t)eh_frame_addr, EH_FRAME_SIZE);
+
+        if (RUNTIMEICS_OMIT_FRAME_PTR)
+            memcpy(eh_frame_addr, _eh_frame_template_ofp, _eh_frame_template_ofp_size);
+        else
+            memcpy(eh_frame_addr, _eh_frame_template_fp, _eh_frame_template_fp_size);
+        register_eh_frame.updateAndRegisterFrameFromTemplate((uint64_t)addr, total_code_size, (uint64_t)eh_frame_addr,
+                                                             EH_FRAME_SIZE);
     } else {
         addr = func_addr;
     }
@@ -321,8 +305,8 @@ RuntimeIC::RuntimeIC(void* func_addr, int total_size) {
 
 RuntimeIC::~RuntimeIC() {
     if (ENABLE_RUNTIME_ICS) {
+        register_eh_frame.deregisterFrame();
         uint8_t* eh_frame_addr = (uint8_t*)addr - EH_FRAME_SIZE;
-        deregisterEHFrames(eh_frame_addr, (uint64_t)eh_frame_addr, EH_FRAME_SIZE);
         memory_manager_512b.dealloc(eh_frame_addr);
     } else {
     }

src/runtime/ics.h

@@ -15,6 +15,7 @@
 #ifndef PYSTON_RUNTIME_ICS_H
 #define PYSTON_RUNTIME_ICS_H
 
+#include "codegen/unwinding.h" // RegisterEHFrame
 #include "core/common.h"
 #include "runtime/objmodel.h"
 
@@ -26,6 +27,7 @@ class RuntimeIC {
 private:
     void* addr; // points to function start not the start of the allocated memory block.
 
+    RegisterEHFrame register_eh_frame;
     std::unique_ptr<ICInfo> icinfo;
 
     RuntimeIC(const RuntimeIC&) = delete;