[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-06-14 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. This generally sounds good. Definitely do submit these changes in small pieces! See http://llvm.org/docs/DeveloperPolicy.html#incremental-development for rationale. https://reviews.llvm.org/D27753 ___ cfe-commits mailing

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-06-14 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo added a comment. Before abandoning this patch and rewriting it, I would like to get a thumbs up for my plans: I will reimplement all functionality included here but without creating a new checker. Some parts which relate to specific checkers will be put into the corresponding checkers (l

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-03-10 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo added a comment. > Stepping back a bit, what do you consider "dirty" vs "clean"? It seems that > you are looking for prove that the values are known to be within the bounds > of min and max int values. What happens if there is a comparison to an > unknown symbolic value? Should that be c

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-03-08 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. > All this leads to the need of several types of taintednesses (string > taintedness, array taintedness, general bound check taintedness) because the > cleansing can only take down one type of taintedness at a time. That would be > the only way for a checker to be abl

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-03-03 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked an inline comment as done. gerazo added a comment. Hmm... I am thinking on this issue for a week now... I've played with the idea of implementing cleansing rules in GenericTaintChecker. It would be elegant but unfortunately, I have to think they are not general. Cleansing of a str

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-02-28 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked an inline comment as done. gerazo added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp:184 +Ty = Ctx.IntTy; + if (!Ty->isIntegerType() || Ctx.getIntWidth(Ty) <= TooNarrowForBoundCheck) +return false; a.sidor

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-02-17 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. > In this checker, I give warnings for values which are both tainted and were > also not checked by the programmer. So unlike GenericTaintChecker, I do > implement the boundedness check here for certain, interesting constructs > (which is controlled by the critical op

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-02-17 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Hello Zoltan. Sorry, I'm a bit busy now. Here are my thoughts about the design. 1. I think we should not add new warnings to GenericTaintChecker. Instead, it is better to move warnings out of it. After that it will become just a plugin used by other checkers. Such sp

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2017-02-17 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo added a comment. Hi, did you have time to check my changes? https://reviews.llvm.org/D27753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked an inline comment as done. gerazo added a comment. So thank you again for the valuable questions. In this checker, I give warnings for values which are both tainted and were also not checked by the programmer. So unlike GenericTaintChecker, I do implement the boundedness check here

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked 6 inline comments as done. gerazo added a comment. Thank you very much for your help. I've added all suggested modifications including tests covering all checker option settings. https://reviews.llvm.org/D27753 ___ cfe-commits mailing

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo updated this revision to Diff 82336. https://reviews.llvm.org/D27753 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp test/Ana

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-15 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. > Did you checked if same warnings may be emitted by another checkers? For > example, > ArrayBoundChecker may warn if index is tainted. I second that. The GenericTaintChecker also reports uses of tainted values. It is not clear that we should add a new separate chec

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-15 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Thank you for your work, Zoltán! Did you checked if same warnings may be emitted by another checkers? For example, ArrayBoundChecker may warn if index is tainted. I also have some comments below. Comment at: lib/StaticAnalyzer/Checkers/DirtyScalarChe

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-14 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo created this revision. gerazo added reviewers: zaks.anna, dcoughlin. gerazo added a subscriber: cfe-commits. Herald added a subscriber: mgorny. Checker for catching tainted value usage without proper bound checking. Uses GenericTaintChecker which is also in alpha.security. https://review