This patch is roughly based on the discussion we've had in http://lists.llvm.org/pipermail/cfe-dev/2017-December/056314.html about how our support for C++ operator new() should provide useful callbacks to the checkers. As in, this patch tries to do a minimal necessary thing while avoiding all decisions that we didn't have a consensus over.
This patch also absorbs D36708.
In the discussion, we've been mentioning PreAllocator/PostAllocator callbacks. However, we already sort of have them: they are PreCall/PostCall callbacks for the allocator call, which are readily available in the c++-allocator-inlining mode.
This patch also causes no change in the behavior of the pre/post CXXNewExpr callbacks. They remain broken in the c++-allocator-inlining mode for now.
Missing in the current system of callbacks, however, is a callback that would be called before construction while providing the casted return value of operator new (in the sense of D41250: operator new returns void *, but we need to have C *). The user can subscribe on the allocator's PostCall and perform the cast manually, but that's an unpleasant thing to do. So it sounds like a good idea to provide a callback that would contain the relevant information.
A similar "Pre" callback doesn't seem necessary - there's not much additional information we can provide compared to PreCall.
In this patch, I add the callback (with a lot of code that is the usual boilerplate around the callback, though there's still a questionable TODO in CheckNewAllocatorContext::runChecker()) and modify MallocChecker to make use of it when c++-allocator-inlining mode is turned on. Specifically:
- If the allocator (operator new) call is evaluated conservatively, checkNewAllocator() provides the return value of the allocator that is semantically equivalent to the value in checkPostStmt<CXXNewExpr>() that we've been using earlier. Without the cast, the value is different and the checker crashes.
- If the allocator call is inlined, the checker does not treat it as an allocator, but instead as a simple inlined function. That is, it does not start tracking the return value here, and in particular avoids crashes, though it may continue tracking the value if it is already being tracked (see the new-ctor-malloc.cpp test). This policy has already been there because previously we could inline the operator when it was written without operator syntax - see tests in test/Analysis/NewDelete-custom.cpp. Whether this policy was correct or not is a matter of discussion. For now it means that in c++-allocator-inlining mode we'd have new negatives (i.e. lost positives) on code that uses custom allocators which do not boil down to simple malloc.
The std::nothrow_t tests in NewDelete-custom.cpp are affected in a surprising manner. I'd fix them in a follow-up commit.