[PATCH] D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, Szelethus, mikhail.ramalho, baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.

The `return_from_top_frame` test demonstrates what happens. The object bound to the sub-expression of the return statement is a lazy compound value in this case. Liveness of that expression ends slightly before the end of the analysis, leaving time for `MallocChecker` to diagnose a leak.

This is fixed by admitting that we do not know how to properly model the memory region in which the return value is constructed. Neither `CXXTempObjectRegion` nor `SymbolicRegion` seems suitable, because the region needs to be in an unknown memory space and must also be live after the last reference disappears. The fix is only applied to C++17 when the object has a destructor (which produces a `CXX17ElidedCopyReturnedValueConstructionContext`) because other cases seem to work due to liveness analysis behaving correctly.

We could (and probably should) teach liveness analysis that with C++17 AST with `CXXBindTemporaryExpr` it is still a good idea to keep the top-frame returned expression alive forever. But we still don't have the correct region to represent construction target. Either we should modify liveness analysis and use `SymbolicRegion`, or we need to come up with a new kind of region (either for the return-to object itself, or for an implicit parameter through which its address is passed into the callee, so that the return-to object would be a `SymbolicRegion` of its `SymbolRegionValue`, similarly to how `CXXThisRegion` works).

Before i forget: re-enable tests for temporaries on C++17. Notice that some still fail. The current patch doesn't regress any of the old tests in this file that are now FIXME'd out.


Repository:
  rC Clang

https://reviews.llvm.org/D55804

Files:
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/temporaries.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55804.178578.patch
Type: text/x-patch
Size: 6660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181218/85febf95/attachment.bin>