[patch] BuryPointer the CXTranslationUnit when a crash has been detected

[Argyrios Kyrtzidis]

On May 11, 2014, at 11:08 AM, Nico Weber <thakis at chromium.org> wrote:

> Hi Ted and Daniel,
> 
> we're trying to get clang LSan-clean: completely free of leaks (PR19521). One of the last areas where leaks are still reported are in c-index-tests – I believe for the tests that test crash recovery. If libclang detects that it has crashed, it doesn't free the CXTranslationUnit in clang_disposeTranslationUnit().

Looks like isUnsafeToFree() was added long time ago, before we started releasing memory if libclang crashes, see all the places where we use CrashRecoveryContextCleanupRegistrar. We already use that to dispose of ASTUnits, we should go ahead and allow clang_disposeTranslationUnit, essentially remove 'isUnsafeToFree()’.

> 
> There are a few other places in clang that intentionally leak things, so BuryPointer() (in lib/Frontend/CompilerInvocation.cpp) exists to mark pointers that are not going to be freed. The attached patch lets clang_disposeTranslationUnit() call BuryPointer() when it's not going to free things.
> 
> BuryPointer only increments an atomic unsigned and then writes to a global array indexed by said unsigned, so this should be a relatively safe thing to do. I suppose if libclang crashes and one is very unlucky, some rogue code could have overwritten the atomic int, but BuryPointer() does range checking. I think this is a safe change to make, but it's tricky code so I thought pre-commit review is the way to go here.
> 
> Thanks,
> Nico
> <clang-burycontext.diff>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits