[ORC][EH] Deregister EH frames in MemoryManager dtor

[myhsu]

Summary

Try to fix Bug 23991

Note that we can not deregister in ~MemoryManager since some compiler runtime implementations (e.g. libgcc) would access memory content during deregisteration, but some of the MemoryManager implementations (e.g. SectionMemoryManager) would unmap all the memory sections in dtor.

This bug was also found when Pony programming language try to upgrade to LLVM 9.x.

[lhames]

Hi myhsu -- This looks good to me. Do you have commit access? If so please feel free to commit. If not just let me know and I will commit on your behalf.

[myhsu]

In D71683#1792286, @lhames wrote:

Hi myhsu -- This looks good to me. Do you have commit access? If so please feel free to commit. If not just let me know and I will commit on your behalf.

Thanks @lhames . I don't have commit access so it would be great if you can help me do it :-)

[lenary]

@myhsu Now that you have three accepted commits to LLVM, you can ask for commit access! Follow these instructions

[myhsu]

In D71683#1792794, @lenary wrote:

@myhsu Now that you have three accepted commits to LLVM, you can ask for commit access! Follow these instructions

Thanks for the advise @lenary , that's a good point. Will do

[lhames]

Actually, are you seeing this bug on ORC? Or just on MCJIT?

Looking at the code again, if you're using ORC this should be handled automatically. If you're using MCJIT it is (unfortunately) contract that you're responsible for calling this yourself. It would be safe to add this call to SectionMemoryManager as redundant calls (from existing clients) are no-ops, but it introduces a hazard: Anyone who brings up a JIT successfully with SectionMemoryManager is suddenly going to discover that their code fails when they try to use a custom memory manager.

[myhsu]

In D71683#1793661, @lhames wrote:

Actually, are you seeing this bug on ORC? Or just on MCJIT?

ORC, specifically ORCv2. I did see deregistration called in LegacyRTDyldObjectLinkingLayer, but well, it’s legacy v1. I didn’t see any deregistration in v2.

Looking at the code again, if you're using ORC this should be handled automatically. If you're using MCJIT it is (unfortunately) contract that you're responsible for calling this yourself. It would be safe to add this call to SectionMemoryManager as redundant calls (from existing clients) are no-ops, but it introduces a hazard: Anyone who brings up a JIT successfully with SectionMemoryManager is suddenly going to discover that their code fails when they try to use a custom memory manager.

So I did see a new EHFrameRegistraPlugin that will deregistrate EH frame. However, I didn’t see any usage of that class other than llvm-jitlink utility. Will that plugin be the mandatory component for EH in the future?

[lhames]

In D71683#1793698, @myhsu wrote:

In D71683#1793661, @lhames wrote:

Actually, are you seeing this bug on ORC? Or just on MCJIT?

ORC, specifically ORCv2.

Oh -- That's an ORCv2 bug. I have fixed that in 9f4f237e29e7. Would you be able to apply that patch to your branch and see if it fixes your issue?

So I did see a new EHFrameRegistraPlugin that will deregistrate EH frame. However, I didn’t see any usage of that class other than llvm-jitlink utility. Will that plugin be the mandatory component for EH in the future?

Yes. Though it will usually be set up for you implicitly. The plugin is only used by ObjectLinkingLayer (which is based on JITLink). RTDyldObjectLinkingLayer will continue to manage eh-frames via the memory manager.

• Lang.

[myhsu]

In D71683#1793713, @lhames wrote:

In D71683#1793698, @myhsu wrote:

In D71683#1793661, @lhames wrote:

Actually, are you seeing this bug on ORC? Or just on MCJIT?

ORC, specifically ORCv2.

Oh -- That's an ORCv2 bug. I have fixed that in 9f4f237e29e7. Would you be able to apply that patch to your branch and see if it fixes your issue?

Yes, your patch fixed my issue. I also agreed that calling deregistration in RTDyldObjectLinkingLayer's dtor is more general/portable than my solution. So I guess I'm not going to proceed this patch.

llvm/lib/ExecutionEngine/SectionMemoryManager.cpp

 
 void SectionMemoryManager::invalidateInstructionCache() {
   for (sys::MemoryBlock &Block : CodeMem.PendingMem)
     sys::Memory::InvalidateInstructionCache(Block.base(),
                                             Block.allocatedSize());
 }
 
 SectionMemoryManager::~SectionMemoryManager() {
+  deregisterEHFrames();
   for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
     for (sys::MemoryBlock &Block : Group->AllocatedMem)
       MMapper.releaseMappedMemory(Block);
   }
 }
 
 SectionMemoryManager::MemoryMapper::~MemoryMapper() {}