[PATCH] D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
Herald added subscribers: rnkovacs, eraman.

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:

1. 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()`.

2. 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()`.

3. 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()`.

4. 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:

1. Fix the live expressions hack.
2. See how reliable is `findElementDirectlyInitializedByCurrentConstructor()` in this case.
3. Understand how my code works for non-object constructors (eg. `new int`).
4. 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.


https://reviews.llvm.org/D40560

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/inline.cpp
  test/Analysis/new-ctor.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40560.124570.patch
Type: text/x-patch
Size: 8200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171128/048d572a/attachment.bin>