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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Y'all writing really good patches lately.

The updates on the C++ tests look spot on to me. I'm not sure if these tests contain any actual UB but the checker was anyway designed to be a bit more aggressive than that and that's exactly the kind of stuff we wanted it to warn us about. And even if it turns out to have been a bad idea from the start, that's a separate story.

I'll have some bikeshedding about warning and note text but overall I really like it.



================
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);
----------------
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"(?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107078