[ExecutionEngine] Make RuntimeDyld::MemoryManager responsible for tracking EH
authorLang Hames <lhames@gmail.com>
Tue, 9 May 2017 21:32:18 +0000 (21:32 +0000)
committerLang Hames <lhames@gmail.com>
Tue, 9 May 2017 21:32:18 +0000 (21:32 +0000)
frames.

RuntimeDyld was previously responsible for tracking allocated EH frames, but it
makes more sense to have the RuntimeDyld::MemoryManager track them (since the
frames are allocated through the memory manager, and written to memory owned by
the memory manager). This patch moves the frame tracking into
RTDyldMemoryManager, and changes the deregisterFrames method on
RuntimeDyld::MemoryManager from:

void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr, size_t Size);

to:

void deregisterEHFrames();

Separating this responsibility will allow ORC to continue to throw the
RuntimeDyld instances away post-link (saving a few dozen bytes per lazy
function) while properly deregistering frames when modules are unloaded.

This patch also updates ORC to call deregisterEHFrames when modules are
unloaded. This fixes a bug where an exception that tears down the JIT can then
unwind through dangling EH frames that have been deallocated but not
deregistered, resulting in UB.

For people using SectionMemoryManager this should be pretty much a no-op. For
people with custom allocators that override registerEHFrames/deregisterEHFrames,
you will now be responsible for tracking allocated EH frames.

Reviewed in https://reviews.llvm.org/D32829

llvm-svn: 302589

18 files changed:
llvm/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
llvm/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
llvm/include/llvm/ExecutionEngine/RTDyldMemoryManager.h
llvm/include/llvm/ExecutionEngine/RuntimeDyld.h
llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h
llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFThumb.h
llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFX86_64.h
llvm/tools/lli/RemoteJITUtils.h
llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
llvm/unittests/ExecutionEngine/Orc/ObjectTransformLayerTest.cpp
llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp

index 7e7f735..1bb911d 100644 (file)
@@ -172,6 +172,11 @@ private:
       return nullptr;
     }
 
+    void removeModulesFromBaseLayer(BaseLayerT &BaseLayer) {
+      for (auto &BLH : BaseLayerHandles)
+        BaseLayer.removeModuleSet(BLH);
+    }
+
     std::unique_ptr<JITSymbolResolver> ExternalSymbolResolver;
     std::unique_ptr<ResourceOwner<RuntimeDyld::MemoryManager>> MemMgr;
     std::unique_ptr<IndirectStubsMgrT> StubsMgr;
@@ -204,6 +209,11 @@ public:
         CreateIndirectStubsManager(std::move(CreateIndirectStubsManager)),
         CloneStubsIntoPartitions(CloneStubsIntoPartitions) {}
 
+  ~CompileOnDemandLayer() {
+    while (!LogicalDylibs.empty())
+      removeModuleSet(LogicalDylibs.begin());
+  }
+  
   /// @brief Add a module to the compile-on-demand layer.
   template <typename ModuleSetT, typename MemoryManagerPtrT,
             typename SymbolResolverPtrT>
@@ -239,6 +249,7 @@ public:
   ///   This will remove all modules in the layers below that were derived from
   /// the module represented by H.
   void removeModuleSet(ModuleSetHandleT H) {
+    H->removeModulesFromBaseLayer(BaseLayer);
     LogicalDylibs.erase(H);
   }
 
@@ -478,6 +489,8 @@ private:
         return 0;
     }
 
+    LD.BaseLayerHandles.push_back(PartH);
+
     return CalledAddr;
   }
 
index 02f59d6..a19c306 100644 (file)
@@ -144,16 +144,16 @@ public:
 
     void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr,
                           size_t Size) override {
-      UnfinalizedEHFrames.push_back(
-          std::make_pair(LoadAddr, static_cast<uint32_t>(Size)));
+      UnfinalizedEHFrames.push_back({LoadAddr, Size});
     }
 
-    void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr,
-                            size_t Size) override {
-      auto Err = Client.deregisterEHFrames(LoadAddr, Size);
-      // FIXME: Add error poll.
-      assert(!Err && "Failed to register remote EH frames.");
-      (void)Err;
+    void deregisterEHFrames() override {
+      for (auto &Frame : RegisteredEHFrames) {
+        auto Err = Client.deregisterEHFrames(Frame.Addr, Frame.Size);
+        // FIXME: Add error poll.
+        assert(!Err && "Failed to register remote EH frames.");
+        (void)Err;
+      }
     }
 
     void notifyObjectLoaded(RuntimeDyld &Dyld,
@@ -320,7 +320,7 @@ public:
       Unfinalized.clear();
 
       for (auto &EHFrame : UnfinalizedEHFrames) {
-        if (auto Err = Client.registerEHFrames(EHFrame.first, EHFrame.second)) {
+        if (auto Err = Client.registerEHFrames(EHFrame.Addr, EHFrame.Size)) {
           // FIXME: Replace this once finalizeMemory can return an Error.
           handleAllErrors(std::move(Err), [&](ErrorInfoBase &EIB) {
             if (ErrMsg) {
@@ -331,7 +331,8 @@ public:
           return false;
         }
       }
-      UnfinalizedEHFrames.clear();
+      RegisteredEHFrames = std::move(UnfinalizedEHFrames);
+      UnfinalizedEHFrames = {};
 
       return false;
     }
@@ -387,7 +388,13 @@ public:
     ResourceIdMgr::ResourceId Id;
     std::vector<ObjectAllocs> Unmapped;
     std::vector<ObjectAllocs> Unfinalized;
-    std::vector<std::pair<uint64_t, uint32_t>> UnfinalizedEHFrames;
+
+    struct EHFrame {
+      JITTargetAddress Addr;
+      uint64_t Size;
+    };
+    std::vector<EHFrame> UnfinalizedEHFrames;
+    std::vector<EHFrame> RegisteredEHFrames;
   };
 
   /// Remote indirect stubs manager.
index babcc7f..5b3426a 100644 (file)
@@ -120,6 +120,10 @@ private:
       buildInitialSymbolTable(PFC->Objects);
     }
 
+    ~ConcreteLinkedObjectSet() override {
+      MemMgr->deregisterEHFrames();
+    }
+    
     void setHandle(ObjSetHandleT H) {
       PFC->Handle = H;
     }
index 5638717..74535fe 100644 (file)
@@ -69,13 +69,8 @@ public:
   /// Deregister EH frames in the current proces.
   static void deregisterEHFramesInProcess(uint8_t *Addr, size_t Size);
 
-  void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr, size_t Size) override {
-    registerEHFramesInProcess(Addr, Size);
-  }
-
-  void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr, size_t Size) override {
-    deregisterEHFramesInProcess(Addr, Size);
-  }
+  void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr, size_t Size) override;
+  void deregisterEHFrames() override;
 
   /// This method returns the address of the specified function or variable in
   /// the current process.
@@ -139,6 +134,13 @@ public:
   /// MCJIT or RuntimeDyld.  Use getSymbolAddress instead.
   virtual void *getPointerToNamedFunction(const std::string &Name,
                                           bool AbortOnFailure = true);
+
+private:
+  struct EHFrame {
+    uint8_t *Addr;
+    size_t Size;
+  };
+  std::vector<EHFrame> EHFrames;
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
index 13a5f99..9470866 100644 (file)
@@ -150,8 +150,7 @@ public:
     /// be the case for local execution) these two values will be the same.
     virtual void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr,
                                   size_t Size) = 0;
-    virtual void deregisterEHFrames(uint8_t *addr, uint64_t LoadAddr,
-                                    size_t Size) = 0;
+    virtual void deregisterEHFrames() = 0;
 
     /// This method is called when object loading is complete and section page
     /// permissions can be applied.  It is up to the memory manager implementation
index a5100a5..a27573f 100644 (file)
@@ -94,9 +94,8 @@ class OrcMCJITReplacement : public ExecutionEngine {
       return ClientMM->registerEHFrames(Addr, LoadAddr, Size);
     }
 
-    void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr,
-                            size_t Size) override {
-      return ClientMM->deregisterEHFrames(Addr, LoadAddr, Size);
+    void deregisterEHFrames() override {
+      return ClientMM->deregisterEHFrames();
     }
 
     void notifyObjectLoaded(RuntimeDyld &RTDyld,
index de73fbd..99e84b7 100644 (file)
@@ -134,6 +134,18 @@ void RTDyldMemoryManager::deregisterEHFramesInProcess(uint8_t *Addr,
 
 #endif
 
+void RTDyldMemoryManager::registerEHFrames(uint8_t *Addr, uint64_t LoadAddr,
+                                          size_t Size) {
+  registerEHFramesInProcess(Addr, Size);
+  EHFrames.push_back({Addr, Size});
+}
+
+void RTDyldMemoryManager::deregisterEHFrames() {
+  for (auto &Frame : EHFrames)
+    deregisterEHFramesInProcess(Frame.Addr, Frame.Size);
+  EHFrames.clear();
+}
+
 static int jit_noop() {
   return 0;
 }
index df9d2ce..e9a4b71 100644 (file)
@@ -73,7 +73,9 @@ namespace llvm {
 
 void RuntimeDyldImpl::registerEHFrames() {}
 
-void RuntimeDyldImpl::deregisterEHFrames() {}
+void RuntimeDyldImpl::deregisterEHFrames() {
+  MemMgr.deregisterEHFrames();
+}
 
 #ifndef NDEBUG
 static void dumpSectionMemory(const SectionEntry &S, StringRef State) {
index 2725d8a..6608437 100644 (file)
@@ -221,22 +221,10 @@ void RuntimeDyldELF::registerEHFrames() {
     uint64_t EHFrameLoadAddr = Sections[EHFrameSID].getLoadAddress();
     size_t EHFrameSize = Sections[EHFrameSID].getSize();
     MemMgr.registerEHFrames(EHFrameAddr, EHFrameLoadAddr, EHFrameSize);
-    RegisteredEHFrameSections.push_back(EHFrameSID);
   }
   UnregisteredEHFrameSections.clear();
 }
 
-void RuntimeDyldELF::deregisterEHFrames() {
-  for (int i = 0, e = RegisteredEHFrameSections.size(); i != e; ++i) {
-    SID EHFrameSID = RegisteredEHFrameSections[i];
-    uint8_t *EHFrameAddr = Sections[EHFrameSID].getAddress();
-    uint64_t EHFrameLoadAddr = Sections[EHFrameSID].getLoadAddress();
-    size_t EHFrameSize = Sections[EHFrameSID].getSize();
-    MemMgr.deregisterEHFrames(EHFrameAddr, EHFrameLoadAddr, EHFrameSize);
-  }
-  RegisteredEHFrameSections.clear();
-}
-
 std::unique_ptr<RuntimeDyldELF>
 llvm::RuntimeDyldELF::create(Triple::ArchType Arch,
                              RuntimeDyld::MemoryManager &MemMgr,
index 84dd810..fb5da6d 100644 (file)
@@ -152,7 +152,6 @@ private:
   // in a table until we receive a request to register all unregistered
   // EH frame sections with the memory manager.
   SmallVector<SID, 2> UnregisteredEHFrameSections;
-  SmallVector<SID, 2> RegisteredEHFrameSections;
 
   // Map between GOT relocation value and corresponding GOT offset
   std::map<RelocationValueRef, uint64_t> GOTOffsetMap;
@@ -180,7 +179,6 @@ public:
                        StubMap &Stubs) override;
   bool isCompatibleFile(const object::ObjectFile &Obj) const override;
   void registerEHFrames() override;
-  void deregisterEHFrames() override;
   Error finalizeLoad(const ObjectFile &Obj,
                      ObjSectionToIDMap &SectionMap) override;
 };
index f5cc883..18c23c5 100644 (file)
@@ -515,7 +515,7 @@ public:
 
   virtual void registerEHFrames();
 
-  virtual void deregisterEHFrames();
+  void deregisterEHFrames();
 
   virtual Error finalizeLoad(const ObjectFile &ObjImg,
                              ObjSectionToIDMap &SectionMap) {
index 109beb3..26e7398 100644 (file)
@@ -194,9 +194,6 @@ public:
     }
     UnregisteredEHFrameSections.clear();
   }
-  void deregisterEHFrames() override {
-    // Stub
-  }
   Error finalizeLoad(const ObjectFile &Obj,
                      ObjSectionToIDMap &SectionMap) override {
     // Look for and record the EH frame section IDs.
index 89a5142..3c82f73 100644 (file)
@@ -118,9 +118,8 @@ public:
     MemMgr->registerEHFrames(Addr, LoadAddr, Size);
   }
 
-  void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr,
-                          size_t Size) override {
-    MemMgr->deregisterEHFrames(Addr, LoadAddr, Size);
+  void deregisterEHFrames() override {
+    MemMgr->deregisterEHFrames();
   }
 
   bool finalizeMemory(std::string *ErrMsg = nullptr) override {
index 75345de..ba130ce 100644 (file)
@@ -175,8 +175,7 @@ public:
 
   void registerEHFrames(uint8_t *Addr, uint64_t LoadAddr,
                         size_t Size) override {}
-  void deregisterEHFrames(uint8_t *Addr, uint64_t LoadAddr,
-                          size_t Size) override {}
+  void deregisterEHFrames() override {}
 
   void preallocateSlab(uint64_t Size) {
     std::string Err;
index 96214a3..362c143 100644 (file)
@@ -304,7 +304,7 @@ TEST(ObjectTransformLayerTest, Main) {
       return nullptr;
     }
     void registerEHFrames(uint8_t *, uint64_t, size_t) override {}
-    void deregisterEHFrames(uint8_t *, uint64_t, size_t) override {}
+    void deregisterEHFrames() override {}
     bool finalizeMemory(std::string *) override { return false; }
   };
 
index de99c02..c13a75a 100644 (file)
@@ -90,7 +90,8 @@ TEST(RTDyldObjectLinkingLayerTest, TestSetProcessAllSections) {
   Objs.push_back(OwningObj.getBinary());
 
   bool DebugSectionSeen = false;
-  SectionMemoryManagerWrapper SMMW(DebugSectionSeen);
+  auto SMMW =
+    std::make_shared<SectionMemoryManagerWrapper>(DebugSectionSeen);
   auto Resolver =
     createLambdaResolver(
       [](const std::string &Name) {
@@ -102,7 +103,7 @@ TEST(RTDyldObjectLinkingLayerTest, TestSetProcessAllSections) {
 
   {
     // Test with ProcessAllSections = false (the default).
-    auto H = ObjLayer.addObjectSet(Objs, &SMMW, &*Resolver);
+    auto H = ObjLayer.addObjectSet(Objs, SMMW, &*Resolver);
     ObjLayer.emitAndFinalize(H);
     EXPECT_EQ(DebugSectionSeen, false)
       << "Unexpected debug info section";
@@ -112,7 +113,7 @@ TEST(RTDyldObjectLinkingLayerTest, TestSetProcessAllSections) {
   {
     // Test with ProcessAllSections = true.
     ObjLayer.setProcessAllSections(true);
-    auto H = ObjLayer.addObjectSet(Objs, &SMMW, &*Resolver);
+    auto H = ObjLayer.addObjectSet(Objs, SMMW, &*Resolver);
     ObjLayer.emitAndFinalize(H);
     EXPECT_EQ(DebugSectionSeen, true)
       << "Expected debug info section not seen";
@@ -178,14 +179,15 @@ TEST_F(RTDyldObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {
         return JITSymbol(nullptr);
       });
 
-  SectionMemoryManagerWrapper SMMW;
-  ObjLayer.addObjectSet(std::move(Obj1Set), &SMMW, &*Resolver);
-  auto H = ObjLayer.addObjectSet(std::move(Obj2Set), &SMMW, &*Resolver);
+  auto SMMW = std::make_shared<SectionMemoryManagerWrapper>();
+  ObjLayer.addObjectSet(std::move(Obj1Set), SMMW, &*Resolver);
+  auto H = ObjLayer.addObjectSet(std::move(Obj2Set), SMMW, &*Resolver);
   ObjLayer.emitAndFinalize(H);
-
+  ObjLayer.removeObjectSet(H);
+  
   // Finalization of module 2 should trigger finalization of module 1.
   // Verify that finalize on SMMW is only called once.
-  EXPECT_EQ(SMMW.FinalizationCount, 1)
+  EXPECT_EQ(SMMW->FinalizationCount, 1)
       << "Extra call to finalize";
 }
 
@@ -238,14 +240,15 @@ TEST_F(RTDyldObjectLinkingLayerExecutionTest, NoPrematureAllocation) {
   std::vector<object::ObjectFile*> Obj2Set;
   Obj2Set.push_back(Obj2.getBinary());
 
-  SectionMemoryManagerWrapper SMMW;
+  auto SMMW = std::make_shared<SectionMemoryManagerWrapper>();
   NullResolver NR;
-  auto H = ObjLayer.addObjectSet(std::move(Obj1Set), &SMMW, &NR);
-  ObjLayer.addObjectSet(std::move(Obj2Set), &SMMW, &NR);
+  auto H = ObjLayer.addObjectSet(std::move(Obj1Set), SMMW, &NR);
+  ObjLayer.addObjectSet(std::move(Obj2Set), SMMW, &NR);
   ObjLayer.emitAndFinalize(H);
-
+  ObjLayer.removeObjectSet(H);
+  
   // Only one call to needsToReserveAllocationSpace should have been made.
-  EXPECT_EQ(SMMW.NeedsToReserveAllocationSpaceCount, 1)
+  EXPECT_EQ(SMMW->NeedsToReserveAllocationSpaceCount, 1)
       << "More than one call to needsToReserveAllocationSpace "
          "(multiple unrelated objects loaded prior to finalization)";
 }