[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked an inline comment as done.
steakhal added a comment.

In D107078#2917892 <https://reviews.llvm.org/D107078#2917892>, @NoQ wrote:

> Y'all writing really good patches lately.

Awesome, thank you!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:397-398
+                               Ctx.getLocationContext());
+      Report->addNote("The temporary object gets destroyed at the end of the "
+                      "full expression",
+                      L);
----------------
NoQ wrote:
> This is part of the path, right? We're reporting these bugs at `checkEndFunction` rather than at the store site so the destruction has already happened by the time we report it. In this case it should be a path note, not an extra blue-bubble note.
> 
> As for wording, I suggest `full-expression` with a dash (that's what other clang diagnostics use) and probably drop the initial "The"(?)
> This is part of the path, right? We're reporting these bugs at `checkEndFunction` rather than at the store site so the destruction has already happened by the time we report it. In this case it should be a path note, not an extra blue-bubble note.

I'm not exactly sure how to address this. AFAIK there is no way currently detecting the completion of a full-expression.
I was thinking of `Check::RegionChanges`, `Check::PostStmt<Expr>`, `Check::Live`, but none of these fits my needs. Do you have anything specific in mind that could help me?

Aside from that, the report looks reasonable even with a 'blue' bubble:
(scan-build): {F18363520}
(CodeChecker): {F18364795}

---

> As for wording, I suggest full-expression with a dash (that's what other clang diagnostics use) and probably drop the initial "The"(?)
I see, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107078