[ExecutionEngine] Track objects using an abstract ObjectKey in JITEventListeners.

[lhames]

Summary

The aim of this patch is to enable JITEventListeners to work with ORC without
forcing us to keep object files alive longer than necessary.

Currently JITEventListener::NotifyObjectEmitted and NotifyObjectFreed take a
reference to the object file being emitted, but in NotifyObjectFreed this is
only ever used as a key to deallocate resources that were allocated in
NotifyObjectEmitted. Switching to an abstract key allows the JIT to deallocate
object file buffers after NotifyObjectEmitted has been called.

No guarantee is made about the key value, except that it will be distinct for
each object that has been allocated. Keys may be re-used after they are freed.

[lhames]

• [ExecutionEngine] Rename/format JITEventListener methods.

[anarazel]

This looks like a sensible improvement. There's probably not many implementors of the interface, so the renaming shouldn't cause much pain.

I noticed however that you didn't update ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp?

lib/ExecutionEngine/MCJIT/MCJIT.cpp

654

Perhaps this should rather live inside ObjectFile? No strong opinion, just pondering. Seems a bit prettier than having the ugly multiple cast in multiple places.

[lhames]

In D53773#1280967, @anarazel wrote:

This looks like a sensible improvement. There's probably not many implementors of the interface, so the renaming shouldn't cause much pain.

I noticed however that you didn't update ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp?

Oops -- I totally missed that. I will fix it and post an updated patch tomorrow.

lib/ExecutionEngine/MCJIT/MCJIT.cpp

654

Unfortunately this can not be moved to ObjectFile: For MCJIT it makes sense to continue using the original ObjectFile address as the key, but for ORC the aim is to decouple the key from the ObjectFile so that the latter can be deallocated.

[andrew.w.kaylor]

include/llvm/ExecutionEngine/JITEventListener.h

53

The comment is still using the old name here.

[lhames]

Updates the PerfJITEventListener API and fixes a comment that referred to the old API.

[lhames]

Gentle ping.

This is not high priority yet, but if there are no further issues with it I would like to land it as I intend to build on it in the future.

[anarazel]

Gentle ping.

This is not high priority yet, but if there are no further issues with it I would like to land it as I intend to build on it in the future.

Sorry, missed the updated version. I don't know if my ack is sufficient to move this forward, but the update certainly fixes the complaint I had. And the rest looks reasonable.

[andrew.w.kaylor]

Looks good to me too.

[lhames]

Committed in r348223.

include/llvm/ExecutionEngine/JITEventListener.h

 
 /// JITEventListener - Abstract interface for use by the JIT to notify clients
 /// about significant events during compilation. For example, to notify
 /// profilers and debuggers that need to know where functions have been emitted.
 ///
 /// The default implementation of each method does nothing.
 class JITEventListener {
 public:
+  using ObjectKey = uint64_t;
+
   JITEventListener() = default;
   virtual ~JITEventListener() = default;
 
   /// NotifyObjectEmitted - Called after an object has been successfully
   /// emitted to memory.  NotifyFunctionEmitted will not be called for
   /// individual functions in the object.
   ///
   /// ELF-specific information
   /// The ObjectImage contains the generated object image
   /// with section headers updated to reflect the address at which sections
   /// were loaded and with relocations performed in-place on debug sections.
-  virtual void NotifyObjectEmitted(const object::ObjectFile &Obj,
+  virtual void NotifyObjectEmitted(ObjectKey K, const object::ObjectFile &Obj,
                                    const RuntimeDyld::LoadedObjectInfo &L) {}
 
   /// NotifyFreeingObject - Called just before the memory associated with
   /// a previously emitted object is released.
-  virtual void NotifyFreeingObject(const object::ObjectFile &Obj) {}
+  virtual void NotifyFreeingObject(ObjectKey K) {}
 
   // Get a pointe to the GDB debugger registration listener.
   static JITEventListener *createGDBRegistrationListener();
 
 #if LLVM_USE_INTEL_JITEVENTS
   // Construct an IntelJITEventListener
   static JITEventListener *createIntelJITEventListener();
 

lib/ExecutionEngine/GDBRegistrationListener.cpp

RegisteredObjectInfo(std::size_t Size, jit_code_entry *Entry,
     : Size(Size), Entry(Entry), Obj(std::move(Obj)) {}
 
   std::size_t Size;
   jit_code_entry *Entry;
   OwningBinary<ObjectFile> Obj;
 };
 
 // Buffer for an in-memory object file in executable memory
-typedef llvm::DenseMap< const char*, RegisteredObjectInfo>
+typedef llvm::DenseMap<JITEventListener::ObjectKey, RegisteredObjectInfo>
     RegisteredObjectBufferMap;
 
 /// Global access point for the JIT debugging interface designed for use with a
 /// singleton toolbox. Handles thread-safe registration and deregistration of
 /// object files that are in executable memory managed by the client of this
 /// class.
 class GDBJITRegistrationListener : public JITEventListener {
   /// A map of in-memory object files that have been registered with the
   /// JIT interface.
   RegisteredObjectBufferMap ObjectBufferMap;
 
 public:
   /// Instantiates the JIT service.
   GDBJITRegistrationListener() : ObjectBufferMap() {}
 
   /// Unregisters each object that was previously registered and releases all
   /// internal resources.
   ~GDBJITRegistrationListener() override;
 
   /// Creates an entry in the JIT registry for the buffer @p Object,
   /// which must contain an object file in executable memory with any
   /// debug information for the debugger.
-  void NotifyObjectEmitted(const ObjectFile &Object,
+  void NotifyObjectEmitted(ObjectKey K, const ObjectFile &Obj,
                            const RuntimeDyld::LoadedObjectInfo &L) override;
 
   /// Removes the internal registration of @p Object, and
   /// frees associated resources.
   /// Returns true if @p Object was found in ObjectBufferMap.
-  void NotifyFreeingObject(const ObjectFile &Object) override;
+  void NotifyFreeingObject(ObjectKey K) override;
 
 private:
   /// Deregister the debug info for the given object file from the debugger
   /// and delete any temporary copies.  This private method does not remove
   /// the function from Map so that it can be called while iterating over Map.
   void deregisterObjectInternal(RegisteredObjectBufferMap::iterator I);
 };
 
for (RegisteredObjectBufferMap::iterator I = ObjectBufferMap.begin(),
     // Call the private method that doesn't update the map so our iterator
     // doesn't break.
     deregisterObjectInternal(I);
   }
   ObjectBufferMap.clear();
 }
 
 void GDBJITRegistrationListener::NotifyObjectEmitted(
-                                       const ObjectFile &Object,
-                                       const RuntimeDyld::LoadedObjectInfo &L) {
+    ObjectKey K, const ObjectFile &Obj,
 
-  OwningBinary<ObjectFile> DebugObj = L.getObjectForDebug(Object);
+  OwningBinary<ObjectFile> DebugObj = L.getObjectForDebug(Obj);
 
   // Bail out if debug objects aren't supported.
   if (!DebugObj.getBinary())
     return;
 
   const char *Buffer = DebugObj.getBinary()->getMemoryBufferRef().getBufferStart();
   size_t      Size = DebugObj.getBinary()->getMemoryBufferRef().getBufferSize();
 
-  const char *Key = Object.getMemoryBufferRef().getBufferStart();
-
-  assert(Key && "Attempt to register a null object with a debugger.");
   llvm::MutexGuard locked(*JITDebugLock);
-  assert(ObjectBufferMap.find(Key) == ObjectBufferMap.end() &&
+  assert(ObjectBufferMap.find(K) == ObjectBufferMap.end() &&
          "Second attempt to perform debug registration.");
   jit_code_entry* JITCodeEntry = new jit_code_entry();
 
   if (!JITCodeEntry) {
     llvm::report_fatal_error(
       "Allocation failed when registering a JIT entry!\n");
   } else {
     JITCodeEntry->symfile_addr = Buffer;
     JITCodeEntry->symfile_size = Size;
 
-    ObjectBufferMap[Key] = RegisteredObjectInfo(Size, JITCodeEntry,
-                                                std::move(DebugObj));
+    ObjectBufferMap[K] =
+        RegisteredObjectInfo(Size, JITCodeEntry, std::move(DebugObj));
     NotifyDebugger(JITCodeEntry);
   }
 }
 
-void GDBJITRegistrationListener::NotifyFreeingObject(const ObjectFile& Object) {
-  const char *Key = Object.getMemoryBufferRef().getBufferStart();
+void GDBJITRegistrationListener::NotifyFreeingObject(ObjectKey K) {
   llvm::MutexGuard locked(*JITDebugLock);
-  RegisteredObjectBufferMap::iterator I = ObjectBufferMap.find(Key);
+  RegisteredObjectBufferMap::iterator I = ObjectBufferMap.find(K);
 
   if (I != ObjectBufferMap.end()) {
     deregisterObjectInternal(I);
     ObjectBufferMap.erase(I);
   }
 }
 
 void GDBJITRegistrationListener::deregisterObjectInternal(

lib/ExecutionEngine/IntelJITEvents/IntelJITEventListener.cpp

class IntelJITEventListener : public JITEventListener {
 
   std::unique_ptr<IntelJITEventsWrapper> Wrapper;
   MethodIDMap MethodIDs;
 
   typedef SmallVector<const void *, 64> MethodAddressVector;
   typedef DenseMap<const void *, MethodAddressVector>  ObjectMap;
 
   ObjectMap  LoadedObjectMap;
-  std::map<const char*, OwningBinary<ObjectFile>> DebugObjects;
+  std::map<ObjectKey, OwningBinary<ObjectFile>> DebugObjects;
 
 public:
   IntelJITEventListener(IntelJITEventsWrapper* libraryWrapper) {
       Wrapper.reset(libraryWrapper);
   }
 
   ~IntelJITEventListener() {
   }
 
-  void NotifyObjectEmitted(const ObjectFile &Obj,
+  void NotifyObjectEmitted(ObjectKey Key, const ObjectFile &Obj,
                            const RuntimeDyld::LoadedObjectInfo &L) override;
 
-  void NotifyFreeingObject(const ObjectFile &Obj) override;
+  void NotifyFreeingObject(ObjectKey Key) override;
 };
 
 static LineNumberInfo DILineInfoToIntelJITFormat(uintptr_t StartAddress,
                                                  uintptr_t Address,
                                                  DILineInfo Line) {
   LineNumberInfo Result;
 
   Result.Offset = Address - StartAddress;
static iJIT_Method_Load FunctionDescToIntelJITFormat(
   Result.user_data = NULL;
   Result.user_data_size = 0;
   Result.env = iJDE_JittingAPI;
 
   return Result;
 }
 
 void IntelJITEventListener::NotifyObjectEmitted(
-                                       const ObjectFile &Obj,
-                                       const RuntimeDyld::LoadedObjectInfo &L) {
+    ObjectKey Key, const ObjectFile &Obj,
 
   OwningBinary<ObjectFile> DebugObjOwner = L.getObjectForDebug(Obj);
   const ObjectFile *DebugObj = DebugObjOwner.getBinary();
   if (!DebugObj)
     return;
 
   // Get the address of the object image for use as a unique identifier
   const void* ObjData = DebugObj->getData().data();
Wrapper->iJIT_NotifyEvent(iJVM_EVENT_TYPE_METHOD_LOAD_FINISHED,
                               &FunctionMessage);
     MethodIDs[(void*)Addr] = FunctionMessage.method_id;
   }
 
   // To support object unload notification, we need to keep a list of
   // registered function addresses for each loaded object.  We will
   // use the MethodIDs map to get the registered ID for each function.
   LoadedObjectMap[ObjData] = Functions;
-  DebugObjects[Obj.getData().data()] = std::move(DebugObjOwner);
+  DebugObjects[Key] = std::move(DebugObjOwner);
 }
 
-void IntelJITEventListener::NotifyFreeingObject(const ObjectFile &Obj) {
+void IntelJITEventListener::NotifyFreeingObject(ObjectKey Key) {
   // This object may not have been registered with the listener. If it wasn't,
   // bail out.
-  if (DebugObjects.find(Obj.getData().data()) == DebugObjects.end())
+  if (DebugObjects.find(Key) == DebugObjects.end())
     return;
 
   // Get the address of the object image for use as a unique identifier
-  const ObjectFile &DebugObj = *DebugObjects[Obj.getData().data()].getBinary();
+  const ObjectFile &DebugObj = *DebugObjects[Key].getBinary();
   const void* ObjData = DebugObj.getData().data();
 
   // Get the object's function list from LoadedObjectMap
   ObjectMap::iterator OI = LoadedObjectMap.find(ObjData);
   if (OI == LoadedObjectMap.end())
     return;
   MethodAddressVector& Functions = OI->second;
 
   // Walk the function list, unregistering each function
   for (MethodAddressVector::iterator FI = Functions.begin(),
                                      FE = Functions.end();
        FI != FE;
        ++FI) {
     void* FnStart = const_cast<void*>(*FI);
     MethodIDMap::iterator MI = MethodIDs.find(FnStart);
     if (MI != MethodIDs.end()) {
       Wrapper->iJIT_NotifyEvent(iJVM_EVENT_TYPE_METHOD_UNLOAD_START,
                                 &MI->second);
       MethodIDs.erase(MI);
     }
   }
 
   // Erase the object from LoadedObjectMap
   LoadedObjectMap.erase(OI);
-  DebugObjects.erase(Obj.getData().data());
+  DebugObjects.erase(Key);
 }
 
 }  // anonymous namespace.
 
 namespace llvm {
 JITEventListener *JITEventListener::createIntelJITEventListener() {
   return new IntelJITEventListener(new IntelJITEventsWrapper);
 }

lib/ExecutionEngine/MCJIT/MCJIT.h

 protected:
   /// emitObject -- Generate a JITed object in memory from the specified module
   /// Currently, MCJIT only supports a single module and the module passed to
   /// this function call is expected to be the contained module.  The module
   /// is passed as a parameter here to prepare for multiple module support in
   /// the future.
   std::unique_ptr<MemoryBuffer> emitObject(Module *M);
 
-  void NotifyObjectEmitted(const object::ObjectFile& Obj,
+  void NotifyObjectEmitted(const object::ObjectFile &Obj,
                            const RuntimeDyld::LoadedObjectInfo &L);
-  void NotifyFreeingObject(const object::ObjectFile& Obj);
+  void NotifyFreeingObject(const object::ObjectFile &Obj);
 
   JITSymbol findExistingSymbol(const std::string &Name);
   Module *findModuleForSymbol(const std::string &Name, bool CheckFunctionsOnly);
 };
 
 } // end llvm namespace
 
 #endif // LLVM_LIB_EXECUTIONENGINE_MCJIT_MCJIT_H

lib/ExecutionEngine/MCJIT/MCJIT.cpp

void MCJIT::UnregisterJITEventListener(JITEventListener *L) {
   if (I != EventListeners.rend()) {
     std::swap(*I, EventListeners.back());
     EventListeners.pop_back();
   }
 }
 
 void MCJIT::NotifyObjectEmitted(const object::ObjectFile& Obj,
                                 const RuntimeDyld::LoadedObjectInfo &L) {
+  uint64_t Key =
+      static_cast<uint64_t>(reinterpret_cast<uintptr_t>(Obj.getData().data()));
   MutexGuard locked(lock);
   MemMgr->notifyObjectLoaded(this, Obj);
   for (unsigned I = 0, S = EventListeners.size(); I < S; ++I) {
-    EventListeners[I]->NotifyObjectEmitted(Obj, L);
+    EventListeners[I]->NotifyObjectEmitted(Key, Obj, L);
   }
 }
 
 void MCJIT::NotifyFreeingObject(const object::ObjectFile& Obj) {
+  uint64_t Key =
+      static_cast<uint64_t>(reinterpret_cast<uintptr_t>(Obj.getData().data()));
   MutexGuard locked(lock);
   for (JITEventListener *L : EventListeners)
-    L->NotifyFreeingObject(Obj);
+    L->NotifyFreeingObject(Key);
 }
 
 JITSymbol
 LinkingSymbolResolver::findSymbol(const std::string &Name) {
   auto Result = ParentEngine.findSymbol(Name, false);
   if (Result)
     return Result;
   if (ParentEngine.isSymbolSearchingDisabled())
     return nullptr;
   return ClientResolver->findSymbol(Name);
 }
 
 void LinkingSymbolResolver::anchor() {}

lib/ExecutionEngine/OProfileJIT/OProfileJITEventListener.cpp

 #define DEBUG_TYPE "oprofile-jit-event-listener"
 
 namespace {
 
 class OProfileJITEventListener : public JITEventListener {
   std::unique_ptr<OProfileWrapper> Wrapper;
 
   void initialize();
-  std::map<const char*, OwningBinary<ObjectFile>> DebugObjects;
+  std::map<ObjectKey, OwningBinary<ObjectFile>> DebugObjects;
 
 public:
   OProfileJITEventListener(std::unique_ptr<OProfileWrapper> LibraryWrapper)
     : Wrapper(std::move(LibraryWrapper)) {
     initialize();
   }
 
   ~OProfileJITEventListener();
 
-  void NotifyObjectEmitted(const ObjectFile &Obj,
+  void NotifyObjectEmitted(ObjectKey Key, const ObjectFile &Obj,
                            const RuntimeDyld::LoadedObjectInfo &L) override;
 
-  void NotifyFreeingObject(const ObjectFile &Obj) override;
+  void NotifyFreeingObject(ObjectKey Key) override;
 };
 
 void OProfileJITEventListener::initialize() {
   if (!Wrapper->op_open_agent()) {
     const std::string err_str = sys::StrError();
     LLVM_DEBUG(dbgs() << "Failed to connect to OProfile agent: " << err_str
                       << "\n");
   } else {
if (Wrapper->op_close_agent() == -1) {
                         << err_str << "\n");
     } else {
       LLVM_DEBUG(dbgs() << "Disconnected from OProfile agent.\n");
     }
   }
 }
 
 void OProfileJITEventListener::NotifyObjectEmitted(
-                                       const ObjectFile &Obj,
-                                       const RuntimeDyld::LoadedObjectInfo &L) {
+    ObjectKey Key, const ObjectFile &Obj,
   if (!Wrapper->isAgentAvailable()) {
     return;
   }
 
   OwningBinary<ObjectFile> DebugObjOwner = L.getObjectForDebug(Obj);
   const ObjectFile &DebugObj = *DebugObjOwner.getBinary();
   std::unique_ptr<DIContext> Context = DWARFContext::create(DebugObj);
 
if (Wrapper->op_write_debug_line_info((void *)Addr, num_entries,
                                           debug_line) == -1) {
       LLVM_DEBUG(dbgs() << "Failed to tell OProfiler about debug object at ["
                         << (void *)Addr << "-" << ((char *)Addr + Size)
                         << "]\n");
       continue;
     }
   }
 
-  DebugObjects[Obj.getData().data()] = std::move(DebugObjOwner);
+  DebugObjects[Key] = std::move(DebugObjOwner);
 }
 
-void OProfileJITEventListener::NotifyFreeingObject(const ObjectFile &Obj) {
+void OProfileJITEventListener::NotifyFreeingObject(ObjectKey Key) {
   if (Wrapper->isAgentAvailable()) {
 
     // If there was no agent registered when the original object was loaded then
     // we won't have created a debug object for it, so bail out.
-    if (DebugObjects.find(Obj.getData().data()) == DebugObjects.end())
+    if (DebugObjects.find(Key) == DebugObjects.end())
       return;
 
-    const ObjectFile &DebugObj = *DebugObjects[Obj.getData().data()].getBinary();
+    const ObjectFile &DebugObj = *DebugObjects[Key].getBinary();
 
     // Use symbol info to iterate functions in the object.
     for (symbol_iterator I = DebugObj.symbol_begin(),
                          E = DebugObj.symbol_end();
          I != E; ++I) {
       if (I->getType() && *I->getType() == SymbolRef::ST_Function) {
         Expected<uint64_t> AddrOrErr = I->getAddress();
         if (!AddrOrErr)
           continue;
         uint64_t Addr = *AddrOrErr;
 
         if (Wrapper->op_unload_native_code(Addr) == -1) {
           LLVM_DEBUG(
               dbgs()
               << "Failed to tell OProfile about unload of native function at "
               << (void *)Addr << "\n");
           continue;
         }
       }
     }
   }
 
-  DebugObjects.erase(Obj.getData().data());
+  DebugObjects.erase(Key);
 }
 
 }  // anonymous namespace.
 
 namespace llvm {
 JITEventListener *JITEventListener::createOProfileJITEventListener() {
   return new OProfileJITEventListener(llvm::make_unique<OProfileWrapper>());
 }
 
 } // namespace llvm
 
 LLVMJITEventListenerRef LLVMCreateOProfileJITEventListener(void)
 {
   return wrap(JITEventListener::createOProfileJITEventListener());
 }

lib/ExecutionEngine/Orc/OrcCBindingsStack.h

private:
   void reportError(Error Err) {
     // FIXME: Report errors on the execution session.
     logAllUnhandledErrors(std::move(Err), errs(), "ORC error: ");
   };
 
   void notifyFinalized(orc::VModuleKey K,
            const object::ObjectFile &Obj,
            const RuntimeDyld::LoadedObjectInfo &LoadedObjInfo) {
+    uint64_t Key = static_cast<uint64_t>(
+        reinterpret_cast<uintptr_t>(Obj.getData().data()));
     for (auto &Listener : EventListeners)
-      Listener->NotifyObjectEmitted(Obj, LoadedObjInfo);
+      Listener->NotifyObjectEmitted(Key, Obj, LoadedObjInfo);
   }
 
   void notifyFreed(orc::VModuleKey K, const object::ObjectFile &Obj) {
+    uint64_t Key = static_cast<uint64_t>(
+        reinterpret_cast<uintptr_t>(Obj.getData().data()));
     for (auto &Listener : EventListeners)
-      Listener->NotifyFreeingObject(Obj);
+      Listener->NotifyFreeingObject(Key);
   }
 
   orc::ExecutionSession ES;
   std::unique_ptr<CompileCallbackMgr> CCMgr;
 
   std::vector<JITEventListener *> EventListeners;
 
   DataLayout DL;