[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso abandoned this revision. Charusso added a comment. This patch moved to D70411 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69813/new/ https://reviews.llvm.org/D69813 ___ cfe-commits mailing list

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) c

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a subscriber: gribozavr. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + uns

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked 14 inline comments as done. Charusso added a comment. Given that we are having two different projects at first let us create the path-sensitive error-catching + false positive suppression + design of the CERT rules, and when we are fine, we get back to the impossible-to-solve pro

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.DestinationP

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.Destina

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.Destina

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.DestinationP

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.DestinationP

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-11 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 228675. Charusso added a comment. - The packaging have not been addressed yet. - Inject the "zombie" size expression to the new function call (`fgets`) if none of the size expression's regions have been modified. The idea is that: When we set up a variable

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D69813#1736667 , @Charusso wrote: > In D69813#1736611 , @aaron.ballman > wrote: > > > Would it make sense to use `cert.str.31.c` to remove the random dash? Would > > this also hel

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D69813#1736611 , @aaron.ballman wrote: > Would it make sense to use `cert.str.31.c` to remove the random dash? Would > this also help the user to do something like `cert.str.*.cpp`? if they want > just the CERT C++ STR rules

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D69813#1736045 , @Charusso wrote: > In D69813#1735988 , @aaron.ballman > wrote: > > > I'm not @NoQ, but I do agree that there should be a separate check per rule > > in terms of t

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-06 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D69813#1735988 , @aaron.ballman wrote: > I'm not @NoQ, but I do agree that there should be a separate check per rule > in terms of the UI presented to the user. The name should follow the rule ID > like they do in clang-tidy

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D69813#1735804 , @Szelethus wrote: > In D69813#1734272 , @Charusso wrote: > > > In D69813#1734193 , @Szelethus > > wrote: > > > > > Hmm, so

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D69813#1734272 , @Charusso wrote: > In D69813#1734193 , @Szelethus wrote: > > > Hmm, so this checker is rather a collection of CERT rule checkers, right? > > Shouldn't the checker name

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D69813#1734193 , @Szelethus wrote: > Hmm, so this checker is rather a collection of CERT rule checkers, right? > Shouldn't the checker name contain the actual rule name (STR31-C)? User > interfacewise, I would much prefer sma

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Hmm, so this checker is rather a collection of CERT rule checkers, right? Shouldn't the checker name contain the actual rule name (STR31-C)? User interfacewise, I would much prefer smaller, leaner checkers than a big one with a lot of options, which are barely support

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 227892. Charusso added a comment. - Support `alloca()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69813/new/ https://reviews.llvm.org/D69813 Files: clang/include/clang/Lex/Preprocessor.h clang/include/clang/StaticAnalyzer/Checkers/Checkers

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 227888. Charusso marked 2 inline comments as done. Charusso retitled this revision from "[analyzer][WIP] CERTStrChecker: Model gets()" to "[analyzer] CERTStrChecker: Model gets()". Charusso added a comment. - Do not try to fix-it an array with offsets. CHA