[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254423. Charusso added a comment. - Remove the last dead comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.llvm.org/D70411 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Chec

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254420. Charusso marked 4 inline comments as done. Charusso added a comment. - Simplify tests. - Remove dead code, they are far away to being used. - Add an extra test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.l

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso added a comment. Thanks for the review, hopefully if I ping @NoQ in every round, it will be green-marked soon. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55 + // they can cause a not null-terminated string. In this case we store the + // strin

[PATCH] D70411: [analyzer] CERT: STR31-C

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55 + // they can cause a not null-terminated string. In this case we store the + // string is being not null-terminated in the 'NullTerminationMap'. + // The func

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso added a comment. Given that the secondary behavior confuse people I have removed it for now. May if someone introduce a NullTerminationChecker then we introduce such option to warn on insecure calls instant. Thanks @balazske for influencing that change. @NoQ this project had a deadline

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254061. Charusso marked 6 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Get rid of the secondary behavior for now. - Fix review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 6 inline comments as done. Charusso added a comment. "To prevent such errors, either limit copies through truncation or, preferably, ensure that the destination is of sufficient size to hold the character data" - from the rule's page. Most of the projects are fine truncating by h

[PATCH] D70411: [analyzer] CERT: STR31-C

balazske added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +""" Charusso wrote: > Szelethus wrote: > > balazske wrote: > > > There are already more checkers that can check for CERT rela

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 4 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803 + +} // end "cert.str" + balazske wrote: > NoQ wrote: > > `alpha.cert.str`. > This text may be hard to understand. T

[PATCH] D70411: [analyzer] CERT: STR31-C

balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803 + +} // end "cert.str" + NoQ wrote: > `alpha.cert.str`. This text may be hard to understand. The option means "if it is off, the problem report happens only i

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 5 inline comments as done. Charusso added a comment. Thanks for the feedback! Given that it will remain an `alpha` checker for a long time (~1 year), no one really should use it. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +

[PATCH] D70411: [analyzer] CERT: STR31-C

Szelethus added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +""" balazske wrote: > There are already more checkers that can check for CERT related problems but > not specially made for

[PATCH] D70411: [analyzer] CERT: STR31-C

balazske added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +""" There are already more checkers that can check for CERT related problems but not specially made for these. These checker

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 241752. Charusso edited the summary of this revision. Charusso added a comment. - 2020-ify the checker writing - Remove extra bullet-points of CERT checker documentation: we only need the checker's documentation, not the packages. CHANGES SINCE LAST ACTION

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 233903. Charusso retitled this revision from "[analyzer] CERT: StrChecker: Implementing most of the STR31-C" to "[analyzer] CERT: STR31-C". Charusso added a comment. - Iterate over parameters rather arguments for searching string-reading. - Better notes of t