llvm dev - Dec 2011 - [LLVMdev] Critical sections cleanup

Hello,

I have an application that uses Clang and LLVM wrapped in a DLL. Last week I was
debugging reported issues with Microsoft Application
Verifier<http://www.microsoft.com/download/en/details.aspx?id=20028> that
were related to how critical sections are handled. The issues are caused by
critical sections that are either still active when heap blocks are deleted or
the DLL is unloaded. Something to keep in mind, our codebase is still using LLVM
2.7 but from what I was able to tell I think my findings also apply to the
trunk.

The first issue had to do with an active critical section in a heap that's
being deleted. The problem was caused by a leak of the PassRegistrar object in
Pass.cpp. I added a cleanup function to delete the PassRegistrar object before
shutting LLVM down. The cleanup function also sets the PassRegistrar object to
NULL so it can be reinitialized again. I see that in the trunk things work a bit
different, the PassRegistrar object is wrapped in a ManagedStatic object which
gets deleted at shutdown.

The second issue is that the critical section created in Signals.inc was still
active when the DLL was unloaded. There are three places where critical sections
are used in Windows, these are Mutex.inc/h, RWMutex.inc/h and Signals.inc/h. The
first two define classes that use the critical sections and have destructors
that delete them. The critical section in Signals has no mechanism for deleting.
I again added a function to my local copy a function to delete the critical
section. This cleanup function is called before shutting LLVM down.

There's a note on PassRegistry.cpp discussing the need for a solution that
prevents the PassRegistrar object leak and works after shutdown. I think what I
did fits the bill but it does need to be called explicitly at shutdown. I've
attached my changes for review along with a sample of a modified llc.cpp to show
how the changes work. If people are interested in them I can polish them up and
get them checked in.

Thanks,
Javier
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llc.cpp.patch
Type: application/octet-stream
Size: 525 bytes
Desc: llc.cpp.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Signals.inc.patch
Type: application/octet-stream
Size: 694 bytes
Desc: Signals.inc.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Signals.h.patch
Type: application/octet-stream
Size: 619 bytes
Desc: Signals.h.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PassRegistry.h.patch
Type: application/octet-stream
Size: 743 bytes
Desc: PassRegistry.h.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PassRegistry.cpp.patch
Type: application/octet-stream
Size: 1717 bytes
Desc: PassRegistry.cpp.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111206/edd8ea64/attachment-0004.obj>

I haven't received any comments regarding the cleanup of critical section or
the solution I proposed. I'm still interested in implementing a solution but
would like feedback on the patches I sent with the original email.

Javier

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Martinez, Javier E
Sent: Tuesday, December 06, 2011 3:27 PM
To: llvmdev at cs.uiuc.edu
Subject: [LLVMdev] Critical sections cleanup

Hello,

I have an application that uses Clang and LLVM wrapped in a DLL. Last week I was
debugging reported issues with Microsoft Application
Verifier<http://www.microsoft.com/download/en/details.aspx?id=20028> that
were related to how critical sections are handled. The issues are caused by
critical sections that are either still active when heap blocks are deleted or
the DLL is unloaded. Something to keep in mind, our codebase is still using LLVM
2.7 but from what I was able to tell I think my findings also apply to the
trunk.

The first issue had to do with an active critical section in a heap that's
being deleted. The problem was caused by a leak of the PassRegistrar object in
Pass.cpp. I added a cleanup function to delete the PassRegistrar object before
shutting LLVM down. The cleanup function also sets the PassRegistrar object to
NULL so it can be reinitialized again. I see that in the trunk things work a bit
different, the PassRegistrar object is wrapped in a ManagedStatic object which
gets deleted at shutdown.

The second issue is that the critical section created in Signals.inc was still
active when the DLL was unloaded. There are three places where critical sections
are used in Windows, these are Mutex.inc/h, RWMutex.inc/h and Signals.inc/h. The
first two define classes that use the critical sections and have destructors
that delete them. The critical section in Signals has no mechanism for deleting.
I again added a function to my local copy a function to delete the critical
section. This cleanup function is called before shutting LLVM down.

There's a note on PassRegistry.cpp discussing the need for a solution that
prevents the PassRegistrar object leak and works after shutdown. I think what I
did fits the bill but it does need to be called explicitly at shutdown. I've
attached my changes for review along with a sample of a modified llc.cpp to show
how the changes work. If people are interested in them I can polish them up and
get them checked in.

Thanks,
Javier
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111221/9274bb4b/attachment.html>