This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by myhsu on Dec 18 2019, 5:55 PM.

Details

Reviewers
lhames
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.

Diff Detail

Event Timeline

myhsu created this revision.Dec 18 2019, 5:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 18 2019, 5:55 PM
myhsu edited the summary of this revision. (Show Details)Dec 18 2019, 5:56 PM
lhames accepted this revision.Dec 20 2019, 1:25 AM

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.

This revision is now accepted and ready to land.Dec 20 2019, 1:25 AM
myhsu added a comment.Dec 20 2019, 3:46 AM

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 added a subscriber: lenary.Dec 20 2019, 6:54 AM

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

myhsu added a comment.Dec 20 2019, 5:01 PM

@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

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 added a comment.Dec 20 2019, 7:29 PM

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?

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 closed this revision.Dec 21 2019, 12:23 AM

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.