[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+      isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
+      isa<StackSpaceRegion>(MR->getMemorySpace());
----------------
xazax.hun wrote:
> I think strictly speaking this might be "unsound" resulting in false positives in scenarios like:
> ```
> void f(bool reset) {
>   static T&& extended = getTemp();
>   if (reset) {
>     extended = getTemp();
>     return;
>   }
>   consume(std::move(extended));
>   f(true);
>   extended.use();
> }
> ```
> 
> In case the call to `f(true)` is not inlined (or in other cases there is some aliasing the analyzer does not know about), we might not realize that the object is reset and might falsely report a use after move.
> 
> But I think this is probably sufficiently rare that we do not care about those. 
Aren't such case already excluded by the `isa<StackSpaceRegion>(MR->getMemorySpace())`, in a same way as we do for variables?
In such case, we should be creating `getCXXStaticLifetimeExtendedObjectRegion` so it will not be located on stack.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+    QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+    os << "stack memory associated with temporary object of type '";
+    Ty.print(os, Ctx.getPrintingPolicy());
----------------
xazax.hun wrote:
> Is this a good wording for static lifetime extended temporaries?
Lifetime extended static temporaries should never enter here, as they are not in `StackSpaceRegion`. 
Previously we have had `CXXStaticTempObjects` and now they are turned into `CXXStaticLifetimeExtendedObjectRegion`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325