Szelethus added a comment.
One important thing I forgot, that you cant rely on `ProgramState` due to
tidy's constraints. Btw, how do you plan to make this into a tidy checker? To
me it seems like it would amplify the already existing false positive issues
(if I understand your currect way of
Szelethus added a comment.
> Thanks for all your detailed and helpful input, I will make sure to go over
> all the comments and answer them, but it will take some time.
Cheers! I can't emphasize enough however that I might be wrong on what I've
said, or say in this comment.
> It was my
lebedev.ri added a comment.
In https://reviews.llvm.org/D52390#1243228, @mate1214 wrote:
> Hi @lebedev.ri!
>
> Thanks for the question. I was not sure as to where exactly put the files.
> The important thing for me is that the `StackUsageMeasuringVisitor` should be
> reachable from the
mate1214 added a comment.
Hi @lebedev.ri!
Thanks for the question. I was not sure as to where exactly put the files. The
important thing for me is that the `StackUsageMeasuringVisitor` should be
reachable from the clang-tidy checker. (For the bigger picture please refer to
my answer to
mate1214 added a comment.
Hi @Szelethus !
Thanks for all your detailed and helpful input, I will make sure to go over all
the comments and answer them, but it will take some time.
A bit more background information on this checker and how it came to be might
help you and others to understand
lebedev.ri added a comment.
Without seeing the full picture (i.e. what you want to do with the clang-tidy
check) it is hard to tell,
but are you *very* sure all this logic should be in
`clang/lib/StaticAnalyzer/`, and not in `clang/lib/Analysis/` ?
Repository:
rC Clang
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:19
+#include "clang/AST/StmtCXX.h"
+#include "clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:104
+ ProgramStateRef State = C.getState();
+ auto StackLevels = State->get();
+ if (length(StackLevels) != countPredecessors(C))
Szelethus wrote:
> It isn't obvious
Szelethus added subscribers: rnkovacs, baloghadamsoftware.
Szelethus added a comment.
I think the idea for a checker like this is great! I left some inline comments,
but most of them are minor nits. I have some general remarks to make however:
- Your code lacks comments, especially a nice
Szelethus added a comment.
Hi!
Always great to see a new checker! I've started working in this project little
over half a year ago, so I don't claim to be an expert, read my remarks as
such! It'll be some time before I go through the entire code, but so far here
are the things that caught my
mate1214 created this revision.
mate1214 added reviewers: NoQ, george.karpenkov, dcoughlin.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin,
szepet, mgorny.
Add StackSizeChecker to StaticAnalyzer
This checker can be used to warn about potential stack overflows
11 matches
Mail list logo
