[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. balazske marked an inline comment as done. Closed by commit rG60f3b071185b: [clang][analyzer] Add checker for bad use of 'errno'. (authored by balazske). Repository:

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:553-554 + CheckerOptions<[ +CmdLineOption Well, now I see why you marked this `Hide` previously. > This is a //modeling// checker, t

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal accepted this revision. steakhal added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:47 + /// Indicates if a read (load) of \c errno is allowed in a non-condition part + /// of \c if, \c

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 437922. balazske marked 16 inline comments as done. balazske added a comment. Removed Errno_ prefix from ErrnoCheckState, documentation fixes, other small fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added a comment. Awesome! The generated doc section looks great. The test coverage it excellent, but I would recommend adding tests for covering the following lines: - `ErrnoChecker.cpp:99` - `ErrnoModeling.cpp:259` - `ErrnoModeling.cpp:264` There are two other uncovered cases, but th

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:73 +REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoState, ErrnoCheckState) + You will need this to get it compiled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added a comment. It looks great. Let me check the test coverage and the generated docs page it everything looks great. Comment at: clang/docs/analyzer/checkers.rst:2538 +may change value of ``errno`` if the call does not fail. +Therefore ``errno`` should only be used

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 437513. balazske added a comment. Use typed state trait. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150 Files: clang/docs/ReleaseNotes.rst clang/docs/analyzer/c

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2538-2539 +yet supported by the checker. +The return values for the failure cases are documented in the standard Linux man +pages of the functions. + steakhal wrote: > Maybe place a cro

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 437471. balazske marked 7 inline comments as done. balazske added a comment. applied review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150 Files: clang/d

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added a comment. I believe `evalAssume` would be a better fit for diagnosing such issues, but I can see your point that we don't have `CheckerContext` there to emit reports. That being said, the `check::Location` is the best alternative. Please also check the not `done` comments. ===

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

martong accepted this revision. martong added a comment. LGTM, it is a good start for an alpha checker. Comment at: clang/docs/ReleaseNotes.rst:525 +- Added a new checker ``alpha.unix.Errno``. This can find the first read + of ``errno`` after not failed standard function calls

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 435557. balazske marked 3 inline comments as done. balazske added a comment. fixed documentation, added release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

martong added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2553-2555 +The supported functions are the same that are modeled by checker +:doc:`alpha.unix.StdCLibraryFunctionArgs` (and affect value of ``errno``), +including effect of the ``ModelPOSIX`` option of th

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:558 + InAlpha, + Hide>, // ?? + ]>, steakhal wrote: > I think we have 'Hidden', but not 'Hide'. > It means that it is a modeling

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 435081. balazske marked 5 inline comments as done. balazske added a comment. applied review comments, test improvement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added a comment. D126801 should simplify the reinterpret casts. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:558 + InAlpha, + Hide>, // ?? + ]>, I think we ha

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:554 +CmdLineOptionhttps://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 433046. balazske added a comment. rebase, fix of failing tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150 Files: clang/include/clang/StaticAnalyzer/Checkers/C

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 432929. balazske added a comment. added option to allow read-only of `errno` other small change in the test checker Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122150/new/ https://reviews.llvm.org/D122150 F

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

martong added a comment. In D122150#3408156 , @balazske wrote: > This errno check will work if a state split is added at every standard > function that may set `errno` at failure. (The success and failure branches > have different //errno check state//,

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske added a comment. This errno check will work if a state split is added at every standard function that may set `errno` at failure. (The success and failure branches have different //errno check state//, and probably different return value for the function.) Probably this is too many sta

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske marked 2 inline comments as done. balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:545 + Dependencies<[ErrnoModeling]>, + Documentation; + steakhal wrote: > Then we should have documentation, and examp

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske updated this revision to Diff 418238. balazske marked 11 inline comments as done. balazske added a comment. Address review comments. - Removed caching of errno-related values from the checker. - Added note tags. - Added documentation comments. Repository: rG LLVM Github Monorepo CHA

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:68-71 + if (ErrnoLoc) { +ErrnoRegion = ErrnoLoc->getAsRegion(); +assert(ErrnoRegion && "The 'errno' location should be a memory region."); + } NoQ wrote: > st

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:47-49 + mutable bool ErrnoInitialized = false; + mutable Optional ErrnoLoc; + mutable const MemRegion *ErrnoRegion = nullptr; steakhal wrote: > One should not define memb

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I'm liking it. We need to improve the diagnostics and the user-facing docs as well. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:545 +

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske added reviewers: martong, steakhal, NoQ. balazske added a comment. Herald added a subscriber: rnkovacs. This checker is made to add a partial support for CERT rule ERR30-C . One part of the rule is "check errno

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

balazske created this revision. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. Herald added a reviewer: Szelethus. Herald added a project: All. balazske request