[PATCH] D71433: [analyzer] CERT: POS34-C

This revision was automatically updated to reflect the committed changes. Closed by commit rGa54d81f59796: [analyzer] CERT: POS34-C (authored by zukatsinadze). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze added a comment. In D71433#1880436 , @Szelethus wrote: > I think for an alpha checker this is ready to land if you're ready -- do you > have commit access or need assistance? Thank you. @Charusso will help. CHANGES SINCE LAST ACTION

[PATCH] D71433: [analyzer] CERT: POS34-C

Szelethus added a comment. Herald added a subscriber: martong. I think for an alpha checker this is ready to land if you're ready -- do you have commit access or need assistance? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze updated this revision to Diff 243149. zukatsinadze marked an inline comment as done. zukatsinadze added a comment. - Removed dead code. - Removed unnecessary if condition. - Changed error phrasing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze marked 2 inline comments as done. zukatsinadze added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3383-3386 +std::unique_ptr getMallocBRVisitor(SymbolRef Sym) { + return std::make_unique(Sym); +} + Szelethus

[PATCH] D71433: [analyzer] CERT: POS34-C

Szelethus accepted this revision. Szelethus added a comment. This is a very neat checker, the source code reads so easily, we might as well put it as the official CERT rule description. I think adding the non-compliant and compliant code examples would be nice. I also wrote some inline

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze added a comment. In D71433#1863638 , @Charusso wrote: > In D71433#1808316 , @Charusso wrote: > > > In D71433#1784238 , @NoQ wrote: > > > > > Currently the check

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a comment. In D71433#1808316 , @Charusso wrote: > In D71433#1784238 , @NoQ wrote: > > > Currently the check may warn on non-bugs of the following kind: > > > > void foo() { > > char env[] =

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze updated this revision to Diff 243127. zukatsinadze added a comment. Addressed new inline comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433 Files: clang/docs/analyzer/checkers.rst

[PATCH] D71433: [analyzer] CERT: POS34-C

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:57-58 +bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr, *Report); + } else if (const SymbolRef Sym = + ArgV.getAsSymbol()) { // It is

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a comment. In D71433#1784238 , @NoQ wrote: > Currently the check may warn on non-bugs of the following kind: > > void foo() { > char env[] = "NAME=value"; > putenv(env); > doStuff(); > putenv("NAME=anothervalue"); > } >

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I have created a `notes.cpp` test-file to test the notes in my checkers, but I think this checker is fine without that test file. @NoQ, what do you think? CHANGES SINCE LAST ACTION

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze updated this revision to Diff 233955. zukatsinadze added a comment. - Removed extra test - Used `CallDescription` for checking call. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433 Files: clang/docs/analyzer/checkers.rst

[PATCH] D71433: [analyzer] CERT: POS34-C

NoQ added a comment. In D71433#1784920 , @zukatsinadze wrote: > @NoQ I like the idea, but I am not really sure how to do that. I started > working on Static Analyzer just lask week. Let's get the initial attempt right first, and delay this for the

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze added a comment. In D71433#1784238 , @NoQ wrote: > Thanks! This looks like a simple and efficient check. I have one overall > suggestion. > > Currently the check may warn on non-bugs of the following kind: > > void foo() { > char env[]

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze marked 4 inline comments as done. zukatsinadze added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:765 + HelpText<"Finds calls to the `putenv` function which pass a pointer to " + "an automatic variable as the

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze updated this revision to Diff 233946. zukatsinadze added a comment. Addressed most of the inline comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433 Files: clang/docs/analyzer/checkers.rst

[PATCH] D71433: [analyzer] CERT: POS34-C

NoQ added a comment. Thanks! This looks like a simple and efficient check. I have one overall suggestion. Currently the check may warn on non-bugs of the following kind: void foo() { char env[] = "NAME=value"; putenv(env); doStuff(); putenv("NAME=anothervalue"); } I.e.,

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a reviewer: aaron.ballman. Charusso added a subscriber: aaron.ballman. Charusso added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1881 + + #include + I would remove that line. Comment at:

[PATCH] D71433: [analyzer] CERT: POS34-C

zukatsinadze created this revision. zukatsinadze added a reviewer: NoQ. zukatsinadze added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. This patch introduces a new checker: