Under the assumption of -analyzer-config c++-allocator-inlining=true, which enables evaluation of operator new as a regular function call, this patch shows what it takes to actually inline the constructor into the return value of such operator call.
The CFG for a new C() expression looks like:
- CXXNewAllocator new C() (a special kind of CFGElement on which operator new is being evaluated)
- CXXConstructExpr C() (a regular CFGStmt element on which we call the constructor)
- CXXNewExpr new C() (a regular CFGStmt element on which we should ideally have nothing to do)
What i did here is:
- First, i relax the requirements for constructor regions, as discussed on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-November/056095.html). We can now construct into ElementRegion unless it actually represents an array element (we're not dealing with operator new yet). Also we remove the explicit bailout for constructions that correspond to operator new parent expressions, as long as -analyzer-config c++-allocator-inlining is enabled. See changes in mayInlineCallKind().
- Then, when the constructor is being modeled, we're trying to get back the value returned by operator new. This is done similarly to other construction cases - by finding the next (!!) element in the CFG, figuring out that it's a CXXNewExpr, and then taking the respective region to construct into from the Environment. The CXXNewAllocator element is not relied upon on this phase - for now it only triggers the evaluation of operator new at the right moment, so that we had the return value. See changes in getRegionForConstructedObject() and canHaveDirectConstructor().
- When we reach the actual CXXNewExpr CFG element (the third one), we need to preserve the value we already have for the CXXNewExpr in the environment. The old code that's conjuring a (heap) pointer here would probably still remain to handle the case when an inlined operator new has actually returned an UnknownVal. Still, this needs polishing, as the FIXME at the top of VisitCXXNewExpr() suggests. See the newly added hack in VisitCXXNewExpr().
- Finally, the CXXNewExpr value keeps dying in the Environment. From the point of view of the current liveness analysis, the new-expression is dead (or rather not yet born) until the CXXNewExpr statement element is reached, so it immediately gets purged on every step. The really dirty code that prevents this, that should never be committed, is in shouldRemoveDeadBindings(): for the sake of this proof-of-concept, i've crudely disabled garbage collection on the respective moments of time. I believe that the proper fix would be on the liveness analysis side: mark the new-expression as live between the CXXNewAllocator element and CXXNewExpr statement element.
My todo list before committing this would be:
- Fix the live expressions hack.
- See how reliable is findElementDirectlyInitializedByCurrentConstructor() in this case.
- Understand how my code works for non-object constructors (eg. new int).
- See how much VisitCXXNewExpr can be refactored.
Once this lands, i think we should be looking into enabling -analyzer-config c++-allocator-inlining by default.