[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

This patch uses the reference to `MaterializeTemporaryExpr` stored in the construction context since https://reviews.llvm.org/D43477 in order to model that expression correctly, following the plan described in http://lists.llvm.org/pipermail/cfe-dev/2018-February/056898.html

`MaterializeTemporaryExpr` is an expression that takes an rvalue object (or one of its xvalue sub-objects) and transforms it into a reference to that object. From the analyzer's point of view, the value of the sub-expression would be a `NonLoc`, and the value of `MaterializeTemporaryExpr` would be a `Loc` that this `NonLoc` is stored in.

For now this behavior has been achieved by creating a `Loc` out of thin air and re-binding the `NonLoc` value into the newly created location. This, however, is incorrect, because the constructor that has constructed the respective `NonLoc` value was operating on a different memory region, so all of the subsequent method calls, including the destructor, should have the exact same "this" region that the constructor was using.

The problem with behaving correctly is that we've already forgot the original storage. It is technically impossible to reliably recover it from the `NonLoc`.

The patch addresses it by relying on the `ConstructionContext` to inform the constructor that lifetime extension is taking place, so that the constructor could store the current "this" region in the program state for later use. The region is uniquely determined by the `MaterializeTemporaryExpr` itself. Then  when time comes to model `MaterializeTemporaryExpr`, it looks up itself in the program state to find the respective region, and avoids the hassle of creating a new region and copying the value in case it does indeed find the existing correct region.

The temporary region's liveness (in the sense of `removeDeadBindings`) is extended until the `MaterializeTemporaryExpr` is resolved, in order to keep the store bindings around, because it wouldn't be referenced from anywhere else in the program state.

Situations where correct lifetime extension behavior is needed require relatively awkward test cases, because most of the time you don't care about the object's address as long as its contents are correct.

This patch is enough to assert that for every initialized temporary (lifetime-extended or not), a destructor is called (with the correct address) on all existing test cases(!). I added a new test case (with `BinaryConditionalOperator`) on which this assertion still fails (due to lack of construction context and other problems) - i've actually seen this operator used on C++ objects. There are more cases that we don't handle yet; i'd see what i can do about them.


Repository:
  rC Clang

https://reviews.llvm.org/D43497

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/lifetime-extension.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43497.135000.patch
Type: text/x-patch
Size: 17924 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180220/55073896/attachment-0001.bin>