[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG957014da2d27: [clang][Analyzer] Add errno state to standard functions modeling. (authored by balazske). Repository: rG LLVM Github Monorepo

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske marked an inline comment as done. balazske added a comment. I did performance measurements in the past and it showed no important difference, run time got more with at most some percent at some of the projects. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM, good job. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 437924. balazske marked 2 inline comments as done. balazske added a comment. Small fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal added a comment. In the `describeErrnoCheckState()` the `Errno_MustBeChecked` is uncovered by tests. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:410 +/// Many of the descendant classes use this value. +

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal added a comment. It looks everything fine, given that the `alpha.unix.Errno` will depend on the `apimodeling.StdCLibraryFunctions`. This should do the expected errno state propagation by which the `Errno` checker can report diagnostics - WHILE the `StdCLibraryFunctions` still won't

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added inline comments. Comment at: clang/test/Analysis/errno-stdlibraryfunctions-notes.c:15-23 +void test1() { + access("path", 0); // no note here + access("path", 0); + // expected-note@-1{{Assuming that function 'access' is successful, in this case the value

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske added a comment. The documentation of `ErrnoChecker` is added in the previous change D122150 . Really this documentation should be updated when any checker is changed related to errno modeling (errno modeling can be added later to any checker that

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. It's getting in really good shape. Nice work. I've got some nits about docs and checker dependencies and also a major comment inline. The documentation needs to be updated to

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 435558. balazske added a comment. small fix in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske marked 2 inline comments as done. balazske added inline comments. Comment at: clang/test/Analysis/errno-stdlibraryfunctions-notes.c:11 + +int access(const char *path, int amode); + martong wrote: > Do you plan to extend the test file for other standard

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added inline comments. Comment at: clang/test/Analysis/errno-stdlibraryfunctions-notes.c:11 + +int access(const char *path, int amode); + Do you plan to extend the test file for other standard functions as well? Comment at:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 435082. balazske added a comment. small test change, removed a compile warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 433047. balazske added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 432930. balazske added a comment. applied the review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. Nice work, looks promising! Once this https://reviews.llvm.org/D125400?vs=431909=431924#inline-1206369 is addressed I will accept. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2273 +// here) may indicate failure.

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 431924. balazske edited the summary of this revision. balazske added a comment. fixed some problems Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:454-460 + std::string Result = "Assuming that function '"; + Result.append(FunctionName.str()); + Result.append("' fails, in this case the value 'errno'

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 431909. balazske marked 13 inline comments as done. balazske added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:412 + class ZeroRelatedErrnoConstraint : public ErrnoConstraintKind { +clang::BinaryOperatorKind Relation; + The explicit qualification is not

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske updated this revision to Diff 431652. balazske added a comment. added comments. added note tag, added test, improved the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125400/new/ https://reviews.llvm.org/D125400 Files:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:435 + assert(Cond); + State = State->assume(*Cond, true); + return errno_modeling::setErrnoValue(State, C.getLocationContext(), balazske

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. In D125400#3516164 , @balazske wrote: > @martong Do you mean that a "describe" function is needed for the return > value constraint of the function and for the errno constraint type? Then a > note tag can contain what the

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. In D125400#3508955 , @balazske wrote: > Function `mkdir` is modeled incorrectly by the checker. According to the man > page it can return 0 or -1 only (-1 is error) but the checker allows > non-negative value at success. So the

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske added a comment. @martong Do you mean that a "describe" function is needed for the return value constraint of the function and for the errno constraint type? Then a note tag can contain what the function is assumed to return on success and what is allowed (or not) to do with `errno`.

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

balazske added a comment. Function `mkdir` is modeled incorrectly by the checker. According to the man page it can return 0 or -1 only (-1 is error) but the checker allows non-negative value at success. So the shown bug report is incorrect (it can be only -1 if not 0 and then check of `errno`

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. The bugreports look promising. However, I think we desperately need a note that describes which function has set the `errno_modeling` state. Below I'd expect the following notes for the highlighted function call. - assuming return value of `mkdir` is in range [0,

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. Nice work! Could you pleas add some lit tests that describe an errno related bugreport for a standard lib function? Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:381 + class ErrnoConstraintKind { + public:

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

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