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

[Balázs Kéri via Phabricator via cfe-commits]

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`. For example: "Assuming that 'mkdir' is successful, it returns zero in this case and value of 'errno' is unspecified after the call".



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:435
+      assert(Cond);
+      State = State->assume(*Cond, true);
+      return errno_modeling::setErrnoValue(State, C.getLocationContext(),
----------------
martong wrote:
> Please check if `State` can be nullptr.
I think here it is never null. A relation is created between a new conjured symbol and zero, this can never fail, or not? (The `ErrnoSVal` should be here a newly created symbol. If the `Tag` is not used it may be the same symbol that was previously bound to the expression if `EvalCallAsPure` is used.)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:603
 
-    Summary &Case(ConstraintSet &&CS, StringRef Note = "") {
-      Cases.push_back(SummaryCase(std::move(CS), Note));
+    Summary &Case(ConstraintSet &&CS, const ErrnoConstraintKind &ErrnoC,
+                  StringRef Note = "") {
----------------
martong wrote:
> Would it make sense to have a `ErrnoIrrelevant` as the default value for `ErrnoC`?
It would be a bit more convenient but in the current design it is not possible to pass the member variable as default value. `ErrnoIrrelevant` is really used in less than half of the cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125400/new/

https://reviews.llvm.org/D125400