[PATCH] D52390: [analyzer] Add StackSizeChecker to StaticAnalyzer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

@Szelethus, thank you a lot for covering this review!

@mate1214, yes, please please split this up, like @Szelethus described.

At the moment this patch is not only huge, it is very under-tested. Eg., the Visitor promises support for C++ temporaries (which can potentially be a very difficult problem), but tests don't contain even a single class. If the functionality was added incrementally, it would have been easy to make sure every improvement is covered by regression tests.

Am i understanding correctly that the visitor is capable of summing up stack usage on the current path, i.e. ignoring objects that are put on the stack only within parts of the function that were not in fact executed on the current path, while also ignoring objects that are already removed from the stack? It should indeed be possible to accomplish this by only looking at the AST (unless VLAs or `alloca()` calls are used excessively), but in this case why is the `checkEndFunction()` callback not specifying the current path?

I guess a good idea for this checker would be to add a `BugReporterVisitor` to it in order to highlight where exactly does the stack usage go up. It is allowed to be slow, so it might be reasonable to just outright re-run the Visitor on every statement in the bug path.

I approve moving the visitor into `lib/Analysis`. That's indeed the library into which we dump all static analysis that is not specific to Static Analyzer.

Once out of alpha, this checker will look great in `optin.performance`, where we already have a checker for excessive structure padding.



================
Comment at: include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:28
+namespace clang {
+namespace stack {
+
----------------
Szelethus wrote:
> Use `ento` after `clang`. Btw, does anybody know what `ento` refers to? Never got to know it :D
> ```
> namespace clang {
> namespace ento {
> namespace stack {
It's Entomology.


Repository:
  rC Clang

https://reviews.llvm.org/D52390