[Analyzer][StreamChecker] Added evaluation of fseek.

[balazske]

Summary

Function fseek is now evaluated with setting error return value
and error flags.

[Szelethus]

We should totally dedicate an error kind of fseek, see (and please respond, if you could to) D75356#inline-689287.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

492–494

This seems a bit excessive, we could merge the last two into FSeekError.

[balazske]

I do not like to make difference between error from fseek and error from any other function that may fail. AnyError can be used to handle every case. After a failed file operation we do not know if it was feof or ferror kind of error. fseek is only special because it can fail additionally without feof or ferror. If the kind of the last operation needs to be known later, it can be saved in the stream state.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

492–494

There are 3 cases:

• fseek did not fail at all. Return value is zero. This is StateNotFailed.
• fseek failed but none of the error flags is true afterwards. Return value is nonzero but feof and ferror are not true. This is StateFailedWithoutFError.
• fseek failed and we have feof or ferror set (never both). Return value is nonzero and feof or ferror will be true. This is StateFailedWithFError. And an use of AnyError, otherwise we need here 2 states, one for feof and one for ferror. But it is not important here if feof or ferror is actually true, so the special value AnyError is used and only one new state instead of two.

[Szelethus]

In D75851#1914874, @balazske wrote:

I do not like to make difference between error from fseek and error from any other function that may fail. AnyError can be used to handle every case.

Okay, it took me a while, but I think I understand where you're coming from. Lets preserve this enum. I think NoteTags would be a better fit to pinpoint which function left the stream in a state that should be checked before usage. This patch is quite small, could we add one then?

After a failed file operation we do not know if it was feof or ferror kind of error.

Is this true for that many stream operations?

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

492–494

Sure, but from the point of the analyzer the latter two are the same, isn't it? Its never a good idea to use a stream on which fseek failed without checking.

State splits are the most expensive things the analyzer can make, which is why I'm cautious here.

[Szelethus]

In D75851#1915014, @Szelethus wrote:

In D75851#1914874, @balazske wrote:

After a failed file operation we do not know if it was feof or ferror kind of error.

Is this true for that many stream operations?

I looked it up, and it totally is.

[balazske]

Probably we need to clarify when to make warnings. Originally I do not wanted a warning after for example failed read, because it is possible to retry the operation. But the standard says that after failed fread the file position is indeterminate, so a new read may get indeterminate values that is a case for warning. The character I/O functions may work not this way so after failed fgetc it may be possible to retry it and no warning is needed. Some operations such as fclose and fseek can be called in error (including EOF) state.

For this to work we need more kinds of error states, or save the type of the last operation in the state (and use it to determine if warning is needed). Probably the best is to have only a single "failed" state and determine what kind of error is possible based on the saved last operation type. A "feof" and "ferror" kind of error state is still needed because after a call to feof if it returned true the error type is fixed to EOF error.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

492–494

Somehow, this should be done with just two new states. Maybe there should be an error state "Unknown" instead of OtherError (or FeoFOrFError what I suggested at the other patch) which can be any of the errors plus the NoError value. Later, when ferror() of feof() is checked we can do a second state split, but not earlier.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

492–494

Yes it is possible to use two new states only. For example by having a new FSeekError that can "manifest" in feof, ferror or no error. Better is to have only one "last-operation-failed-with-any-error" state and determine later if needed what errors are possible (based on the last operation). For fseek it is possible to get feof, ferror, or none of these (but still failed fseek). For reading operations the error can be feof or ferror. After write only ferror is possible.

[balazske]

Rebased, added handling of UnknownError.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

559–560

Please explain the high-level idea behind this code.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

559–560

The higher level idea is that if a stream function fails we do not create a new state for every type of error (EOF and "other" error). Instead there will be an "unknown error" state. The description for each stream function contains what errors are possible after that function (PossibleErrors).

If it is needed to know the exact error (like here, what should feof return?) we look at the previous function to see what errors are possible after it. If EOF is not possible at all, the feof returns false. If EOF is possible and only one other type of error, we make a state split with EOF error and the other error set. If EOF and two possible other errors are possible there is state split again but the non-EOF state contains UnknownError.

In PossibleErrors the NoError state is possible. This indicates that the function failed (returned an error value) but the stream error flags are not set (can happen at fseek). There are 3 possible error values (EOF, "other" and no error), if after a feof there is UnknownError we know that the remaining 2 error types are possible.

[balazske]

Rebased.

[balazske]

Simplified code.

[Szelethus]

In D75851#1933538, @balazske wrote:

Simplified code.

I can tell! The patch is amazing, the code practically reads itself aloud. I have some inlines but nothing major, the high level idea is great.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

98–105

This feels clunky to me.

Suppose that we're curious about the stream reaching EOF. We know that the last operation on the stream could cause EOF and ERROR. This function then would return FError. That doesn't really feel like the answer to the question "isErrorPossible".

In what contexts do you see calling this function that isn't like this?:

Optional<StreamState::ErrorKindTy> NewError =
    SS->isErrorPossible(ErrorKind);
if (!NewError) {
  // This kind of error is not possible, function returns zero.
  // Error state remains unknown.
  AddTransition(bindInt(0, State, C, CE), StreamState::UnknownError);
} else {
  // Add state with true returned.
  AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind);
  // Add state with false returned and the new remaining error type.
  AddTransition(bindInt(0, State, C, CE), *NewError);
}

Would it make more sense to move the logic of creating state transitions into a function instead?

536–544

It took me a while to realize that this also includes the case where the stat is not in any error :) Could you add a line of comment please?

553–556

I gave this plenty of thought, and I now believe that the state split here is appropriate. We really should ensure that the user checked both feof and ferror.

clang/test/Analysis/stream-error.c

62

That is a bit misleading, because Eof isn't EOF as specified in the standard. How about IsEof?

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

98–105

The name of the function may be misleading. The non-empty Optional means "yes" and an empty means "no". And if yes additional information is returned in the optional value. If the transitions would be included in this function it needs more parameters. This function is for the part of the operation that computes the remaining error ( getRemainingPossibleError is a better name?). Later it will turn out in what way the function is used and if there is code repetition a new function can be introduced, but not now (the needed state transitions may be different).

[balazske]

• Rebase
• Improved comments
• Changed UnknownError to Unknown
• Test changes (removed unneeded test)

[Szelethus]

LGTM! You seem to have a firm grasp on where you want this checker to go, and I like everything about it.

The code needs seme clang-format treatment, and another eye might not hurt, but as far as I'm concerned, I'm super happy how this patch turned out.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

98–105

getRemainingPossibleError sounds great! You convinced me, if we ever need to stuff more things into this function, we will do that then.

[balazske]

Addressed review comments (reformat and rename).

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 
+struct FnDescription;
+
 /// Full state information about a stream pointer.
 struct StreamState {
+  /// The last file operation called in the stream.
+  const FnDescription *LastOperation;
+
   /// State of a stream symbol.
   /// FIXME: We need maybe an "escaped" state later.
   enum KindTy {
     Opened, /// Stream is opened.
     Closed, /// Closed stream (an invalid stream pointer after it was closed).
     OpenFailed /// The last open operation has failed.
   } State;
 
   /// The error state of a stream.
   /// Valid only if the stream is opened.
   /// It is assumed that feof and ferror flags are never true at the same time.
   enum ErrorKindTy {
     /// No error flag is set (or stream is not open).
     NoError,
     /// EOF condition (`feof` is true).
     FEof,
     /// Other generic (non-EOF) error (`ferror` is true).
     FError,
+    /// Unknown error flag is set (or none), the meaning depends on the last
+    /// operation.
+    Unknown
   } ErrorState = NoError;
 
   bool isOpened() const { return State == Opened; }
   bool isClosed() const { return State == Closed; }
   bool isOpenFailed() const { return State == OpenFailed; }
 
   bool isNoError() const {
     assert(State == Opened && "Error undefined for closed stream.");
     return ErrorState == NoError;
   }
   bool isFEof() const {
     assert(State == Opened && "Error undefined for closed stream.");
     return ErrorState == FEof;
   }
   bool isFError() const {
     assert(State == Opened && "Error undefined for closed stream.");
     return ErrorState == FError;
   }
+  bool isUnknown() const {
+    assert(State == Opened && "Error undefined for closed stream.");
+    return ErrorState == Unknown;
+  }
 
   bool operator==(const StreamState &X) const {
     // In not opened state error should always NoError.
-    return State == X.State && ErrorState == X.ErrorState;
+    return LastOperation == X.LastOperation && State == X.State &&
+           ErrorState == X.ErrorState;
   }
 
-  static StreamState getOpened() { return StreamState{Opened}; }
-  static StreamState getClosed() { return StreamState{Closed}; }
-  static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
-  static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
-  static StreamState getOpenedWithFError() {
-    return StreamState{Opened, FError};
+  static StreamState getOpened(const FnDescription *L) {
+    return StreamState{L, Opened};
+  }
+  static StreamState getOpened(const FnDescription *L, ErrorKindTy E) {
+    return StreamState{L, Opened, E};
+  }
+  static StreamState getClosed(const FnDescription *L) {
+    return StreamState{L, Closed};
+  }
+  static StreamState getOpenFailed(const FnDescription *L) {
+    return StreamState{L, OpenFailed};
   }
 
+  /// Return if the specified error kind is possible on the stream in the
+  /// current state.
+  /// This depends on the stored `LastOperation` value.
+  /// If the error is not possible returns empty value.
+  /// If the error is possible returns the remaining possible error type
+  /// (after taking out `ErrorKind`). If a single error is possible it will
+  /// return that value, otherwise unknown error.
+  Optional<ErrorKindTy> getRemainingPossibleError(ErrorKindTy ErrorKind) const;
+
   void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.AddPointer(LastOperation);
     ID.AddInteger(State);
     ID.AddInteger(ErrorState);
   }
 };
 
 class StreamChecker;
-struct FnDescription;
 using FnCheck = std::function<void(const StreamChecker *, const FnDescription *,
                                    const CallEvent &, CheckerContext &)>;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits<ArgNoTy>::max();
 
 struct FnDescription {
   FnCheck PreFn;
   FnCheck EvalFn;
   ArgNoTy StreamArgNo;
+  // What errors are possible after this operation.
+  // Used only if this operation resulted in Unknown state
+  // (otherwise there is a known single error).
+  // Must contain 2 or 3 elements, or zero.
+  llvm::SmallVector<StreamState::ErrorKindTy, 3> PossibleErrors = {};
 };
 
+Optional<StreamState::ErrorKindTy>
+StreamState::getRemainingPossibleError(ErrorKindTy ErrorKind) const {
+  assert(ErrorState == Unknown &&
+         "Function to be used only if error is unknown.");
+  llvm::SmallVector<StreamState::ErrorKindTy, 3> NewPossibleErrors;
+  for (StreamState::ErrorKindTy E : LastOperation->PossibleErrors)
+    if (E != ErrorKind)
+      NewPossibleErrors.push_back(E);
+  if (NewPossibleErrors.size() == LastOperation->PossibleErrors.size())
+    return {};
+  if (NewPossibleErrors.size() == 1)
+    return NewPossibleErrors.front();
+  return Unknown;
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
   assert(Desc && Desc->StreamArgNo != ArgNone &&
          "Try to get a non-existing stream argument.");
   return Call.getArgSVal(Desc->StreamArgNo);
 }
 
 /// Create a conjured symbol return value for a call expression.
 DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
   assert(CE && "Expecting a call expression.");
 
   const LocationContext *LCtx = C.getLocationContext();
   return C.getSValBuilder()
       .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
       .castAs<DefinedSVal>();
 }
 
+ProgramStateRef bindAndAssumeTrue(ProgramStateRef State, CheckerContext &C,
+                                  const CallExpr *CE) {
+  DefinedSVal RetVal = makeRetVal(C, CE);
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  State = State->assume(RetVal, true);
+  assert(State && "Assumption on new value should not fail.");
+  return State;
+}
+
+ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
+                        CheckerContext &C, const CallExpr *CE) {
+  State = State->BindExpr(CE, C.getLocationContext(),
+                          C.getSValBuilder().makeIntVal(Value, false));
+  return State;
+}
+
 class StreamChecker
     : public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
   mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
       BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak;
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
   /// If true, evaluate special testing stream functions.
   bool TestMode = false;
 
 private:
   CallDescriptionMap<FnDescription> FnDescriptions = {
-      {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+      {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}},
       {{"freopen", 3},
-       {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
-      {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+       {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2, {}}},
+      {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}},
       {{"fclose", 1},
-       {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
-      {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
-      {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
-      {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 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}},
+       {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0, {}}},
+      {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}},
+      {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}},
+      {{"fseek", 3},
+       {&StreamChecker::preFseek,
+        &StreamChecker::evalFseek,
+        0,
+        {StreamState::FEof, StreamState::FError, StreamState::NoError}}},
+      {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+      {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+      {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+      {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}},
       {{"clearerr", 1},
-       {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
+       {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0, {}}},
+      // Note: feof can result in Unknown if at the call there is a
+      // PossibleErrors with all 3 error states (including NoError).
+      // Then if feof is false the remaining error could be FError or NoError.
       {{"feof", 1},
        {&StreamChecker::preDefault,
-        &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
+        &StreamChecker::evalFeofFerror<StreamState::FEof>,
+        0,
+        {StreamState::FError, StreamState::NoError}}},
+      // Note: ferror can result in Unknown if at the call there is a
+      // PossibleErrors with all 3 error states (including NoError).
+      // Then if ferror is false the remaining error could be FEof or NoError.
       {{"ferror", 1},
        {&StreamChecker::preDefault,
-        &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
-      {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+        &StreamChecker::evalFeofFerror<StreamState::FError>,
+        0,
+        {StreamState::FEof, StreamState::NoError}}},
+      {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
   };
 
   CallDescriptionMap<FnDescription> FnTestDescriptions = {
       {{"StreamTesterChecker_make_feof_stream", 1},
-       {nullptr,
-        &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}},
+       {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FEof>, 0}},
       {{"StreamTesterChecker_make_ferror_stream", 1},
-       {nullptr,
-        &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>,
-        0}},
+       {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FError>, 0}},
   };
 
   void evalFopen(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
 
   void preFreopen(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
   void evalFreopen(const FnDescription *Desc, const CallEvent &Call,
                    CheckerContext &C) const;
 
   void evalFclose(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
                 CheckerContext &C) const;
+  void evalFseek(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;
 
-  template <bool (StreamState::*IsOfError)() const>
+  template <StreamState::ErrorKindTy ErrorKind>
   void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
                       CheckerContext &C) const;
 
-  template <StreamState (*GetState)()>
+  template <StreamState::ErrorKindTy EK>
   void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
                          CheckerContext &C) const;
 
   /// Check that the stream (in StreamVal) is not NULL.
   /// If it can only be NULL a fatal error is emitted and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
   /// to be non-null.
   ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
-                                      ProgramStateRef State) const;  
+                                      ProgramStateRef State) const;
 
   /// Check that the stream is the opened state.
   /// If the stream is known to be not opened an error is generated
   /// and nullptr returned, otherwise the original state is returned.
   ProgramStateRef ensureStreamOpened(SVal StreamVal, CheckerContext &C,
                                      ProgramStateRef State) const;
 
   /// Check the legality of the 'whence' argument of 'fseek'.
   /// Generate error and return nullptr if it is found to be illegal.
   /// Otherwise returns the state.
   /// (State is not changed here because the "whence" value is already known.)
   ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
-                                           ProgramStateRef State) const;  
+                                           ProgramStateRef State) const;
 
   /// Find the description data of the function called by a call event.
   /// Returns nullptr if no function is recognized.
   const FnDescription *lookupFn(const CallEvent &Call) const {
     // Recognize "global C functions" with only integral or pointer arguments
     // (and matching name) as stream functions.
     if (!Call.isGlobalCFunction())
       return nullptr;
     for (auto P : Call.parameters()) {
       QualType T = P->getType();
       if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
         return nullptr;
     }
 
     return FnDescriptions.lookup(Call);
-  }  
+  }
 };
 
 } // end anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
 inline void assertStreamStateOpened(const StreamState *SS) {
   assert(SS->isOpened() &&
void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
   State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
   // Bifurcate the state into two: one with a valid FILE* pointer, the other
   // with a NULL.
   ProgramStateRef StateNotNull, StateNull;
   std::tie(StateNotNull, StateNull) =
       C.getConstraintManager().assumeDual(State, RetVal);
 
-  StateNotNull = StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened());
-  StateNull = StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed());
+  StateNotNull =
+      StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
+  StateNull =
+      StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
 
   C.addTransition(StateNotNull);
   C.addTransition(StateNull);
 }
 
 void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   // Do not allow NULL as passed stream pointer but allow a closed stream.
void StreamChecker::evalFreopen(const FnDescription *Desc,
   ProgramStateRef StateRetNotNull =
       State->BindExpr(CE, C.getLocationContext(), *StreamVal);
   // Generate state for NULL return value.
   // Stream switches to OpenFailed state.
   ProgramStateRef StateRetNull = State->BindExpr(CE, C.getLocationContext(),
                                                  C.getSValBuilder().makeNull());
 
   StateRetNotNull =
-      StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened());
+      StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
   StateRetNull =
-      StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed());
+      StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
 
   C.addTransition(StateRetNotNull);
   C.addTransition(StateRetNull);
 }
 
 void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
   if (!Sym)
     return;
 
   const StreamState *SS = State->get<StreamMap>(Sym);
   if (!SS)
     return;
 
   assertStreamStateOpened(SS);
 
   // Close the File Descriptor.
   // Regardless if the close fails or not, stream becomes "closed"
   // and can not be used any more.
-  State = State->set<StreamMap>(Sym, StreamState::getClosed());
+  State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
 
   C.addTransition(State);
 }
 
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
                              CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
   State = ensureStreamNonNull(StreamVal, C, State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
   State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
   if (!State)
     return;
 
   C.addTransition(State);
 }
 
+void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
+                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  // Ignore the call if the stream is not tracked.
+  if (!State->get<StreamMap>(StreamSym))
+    return;
+
+  DefinedSVal RetVal = makeRetVal(C, CE);
+
+  // Make expression result.
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+
+  // Bifurcate the state into failed and non-failed.
+  // Return zero on success, nonzero on error.
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateFailed, StateNotFailed) =
+      C.getConstraintManager().assumeDual(State, RetVal);
+
+  // Reset the state to opened with no error.
+  StateNotFailed =
+      StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+  // We get error.
+  StateFailed = StateFailed->set<StreamMap>(
+      StreamSym, StreamState::getOpened(Desc, StreamState::Unknown));
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
 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;
 
   const StreamState *SS = State->get<StreamMap>(StreamSym);
   if (!SS)
     return;
 
   assertStreamStateOpened(SS);
 
-  if (SS->isNoError())
-    return;
-
-  State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+  State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
   C.addTransition(State);
 }
 
-template <bool (StreamState::*IsOfError)() const>
+template <StreamState::ErrorKindTy ErrorKind>
 void StreamChecker::evalFeofFerror(const FnDescription *Desc,
                                    const CallEvent &Call,
                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
   if (!StreamSym)
     return;
 
   const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
 
+  auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State,
+                                             StreamState::ErrorKindTy SSError) {
+    StreamState SSNew = StreamState::getOpened(Desc, SSError);
+    State = State->set<StreamMap>(StreamSym, SSNew);
+    C.addTransition(State);
+  };
+
   const StreamState *SS = State->get<StreamMap>(StreamSym);
-  // Ignore the call if the stream is not tracked.
   if (!SS)
     return;
 
   assertStreamStateOpened(SS);
 
-  if ((SS->*IsOfError)()) {
-    // Function returns nonzero.
-    DefinedSVal RetVal = makeRetVal(C, CE);
-    State = State->BindExpr(CE, C.getLocationContext(), RetVal);
-    State = State->assume(RetVal, true);
-    assert(State && "Assumption on new value should not fail.");
+  if (!SS->isUnknown()) {
+    // The stream is in exactly known state (error or not).
+    // Check if it is in error of kind `ErrorKind`.
+    if (SS->ErrorState == ErrorKind)
+      State = bindAndAssumeTrue(State, C, CE);
+    else
+      State = bindInt(0, State, C, CE);
+    // Error state is not changed in the new state.
+    AddTransition(State, SS->ErrorState);
   } else {
-    // Return zero.
-    State = State->BindExpr(CE, C.getLocationContext(),
-                            C.getSValBuilder().makeIntVal(0, false));
+    // Stream is in unknown state, check if error `ErrorKind` is possible.
+    Optional<StreamState::ErrorKindTy> NewError =
+        SS->getRemainingPossibleError(ErrorKind);
+    if (!NewError) {
+      // This kind of error is not possible, function returns zero.
+      // Error state remains unknown.
+      AddTransition(bindInt(0, State, C, CE), StreamState::Unknown);
+    } else {
+      // Add state with true returned.
+      AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind);
+      // Add state with false returned and the new remaining error type.
+      AddTransition(bindInt(0, State, C, CE), *NewError);
+    }
   }
-  C.addTransition(State);
 }
 
 void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
   State = ensureStreamNonNull(StreamVal, C, State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
 
   C.addTransition(State);
 }
 
-template <StreamState (*GetState)()>
+template <StreamState::ErrorKindTy EK>
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
                                       const CallEvent &Call,
                                       CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
   assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
-  State = State->set<StreamMap>(StreamSym, (*GetState)());
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  assert(SS && "Stream should be tracked by the checker.");
+  State = State->set<StreamMap>(StreamSym,
+                                StreamState::getOpened(SS->LastOperation, EK));
   C.addTransition(State);
 }
 
 ProgramStateRef
 StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
                                    ProgramStateRef State) const {
   auto Stream = StreamVal.getAs<DefinedSVal>();
   if (!Stream)

 void ento::registerStreamTesterChecker(CheckerManager &Mgr) {
   auto *Checker = Mgr.getChecker<StreamChecker>();
   Checker->TestMode = true;
 }
 
 bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) {
   return true;
 }
-

clang/test/Analysis/stream-error.c

void stream_error_ferror() {
   StreamTesterChecker_make_ferror_stream(F);
   clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
   clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
   clearerr(F);
   clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
   clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
   fclose(F);
 }
+
+void error_fseek() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+    int IsFEof = feof(F), IsFError = ferror(F);
+    // Get feof or ferror or no error.
+    clang_analyzer_eval(IsFEof || IsFError);
+    // expected-warning@-1 {{FALSE}}
+    // expected-warning@-2 {{TRUE}}
+    clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
+    // Error flags should not change.
+    if (IsFEof)
+      clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+    else
+      clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+    if (IsFError)
+      clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+    else
+      clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+    // Error flags should not change.
+    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  }
+  fclose(F);
+}