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

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

balazske added a comment.

Probably if the `StdLibraryFunctionsChecker` object can be get from `StreamChecker` (a new function is needed) it is possible to check the option and use `CheckerManager::reportInvalidCheckerOptionValue` (for the `StdLibraryFunctionsChecker`). But not sure if it does what we want here.



================
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);
----------------
martong wrote:
> Why do we need this change?
It is possible that only the errno related state is changed, no new constraints are added (if the constraint is already here from `evalCall` but the errno was not set there, for example at `fclose` or other stream functions maybe no new state is created here). In such case the note tag is still needed.


================
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));
----------------
martong wrote:
> This is redundant with the summary in the `StdLibraryFunctionsChecker`. Why do we need this as well?
It is probably needed to have a (any) bound value to the `fclose` function call, otherwise setting constraints in the other checker do not work. It may work to bind only a conjured value but it looks better if the correct return value is used. This makes less inter-dependence between the two checkers (`StdLibraryFunctionChecker` sets only the errno state as far as possible).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247