[clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

[balazske]

Summary

Add support for functions 'fgetpos', 'fsetpos', 'ftell', 'rewind'.

[balazske]

I found some anomalies during development:

• If the checker StdCLibraryFunctions is added as dependency for alpha.unix.Stream in checkers.td I get some "unexplainable" test failures.
• In the comments section at CERT ERR30-C is pointed out that function ftell can change the value of errno if it is successful. This is the "normal" expected behavior of all standard functions unless the documentation tells other. But this place (search for ftell) tells that ftell should not change errno if successful. The persons commented at the CERT rule probably checked only one or more of the C standards. But there should be no conflict between POSIX and C ("Any conflict between the requirements described here and the ISO C standard is unintentional."). It would be unconventional if different check rules are needed (and add of a global analyzer option for POSIX or C mode).

[martong]

In D135360#3839890, @balazske wrote:

I found some anomalies during development:

• If the checker StdCLibraryFunctions is added as dependency for alpha.unix.Stream in checkers.td I get some "unexplainable" test failures.

Could you please elaborate? I don't see how to help you with it without seeing more details.

• In the comments section at CERT ERR30-C is pointed out that function ftell can change the value of errno if it is successful. This is the "normal" expected behavior of all standard functions unless the documentation tells other. But this place (search for ftell) tells that ftell should not change errno if successful. The persons commented at the CERT rule probably checked only one or more of the C standards. But there should be no conflict between POSIX and C ("Any conflict between the requirements described here and the ISO C standard is unintentional."). It would be unconventional if different check rules are needed (and add of a global analyzer option for POSIX or C mode).

When it is ambiguous then I'd check the latest standards of both POSIX and C. If it is still doubtful then I'd vote for the C standard and would report a defect towards the POSIX community.

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1818–1857

It is very good to see these new summaries! Would they be meaningful and usable without the changes in the StreamChecker? If yes, then I think we should split this into 2 patches.

[balazske]

If we look only at the C standard we can not tell much about if the functions should set errno. It seems that setting errno is totally implementation-dependent, except for a few functions that mention errno. These are fgetpos, fsetpos, ftell and should set errno to a positive value on error (but there is no guarantee that value of errno is preserved if no error occurs). And errno is always positive. This is different than what the checker currently does (with the new patches). Probably this should be a discourse question?

[martong]

In D135360#3885494, @balazske wrote:

If we look only at the C standard we can not tell much about if the functions should set errno. It seems that setting errno is totally implementation-dependent, except for a few functions that mention errno. These are fgetpos, fsetpos, ftell and should set errno to a positive value on error (but there is no guarantee that value of errno is preserved if no error occurs). And errno is always positive. This is different than what the checker currently does (with the new patches). Probably this should be a discourse question?

Okay then, I think it is worth to have a discourse question. But you could ask the wider "Clang" community, so I would not post the question as something that is related strictly to the static analyzer.

[balazske]

About the "ftell" problem: The POSIX rules are really an extension of the C standard rules. At ftell according to C standard errno should be set to a positive value if error occurs. The POSIX rules extend this: errno is not changed if no error occurs. This can be correct, then the CERT ftell non-compliant example is wrong (on success errno remains always 0). It can be fixed if line errno=0 is removed from the start of the non-compliant code. It may be best to use the POSIX rules for the checker, because the C standard does not say much and may need to require setting of errno to 0 before a standard function call.

[balazske]

Added more functions to StdLibraryFunctionsChecker.
This is redundant with the StreamChecker for now but
makes possible to remove the NULL stream check from
StreamChecker and reduce overlap of the checkers.
This way no dependency should be needed between these checkers.

[balazske]

Rebase to main and D137722.

[Szelethus]

In D135360#3862260, @martong wrote:

In D135360#3839890, @balazske wrote:

I found some anomalies during development:

• If the checker StdCLibraryFunctions is added as dependency for alpha.unix.Stream in checkers.td I get some "unexplainable" test failures.

Could you please elaborate? I don't see how to help you with it without seeing more details.

Mind that dependencies also establish the order of callbacks (dependent checkers are called after their dependencies).

When it is ambiguous then I'd check the latest standards of both POSIX and C. If it is still doubtful then I'd vote for the C standard and would report a defect towards the POSIX community.

In D135360#3888632, @balazske wrote:

About the "ftell" problem: The POSIX rules are really an extension of the C standard rules. At ftell according to C standard errno should be set to a positive value if error occurs. The POSIX rules extend this: errno is not changed if no error occurs. [...] It may be best to use the POSIX rules for the checker, because the C standard does not say much and may need to require setting of errno to 0 before a standard function call.

Interesting pair of perspectives, I think a reasonable checker should be a little more conservative, more akin to what POSIX seems to specify.

In D135360#3885556, @martong wrote:

In D135360#3885494, @balazske wrote:

[...] Probably this should be a discourse question?

Okay then, I think it is worth to have a discourse question. But you could ask the wider "Clang" community, so I would not post the question as something that is related strictly to the static analyzer.

Did this thread ever materialize? I admit I didn't follow :)

[balazske]

My current approach is that the POSIX is more strict than the C standard (POSIX allows a subset of what C allows). I do not see (errno related) contradiction between these standards (except initialization to zero of errno missing from POSIX, and possible negative value in errno in POSIX). The checker currently works only in POSIX mode for every function, the ModelPOSIX setting controls only if additional functions are added (all with POSIX rules) (these functions could be added with C rules too). One problem exists now, that errno is set to a non-zero (but not positive only) value at error. Probably we should change this to be always positive at error, according to the C standard errno is positive only, and POSIX does not tell that errno can be negative.

[Szelethus]

In D135360#3981420, @balazske wrote:

My current approach is that the POSIX is more strict than the C standard (POSIX allows a subset of what C allows). I do not see (errno related) contradiction between these standards

Alright, so if we wanna support the C standard thinking, we would just create an option that would be a little more noisy. Sounds fair enough.

(except initialization to zero of errno missing from POSIX, and possible negative value in errno in POSIX). One problem exists now, that errno is set to a non-zero (but not positive only) value at error. Probably we should change this to be always positive at error, according to the C standard errno is positive only, and POSIX does not tell that errno can be negative.

According to this (which I assume you've seen as well): https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

Description

[...]
Some of the functionality described on this reference page extends the ISO C standard. Any conflict between the requirements described here and the ISO C standard is unintentional. [...]
[...]

Issue 6

Values for errno are now required to be distinct positive values rather than non-zero values. This change is for alignment with the ISO/IEC 9899:1999 standard.

So yes, I'd say its reasonable to assume in POSIX modthat if errno is non-zero, it is positive.

The checker currently works only in POSIX mode for every function, the ModelPOSIX setting controls only if additional functions are added (all with POSIX rules) (these functions could be added with C rules too).

I see a lot of mention of POSIX vs. the C standard, yet there seems to be no mention of these differences in the comments added by this patch or already commited. Maybe we could do better on documenting this important difference in behaviour.

In D135360#3981369, @Szelethus wrote:

In D135360#3885556, @martong wrote:

Okay then, I think it is worth to have a discourse question. But you could ask the wider "Clang" community, so I would not post the question as something that is related strictly to the static analyzer.

Did this thread ever materialize? I admit I didn't follow :)

I suppose the answer is no then :)

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1818–1857

Can you please do this?

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1818–1857

This is done now. There were 2 consecutive patches that modify StdLibraryFunctionsChecker and other checkers. These are rearranged into 2 new patches. One for all changes in StdLibraryFunctionsChecker (and related changes in ErrnoModeling): D140387. Other for all other changes, StreamChecker and many of the tests that exercise both checkers: D140395.

clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h

 /// Set \c errno value to be not equal to zero and \c ErrnoCheckState to
 /// \c Irrelevant . The irrelevant errno state ensures that no related bug
 /// report is emitted later and no note tag is needed.
 /// \arg \c ErrnoSym Value to be used for \c errno and constrained to be
 /// non-zero.
 ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
                                       NonLoc ErrnoSym);
 
+/// Set errno state for the common case when a standard function indicates
+/// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
+/// invalidates the errno region (clear of previous value).
+/// At the state transition a note tag created by
+/// \c getNoteTagForStdMustBeChecked can be used.
+/// \arg \c InvalE Expression that causes invalidation of \c errno.
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+                                         CheckerContext &C, const Expr *InvalE);
+
 /// Generate the note tag that can be applied at the state generated by
 /// \c setErrnoForStdSuccess .
 /// \arg \c Fn Name of the (standard) function that is modeled.
 const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
 
+/// Generate the note tag that can be applied at the state generated by
+/// \c setErrnoStdMustBeChecked .
+/// \arg \c Fn Name of the (standard) function that is modeled.
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+                                             llvm::StringRef Fn);
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
 
 #endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ERRNOMODELING_H

clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp

DefinedOrUnknownSVal Cond =
       SVB.evalBinOp(State, BO_NE, ErrnoSym, ZeroVal, SVB.getConditionType())
           .castAs<DefinedOrUnknownSVal>();
   State = State->assume(Cond, true);
   if (!State)
     return nullptr;
   return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
 }
 
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+                                         CheckerContext &C,
+                                         const Expr *InvalE) {
+  const MemRegion *ErrnoR = State->get<ErrnoRegion>();
+  if (!ErrnoR)
+    return State;
+  State = State->invalidateRegions(ErrnoR, InvalE, C.blockCount(),
+                                   C.getLocationContext(), false);
+  if (!State)
+    return nullptr;
+  return setErrnoState(State, MustBeChecked);
+}
+
 const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
   return getErrnoNoteTag(
       C, (Twine("Assuming that function '") + Twine(Fn) +
           Twine("' is successful, in this case the value 'errno' ") +
           Twine(describeErrnoCheckState(MustNotBeChecked)))
              .str());
 }
 
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+                                             llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+      C, (Twine("Function '") + Twine(Fn) +
+          Twine("' indicates failure only by setting of 'errno'"))
+             .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
 
 void ento::registerErrnoModeling(CheckerManager &mgr) {
   mgr.registerChecker<ErrnoModeling>();
 }
 
 bool ento::shouldRegisterErrnoModeling(const CheckerManager &mgr) {
   return true;
 }

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

public:
     }
 
     const NoteTag *describe(CheckerContext &C,
                             StringRef FunctionName) const override {
       return errno_modeling::getNoteTagForStdSuccess(C, FunctionName);
     }
   };
 
+  class ErrnoMustBeCheckedConstraint : public ErrnoConstraintBase {
+  public:
+    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
+      return errno_modeling::setErrnoStdMustBeChecked(State, C,
+                                                      Call.getOriginExpr());
+    }
+
+    const NoteTag *describe(CheckerContext &C,
+                            StringRef FunctionName) const override {
+      return errno_modeling::getNoteTagForStdMustBeChecked(C, FunctionName);
+    }
+  };
+
   /// A single branch of a function summary.
   ///
   /// A branch is defined by a series of constraints - "assumptions" -
   /// that together form a single possible outcome of invoking the function.
   /// When static analyzer considers a branch, it tries to introduce
   /// a child node in the Exploded Graph. The child node has to include
   /// constraints that define the branch. If the constraints contradict
   /// existing constraints in the state, the node is not created and the branch
private:
 
   /// These are the errno constraints that can be passed to summary cases.
   /// One of these should fit for a single summary case.
   /// Usually if a failure return value exists for function, that function
   /// needs different cases for success and failure with different errno
   /// constraints (and different return value constraints).
   const NoErrnoConstraint ErrnoUnchanged{};
   const ResetErrnoConstraint ErrnoIrrelevant{};
+  const ErrnoMustBeCheckedConstraint ErrnoMustBeChecked{};
   const SuccessErrnoConstraint ErrnoMustNotBeChecked{};
   const FailureErrnoConstraint ErrnoNEZeroIrrelevant{};
 };
 
 int StdLibraryFunctionsChecker::ErrnoConstraintBase::Tag = 0;
 
 const StdLibraryFunctionsChecker::ArgNo StdLibraryFunctionsChecker::Ret =
     std::numeric_limits<ArgNo>::max();
void StdLibraryFunctionsChecker::initFunctionSummaries(
   };
   auto LessThanOrEq = BO_LE;
   auto NotNull = [&](ArgNo ArgN) {
     return std::make_shared<NotNullConstraint>(ArgN);
   };
   auto IsNull = [&](ArgNo ArgN) {
     return std::make_shared<NotNullConstraint>(ArgN, false);
   };
+  auto NotZero = [&](ArgNo ArgN) {
+    return std::make_shared<NotZeroConstraint>(ArgN);
+  };
+  auto IsZero = [&](ArgNo ArgN) {
+    return std::make_shared<NotZeroConstraint>(ArgN, false);
+  };
 
   Optional<QualType> FileTy = lookupTy("FILE");
   Optional<QualType> FilePtrTy = getPointerTy(FileTy);
   Optional<QualType> FilePtrRestrictTy = getRestrictTy(FilePtrTy);
 
+  Optional<QualType> FPosTTy = lookupTy("fpos_t");
+  Optional<QualType> FPosTPtrTy = getPointerTy(FPosTTy);
+  Optional<QualType> ConstFPosTPtrTy = getPointerTy(getConstTy(FPosTTy));
+  Optional<QualType> FPosTPtrRestrictTy = getRestrictTy(FPosTPtrTy);
+
   // We are finally ready to define specifications for all supported functions.
   //
   // Argument ranges should always cover all variants. If return value
   // is completely unknown, omit it from the respective range set.
   //
   // Every item in the list of range sets represents a particular
   // execution path the analyzer would need to explore once
   // the call is modeled - a new program state is constructed
addToFunctionSummaryMap(
         Summary(NoEvalCall)
             .Case({ReturnValueCondition(WithinRange, SingleValue(0))},
                   ErrnoMustNotBeChecked)
             .Case({ReturnValueCondition(WithinRange, SingleValue(-1))},
                   ErrnoNEZeroIrrelevant)
             .ArgConstraint(NotNull(ArgNo(0)))
             .ArgConstraint(ArgumentCondition(2, WithinRange, {{0, 2}})));
 
+    // int fgetpos(FILE *restrict stream, fpos_t *restrict pos);
+    // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+    // "The fgetpos() function shall not change the setting of errno if
+    // successful."
+    addToFunctionSummaryMap(
+        "fgetpos",
+        Signature(ArgTypes{FilePtrRestrictTy, FPosTPtrRestrictTy},
+                  RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case({IsZero(ArgNo(Ret))}, ErrnoUnchanged)
+            .Case({NotZero(ArgNo(Ret))}, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0)))
+            .ArgConstraint(NotNull(ArgNo(1))));
+
+    // int fsetpos(FILE *stream, const fpos_t *pos);
+    // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+    // "The fsetpos() function shall not change the setting of errno if
+    // successful."
+    addToFunctionSummaryMap(
+        "fsetpos",
+        Signature(ArgTypes{FilePtrTy, ConstFPosTPtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case({IsZero(ArgNo(Ret))}, ErrnoUnchanged)
+            .Case({NotZero(ArgNo(Ret))}, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0)))
+            .ArgConstraint(NotNull(ArgNo(1))));
+
+    // long ftell(FILE *stream);
+    // From 'The Open Group Base Specifications Issue 7, 2018 edition':
+    // "The ftell() function shall not change the setting of errno if
+    // successful."
+    addToFunctionSummaryMap(
+        "ftell", Signature(ArgTypes{FilePtrTy}, RetType{LongTy}),
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, Range(1, LongMax))},
+                  ErrnoUnchanged)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(-1))},
+                  ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0))));
+
     // int fileno(FILE *stream);
     addToFunctionSummaryMap(
         "fileno", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
             .ArgConstraint(NotNull(ArgNo(0))));
 
+    // void rewind(FILE *stream);
+    // This function indicates error only by setting of 'errno'.
+    addToFunctionSummaryMap("rewind",
+                            Signature(ArgTypes{FilePtrTy}, RetType{VoidTy}),
+                            Summary(NoEvalCall)
+                                .Case({}, ErrnoMustBeChecked)
+                                .ArgConstraint(NotNull(ArgNo(0))));
+
+    // void clearerr(FILE *stream);
+    addToFunctionSummaryMap(
+        "clearerr", Signature(ArgTypes{FilePtrTy}, RetType{VoidTy}),
+        Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
+    // int feof(FILE *stream);
+    addToFunctionSummaryMap(
+        "feof", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
+    // int ferror(FILE *stream);
+    addToFunctionSummaryMap(
+        "ferror", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
+
     // long a64l(const char *str64);
     addToFunctionSummaryMap(
         "a64l", Signature(ArgTypes{ConstCharPtrTy}, RetType{LongTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
     // char *l64a(long value);
     addToFunctionSummaryMap("l64a",
                             Signature(ArgTypes{LongTy}, RetType{CharPtrTy}),

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

CallDescriptionMap<FnDescription> FnDescriptions = {
        {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
       {{"fread", 4},
        {&StreamChecker::preFread,
         std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
       {{"fwrite", 4},
        {&StreamChecker::preFwrite,
         std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
       {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
-      {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+      {{"ftell", 1},
+       {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+      {{"rewind", 1},
+       {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
+      {{"fgetpos", 2},
+       {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
+      {{"fsetpos", 2},
+       {&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
       {{"clearerr", 1},
        {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
       {{"feof", 1},
        {&StreamChecker::preDefault,
         std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
         0}},
       {{"ferror", 1},
        {&StreamChecker::preDefault,
private:
   void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
                        CheckerContext &C, bool IsFread) const;
 
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
                 CheckerContext &C) const;
   void evalFseek(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
 
+  void evalFgetpos(const FnDescription *Desc, const CallEvent &Call,
+                   CheckerContext &C) const;
+
+  void evalFsetpos(const FnDescription *Desc, const CallEvent &Call,
+                   CheckerContext &C) const;
+
+  void evalFtell(const FnDescription *Desc, const CallEvent &Call,
+                 CheckerContext &C) const;
+
+  void evalRewind(const FnDescription *Desc, const CallEvent &Call,
+                  CheckerContext &C) const;
+
   void preDefault(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
   void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
                     CheckerContext &C) const;
 
   void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
                       CheckerContext &C,
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   StateFailed = StateFailed->set<StreamMap>(
       StreamSym,
       StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
 
   C.addTransition(StateNotFailed);
   C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
 }
 
+void StreamChecker::evalFgetpos(const FnDescription *Desc,
+                                const CallEvent &Call,
+                                CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!Sym)
+    return;
+
+  // Do not evaluate if stream is not found.
+  if (!State->get<StreamMap>(Sym))
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  DefinedSVal RetVal = makeRetVal(C, CE);
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateFailed, StateNotFailed) =
+      C.getConstraintManager().assumeDual(State, RetVal);
+
+  // This function does not affect the stream state.
+  // Still we add success and failure state with the appropriate return value.
+  // StdLibraryFunctionsChecker can change these states (with 'errno').
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFsetpos(const FnDescription *Desc,
+                                const CallEvent &Call,
+                                CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  if (!SS)
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  DefinedSVal RetVal = makeRetVal(C, CE);
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateFailed, StateNotFailed) =
+      C.getConstraintManager().assumeDual(State, RetVal);
+
+  StateNotFailed = StateNotFailed->set<StreamMap>(
+      StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+
+  // At failure ferror could set.
+  // The standards do not tell what happens with the file position at failure.
+  // But we can assume that it is dangerous to make a next I/O operation after
+  // the position was not set correctly (similar to 'fseek').
+  StateFailed = StateFailed->set<StreamMap>(
+      StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
+                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!Sym)
+    return;
+
+  if (!State->get<StreamMap>(Sym))
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+  ProgramStateRef StateNotFailed =
+      State->BindExpr(CE, C.getLocationContext(), RetVal);
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
+                            SVB.makeZeroVal(C.getASTContext().LongTy),
+                            SVB.getConditionType())
+                  .getAs<DefinedOrUnknownSVal>();
+  if (!Cond)
+    return;
+  StateNotFailed = StateNotFailed->assume(*Cond, true);
+  if (!StateNotFailed)
+    return;
+
+  ProgramStateRef StateFailed = State->BindExpr(
+      CE, C.getLocationContext(), SVB.makeIntVal(-1, C.getASTContext().LongTy));
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalRewind(const FnDescription *Desc, const CallEvent &Call,
+                               CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  if (!SS)
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  State = State->set<StreamMap>(StreamSym,
+                                StreamState::getOpened(Desc, ErrorNone, false));
+
+  C.addTransition(State);
+}
+
 void StreamChecker::evalClearerr(const FnDescription *Desc,
                                  const CallEvent &Call,
                                  CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
   if (!StreamSym)
     return;
 

clang/test/Analysis/stream-errno-note.c

if (!F)
     return;
   (void)fseek(F, 11, SEEK_SET);
   // expected-note@-1{{Assuming that function 'fseek' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note@-1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
 }
 
+void check_rewind_errnocheck(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  errno = 0;
+  rewind(F); // expected-note{{Function 'rewind' indicates failure only by setting of 'errno'}}
+  fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
+  // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
+}
+
 void check_fileno(void) {
   FILE *F = tmpfile();
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
     return;
   fileno(F);
   // expected-note@-1{{Assuming that function 'fileno' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note@-1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
 }

clang/test/Analysis/stream-errno.c

void check_no_errno_change(void) {
   feof(F);
   if (errno) {} // no-warning
   ferror(F);
   if (errno) {} // no-warning
   clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}}
   fclose(F);
 }
 
+void check_fgetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_fsetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_ftell(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_rewind(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  rewind(F);
+  clang_analyzer_eval(errno == 0);
+  // expected-warning@-1{{FALSE}}
+  // expected-warning@-2{{TRUE}}
+  fclose(F);
+}
+
 void check_fileno(void) {
   FILE *F = tmpfile();
   if (!F)
     return;
   int N = fileno(F);
   if (N == -1) {
     clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
     if (errno) {} // no-warning
     fclose(F);
     return;
   }
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
 }

clang/test/Analysis/stream-noopen.c

void test_fseek(FILE *F) {
   } else {
     clang_analyzer_eval(Ret == -1); // expected-warning {{TRUE}}
     clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
   }
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
 }
 
+void check_fgetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                     // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fsetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                     // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_ftell(FILE *F) {
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                     // expected-warning@-1{{FALSE}}
+    clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
 void freadwrite_zerosize(FILE *F) {
   fwrite(WBuf, 1, 0, F);
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
   if (errno) {} // no-warning
   fwrite(WBuf, 0, 1, F);
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}