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

[Gabor Marton via Phabricator via cfe-commits]

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 wrote:
> 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.)
Okay, I see your reasoning. However, we might never know how the constraint solver reasons about it, thus we should not rely on that. As a contrived example, it might check recursively that the LHS of the BinOp you just conjured and it might recognize that the State is infeasible.


================
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 = "") {
----------------
balazske wrote:
> 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.
Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400