[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for patch and the many test cases.

You still haven't answered my below concern:

> Is there a way to stop the analysis with an error if "ModelPOSIX=true" is not set but the 'alpha.unix.Stream' checker is enabled?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1045
           NT = Case.getErrnoConstraint().describe(C, D->getNameAsString());
+          // llvm::errs()<<D->getNameAsString();
+        }
----------------
Please remove this line (leftover of some debugging?).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1060-1064
+    } else if (NewState == State) {
+      if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
+        if (const NoteTag *NT =
+                Case.getErrnoConstraint().describe(C, D->getNameAsString()))
+          C.addTransition(NewState, NT);
----------------
Why do we need this change?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1724-1797
+    const auto ReturnsZeroOrMinusOne =
+        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
+    const auto ReturnsZero =
+        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(0))};
+    const auto ReturnsMinusOne =
+        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(-1))};
+    const auto ReturnsNonnegative =
----------------
Very good!


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:604-613
+  // Return 0 on success, EOF on failure.
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef StateSuccess = State->BindExpr(
+      CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy));
+  ProgramStateRef StateFailure =
+      State->BindExpr(CE, C.getLocationContext(),
+                      SVB.makeIntVal(*EofVal, C.getASTContext().IntTy));
----------------
This is redundant with the summary in the `StdLibraryFunctionsChecker`. Why do we need this as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247