[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

451–453

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

451–453

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

451–453

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

451–453

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

451–453

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

536–537

Please explain the high-level idea behind this code.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

536–537

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

106–113

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?

492–525

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?

534–537

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

48

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

106–113

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

106–113

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

 using namespace std::placeholders;
 
 #define ASSERT_STREAMSTATE_OPENED                                              \
   assert(SS->isOpened() &&                                                     \
          "Create of error node failed (only way to get this state)?");
 
 namespace {
 
+struct FnDescription;
+
 /// Full state information about a stream pointer.
 struct StreamState {
+  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.
   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, the meaning depends on the last operation.
+    UnknownError
   } 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 isUnknownError() const {
+    assert(State == Opened && "Error undefined for closed stream.");
+    return ErrorState == UnknownError;
+  }
 
   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 getClosed(const FnDescription *L) {
+    return StreamState{L, Closed};
+  }
+  static StreamState getOpenFailed(const FnDescription *L) {
+    return StreamState{L, OpenFailed};
+  }
+  static StreamState getOpened(const FnDescription *L, ErrorKindTy E) {
+    return StreamState{L, Opened, E};
+  }
+  static StreamState getOpenedWithUnknownError(const FnDescription *L) {
+    return StreamState{L, Opened, UnknownError};
   }
 
   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 UnknownError
+  // (otherwise there is a known single error).
+  // Must contain 2 or 3 elements, or zero.
+  llvm::SmallVector<StreamState::ErrorKindTy, 3> PossibleErrors = {};
 };
 
 /// 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;
 
 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 UnknownError 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 UnknownError 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, {}}},
   };
 
   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;
 
   /// 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,
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;
 
   ASSERT_STREAMSTATE_OPENED;
 
   // 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::getOpenedWithUnknownError(Desc));
+
+  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;
 
   ASSERT_STREAMSTATE_OPENED;
 
   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;
 
   const StreamState *SS = State->get<StreamMap>(StreamSym);
   // Ignore the call if the stream is not tracked.
   if (!SS)
     return;
 
   ASSERT_STREAMSTATE_OPENED;
 
-  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->isUnknownError()) {
+    llvm::SmallVector<StreamState::ErrorKindTy, 3> NewPossibleErrors;
+    for (StreamState::ErrorKindTy E : SS->LastOperation->PossibleErrors)
+      if (E != ErrorKind)
+        NewPossibleErrors.push_back(E);
+    if (NewPossibleErrors.size() == SS->LastOperation->PossibleErrors.size()) {
+      // This kind of error is not possible, function returns zero.
+      // Error state remains unknown (we know only that it is not ErrorKind).
+      State = bindInt(0, State, C, CE);
+      State = State->set<StreamMap>(
+          StreamSym, StreamState::getOpenedWithUnknownError(Desc));
+      C.addTransition(State);
     } else {
-    // Return zero.
-    State = State->BindExpr(CE, C.getLocationContext(),
-                            C.getSValBuilder().makeIntVal(0, false));
+      // The previously unknown error could be ErrorKind.
+      ProgramStateRef StateTrue = bindAndAssumeTrue(State, C, CE);
+      ProgramStateRef StateFalse = bindInt(0, State, C, CE);
+      // If return true, set error state to ErrorKind.
+      StateTrue = StateTrue->set<StreamMap>(
+          StreamSym, StreamState::getOpened(Desc, ErrorKind));
+      // Return false
+      if (NewPossibleErrors.size() == 1)
+        // One other error is possible, new error kind is fixed to that.
+        StateFalse = StateFalse->set<StreamMap>(
+            StreamSym, StreamState::getOpened(Desc, NewPossibleErrors.front()));
+      else
+        // Multiple other "errors" possible, new error kind is unknown.
+        // In this case we known later from the last operation's (will be feof
+        // or ferror) PossibleErrors list what errors are still possible. This
+        // should be the same as the content of PossibleErrors at this point.
+        StateFalse = StateFalse->set<StreamMap>(
+            StreamSym, StreamState::getOpened(Desc, StreamState::UnknownError));
+      C.addTransition(StateTrue);
+      C.addTransition(StateFalse);
     }
+  } else {
+    // We know exactly what the error is.
+    // Make the return value accordingly to the error.
+    if (SS->ErrorState == ErrorKind)
+      State = bindAndAssumeTrue(State, C, CE);
+    else
+      State = bindInt(0, State, C, CE);
+    State = State->set<StreamMap>(StreamSym,
+                                  StreamState::getOpened(Desc, SS->ErrorState));
     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;

clang/test/Analysis/stream-error.c

void error_clearerr() {
   if (ch == EOF) {
     // FIXME: fputc not handled by checker yet, should expect TRUE
     clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
     clearerr(F);
     clang_analyzer_eval(feof(F) || 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 Eof = feof(F), Error = ferror(F);
+    // Get feof or ferror or no error.
+    clang_analyzer_eval(Eof || Error); // expected-warning {{FALSE}} \
+                                       // expected-warning {{TRUE}}
+    clang_analyzer_eval(Eof && Error); // expected-warning {{FALSE}}
+    // Error flags should not change.
+    if (Eof)
+      clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+    else
+      clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+    if (Error)
+      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);
+}
+
+void error_fseek_clearerr() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc && feof(F)) {
+    clearerr(F);
+    clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+  } else if (rc && ferror(F)) {
+    clearerr(F);
+    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  }
+  fclose(F);
+}