[PATCH] D42457: [analyzer] Don't communicate evaluation failures through memregion hierarchy.

[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.

- If we are unable to determine a good target region for a C++ constructor call, we come up with a temporary region and construct into that.
- If we suddenly notice that our constructor is part of an C++ array construction (operator new[] or an array-typed local variable), we come up with the element-zero region and construct into that, same for destructors.

Both are bad not only because we fail to model stuff correctly, but also because we use region structure to communicate the fact that we failed to `ExprEngine::evalCall`, which would later decide if we should inline the call or not by looking at the region structure. The former is particularly bad because this prevents us from supporting the actual construction into temporaries - even if we produce a correct temporary region, we won't inline the constructor because temporary region is an (incorrect) indication of failure.

In order to unblock the progress in this area, this patch adds an auxiliary flag for `evalCall`'s clients to communicate potential problems with the call. Then `evalCall` would decide if it should still inline the call. Apparently, the current decision process is non-trivial in this case: for instance, we may still inline the constructor even if the target region is incorrect aka temporary, when the class's destructor is trivial. In any case, it's good to have all the when-to-inline heuristics in one place.

For now `ExprEngine::evalCall` itself doesn't have a flag - only `ExprEngine::defaultEvalCall` does. Because for now it's only relevant to constructor/destructor calls, but checker-side `evalCall` only works for simple call expressions - that's a separate issue. Ideally the flag should go there as well, and `defaultEvalCall` would only be called from `ExprEngine::evalCall`.

Functional change here is accidental - by communicating array destructor situation properly, we're able to fix an old FIXME.


Repository:
  rC Clang

https://reviews.llvm.org/D42457

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/new.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42457.131184.patch
Type: text/x-patch
Size: 13070 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180124/203ebbe5/attachment.bin>