balazske marked 5 inline comments as done. balazske added a comment. > I suppose the tests are done in the followup patch mostly?
Yes the tests in the next patch should cover the cases. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838 + Result += getArgDesc(ArgN); + Result += DK == Violation ? " should not be zero" : " is not zero"; + return Result.c_str(); ---------------- Szelethus wrote: > I don't mean to make you test every single `Case` or `ArgumentConstraint` you > added, but `NotZeroConstraint` is brand new, and is not tested anywhere. This new class is probably not needed at all. It is always possible to use `ReturnValueCondition(WithinRange, SingleValue(0))` or similar for nonzero cases. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1105-1113 + // It is possible that the function was evaluated in a checker callback + // where the state constraints are already applied, then no change happens + // here to the state (if the ErrnoConstraint did not change it either). + // If the evaluated function requires a NoteTag for errno change, it is + // added here. + if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl())) + if (const NoteTag *NT = ---------------- Szelethus wrote: > Can you name an example for that? `fgetpos`? `fgetpos` is a good example, the return value is already bound and a state split for success and failure was performed in the `evalCall` function (in `StreamChecker`) and the previous line `Case.getErrnoConstraint().apply` did not create a new state (this is true for `fgetpos` because it has a `ErrnoUnchanged` condition in the summary). ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1848-1849 + // int fseek(FILE *stream, long offset, int whence); + // FIXME: It is possible to get the 'SEEK_' values (like EOFv) for arg 2 + // condition. + addToFunctionSummaryMap( ---------------- Szelethus wrote: > Sure, but what should be fixed? We should check whether the the last argument > is a `SEEK_*` value? It could be an improvement to detect possible values for the argument 2 that are the `SEEK_*` values. Now the range [0,2] is used (the `SEEK_*` constants are usually 0,1,2). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140387/new/ https://reviews.llvm.org/D140387 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
