On Fri, Dec 27, 2013 at 11:39 AM, Chandler Carruth <[email protected]>wrote:
> > On Fri, Dec 27, 2013 at 2:10 AM, Kostya Serebryany <[email protected]> wrote: > >> + // This function may be called only a small fixed amount of times per >> each >> + // invocation, otherwise we do actually have a leak which we want to >> report. >> + // If this function is called more than kGraveYardMaxSize times, the >> pointers >> + // will not be properly buried and a leak detector will report a leak, >> which >> + // is what we want in such case. >> > > Interesting. I didn't realize it was going to be *that* tightly bounded. > > This makes me wonder if the whole disable free thing should just be > removed at this point. > I am all for it, --disable-free looks like a hack and we did not see any compile-time loss from removing it (measure on bootstrap). > Back in the day, we didn't so carefully use a BumpPtrAllocator in the > ASTContext. Today, we might be fine to call free 10 times. > But this is not 10 calls to free. This is 10-ish calls to delete, where the DTOR destructs other objects and so on. It may call much more than 10 frees. Still, see above. > Anyways, not trying to shift the goal posts; I mostly wanted to mention it > in case someone gets some time to look into removing all of the leak stuff. > > >> + static const size_t kGraveYardMaxSize = 16; >> + static const void *GraveYard[kGraveYardMaxSize]; >> + static llvm::sys::cas_flag GraveYardSize; >> > > Do these being function-local statics defeat the purpose of using a basic > atomic increment? I can't recall if we correctly eliminate the cxa_guard in > these cases... > We will not have cxa_guard here because there is no dynamic init -- both objects (the size and the array) are linker-initialized. Atomic increment is here for the future case when clang becomes multi-threaded.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
