[Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[balazske]

Summary

Functions fread and fwrite are now modeled with the possible
return values and stream error flags on error.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

153–159

If we add methods to FnDescription, we might as well add this as well, but I don't insist.

207–208

C'99 standard §7.19.9.4.3, about the ftell function:

If successful, the ftell function returns the current value of the file position indicator for the stream. On failure, the ftell function returns -1L and stores an implementation-defined positive value in errno.

So we need an evalCall for this.

502

Isn't the state change redundant? We have a preCall to this function and we assert this as well.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

207–208

I mean, this function can only fail if the stream itself is an erroneous or closed state, right? So we can associate the -1 return value with the stream being in that, and the other with the stream being opened.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

207–208

The ftell is part of a later patch that is adding ftell (and probably other functions). Only the "PossibleErrors" was updated in this patch because its meaning was changed (it contains value even if only one error kind is possible, before it was used only if at least two errors are possible).
It is not sure how the file operations work, maybe the ftell needs access to some data that is not available at the moment (and can fail even if the stream was not OK). If the stream is already in failed state the question is: Do ftell fail (then the we can make it return -1) or it does not fail but gives an unusable value (then generate a checker warning and stop the analysis).

502

We need to clarify what file operations are valid if the stream is in failed state or in EOF state. Again the question is if the function will simply fail again or it does not fail but does wrong things, or it will work correctly. Currently the initial stream error state is not taken into account for fread and fwrite, this needs to be fixed. For example for fread: "If an error occurs, the resulting value of the file position indicator for the stream is indeterminate." So at least a next read or write or ftell can not work correct after this (or works but with unusable result).

[balazske]

Finally I had to make the decision to remove the ErrorKindTy enum and use boolean flags instead for every possible error (including no error). This is needed because we do not know always what error is possible if it is "unknown". It could be determined from the last operation but now not any more. The documentation for fread says that if 0 is passed as the size or count argument the function returns zero and does not change the state. So the error state before fread remains active. The new design is to have bool values for every error kind (feof and ferror and a no-error state) so multiple can be active in a state to indicate that more than one error state is possible. If no-error or feof is possible these flags are turned on. If we need to know in such a state if the stream is in EOF a state split is needed to handle both cases (one with EOF and one with no-error). This split must be done in the pre-call handler, for example if we want a warning that the operation is not safe to use in EOF state. (But in this case really no split is needed, only clear the EOF flag and make a warning.)

We can have another approach if we do not set return values of the functions at all, instead save the symbol of the function and determine the returned value later from the constraints actually applied on it. This may save state splits, but only until a condition is encountered that checks for the function's return value.

int rc = fread(buf, size, count, fp);
if (rc  < count) {
  int eof = feof(fp);
}

In this code if the fread has no concrete value set, the if would result in state split to rc < count and rc >= count. If the fread sets the return value the same split is done at fread but not at the if. If there is no such if or only later, this saves some state splits. In the first case the possible error state of the stream may be hard to determine because it depends on the things that were done before the fread call too. (We can save the symbol of fread and count and ask at a later point if the condition fread < count is true to get if fread has failed. But need to know additionally if count or size was zero and if yes, use a previous error state.)

[balazske]

• Stream error is stored in separate boolean flags instead of enum.
• Removed PossibleErrors.
• Added "FilePositionIndeterminate".
• Updated fread, fwrite, fseek, clearerr.
• Added tests.

[Szelethus]

I gave a lot of thought to the high level logic, but I need some time to really internalize whats happening here.

In D78374#1993858, @balazske wrote:

Finally I had to make the decision to remove the ErrorKindTy enum and use boolean flags instead for every possible error (including no error). This is needed because we do not know always what error is possible if it is "unknown". It could be determined from the last operation but now not any more. The documentation for fread says that if 0 is passed as the size or count argument the function returns zero and does not change the state. So the error state before fread remains active. The new design is to have bool values for every error kind (feof and ferror and a no-error state) so multiple can be active in a state to indicate that more than one error state is possible. If no-error or feof is possible these flags are turned on. If we need to know in such a state if the stream is in EOF a state split is needed to handle both cases (one with EOF and one with no-error). This split must be done in the pre-call handler, for example if we want a warning that the operation is not safe to use in EOF state. (But in this case really no split is needed, only clear the EOF flag and make a warning.)

I guess one solution would be to have a history of last operations on the stream, but overall, it makes sense to have a make a shift to calculate the possible errors as we're evaluating the functions. Great idea!

We can have another approach if we do not set return values of the functions at all, instead save the symbol of the function and determine the returned value later from the constraints actually applied on it. This may save state splits, but only until a condition is encountered that checks for the function's return value.

Or any stream modifying operation. If we don't have branches for the return value, that is almost a bug in itself. Also, digging the return value out of a branch condition may be difficult (see ReturnValueChecker). Lets stick with the state splits at the function call for now.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

37

We're not describing the error state of a stream here, but rather possible error states, so the name should reflect that.

207–208

In any case, this should be removed from the patch, because adding it seems to be an orthogonal change to this one.

209–212

What does this mean? I guess not the possible states after an fread call, but rather the possible states after a failed fread call, but then...

224–228

...this doesn't make much sense, feof doesn't cause and error, it merely detects it.

491

Will that be the next patch? :D Eager to see it!

525

castAs doesn't return an Optional.

546–552

This is where I'd really like to see a precise quote from the standard.

C'99 standard, §7.19.8.1.3, the return value of fread:

The fread function returns the number of elements successfully read, which may be
less than nmemb if a read error or end-of-file is encountered. If size or nmemb is zero,
fread returns zero and the contents of the array and the state of the stream remain
unchanged.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

209–212

Name of the parameter may be misleading, for evalFreadFwrite the last argument describes the new state after the operation has failed.

224–228

At evalFeofFerror the last parameter simply indicates if it is the feof or the ferror case.

491

It may be too big change to add it as well here (or cause more difficulties). But it should be a check for ErrorFEof in the ErrorState (and add another bug type). This is not a fatal error.

It may be more difficult to make fread return the feof error again if it starts already with feof state. (Because the state with FEof should be split from the generic error state that can contain other possible errors.)

[balazske]

Small review related fixes.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

491

It may be too big change to add it as well here (or cause more difficulties). But it should be a check for ErrorFEof in the ErrorState (and add another bug type). This is not a fatal error.

Oh, yea, right, we went over this once :) What I really meant, is an EOF related error, like reading a variable with an EOF value in any context that isn't a comparison to the actual EOF (which in many contexts still isn't a fatal error, but is far more an indicator of code smell). But I'm just thinking aloud, please proceed with this project however it is convenient to you!

It may be more difficult to make fread return the feof error again if it starts already with feof state. (Because the state with FEof should be split from the generic error state that can contain other possible errors.)

I gave this plenty of thought, how do you imagine the problem of us not splitting up to 3 states to show up? Suppose we're calling fread on a stream where the error state is either EOF or ERROR, but we don't know which. We could just leave the StreamErrorState as-is, couldn't we?

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

37

I do not think that StreamPossibleErrorState is better, it is anyway a state that can contain any information. It is an error related state even if it defines not one exact error.

153–159

Now this is again the only function that could be member, in the current form the assert can be done but not in the member form.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

491

reading a variable with an EOF value in any context that isn't a comparison to the actual EOF

This could be another checker, or it can be integrated somehow into ErrorReturnChecker (that does something similar already). I mean, remembering if a variable contains EOF that comes from a function that may return EOF (otherwise the program can do nothing with a simple -1 numerical value without getting the warning), then if any other thing is done with it than comparison to EOF (or passing it to other function) it can be an error case.

491

Suppose we're calling fread on a stream where the error state is either EOF or ERROR, but we don't know which. We could just leave the StreamErrorState as-is, couldn't we?

• If fread is called with FEOF flag, it returns 0 and FEOF remains.
• If fread is called with FERROR flag, it may fail (with any error) or not. This is similar as calling fread in no-error state.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

491

• If fread is called with FERROR flag, it may fail (with any error) or not. This is similar as calling fread in no-error state.

Is it ever possible that an fread on a stream in FERROR returns non-zero and changes the streams state? Lets unpack this.

§7.19.8.1.2 states that

The fread function reads, into the array pointed to by ptr, up to nmemb elements whose size is specified by size, from the stream pointed to by stream. For each object, size calls are made to the fgetc function and the results stored, in the order read, in an array of unsigned char exactly overlaying the object. The file position indicator for the stream (if defined) is advanced by the number of characters successfully read. If an error occurs, the resulting value of the file position indicator for the stream is indeterminate. If a partial element is read, its value is indeterminate.

§7.19.7.1.3 talks about the return value of fgetc:

If the end-of-file indicator for the stream is set, or if the stream is at end-of-file, the end-of-file indicator for the stream is set and the fgetc function returns EOF. Otherwise, the fgetc function returns the next character from the input stream pointed to by stream. If a read error occurs, the error indicator for the stream is set and the fgetc function returns EOF.

Now, speaking of fgetc, its true that the standard doesn't specifically talk about what happens if the stream is already in error state, yet it does mention what happens if it is in eof-state, but then again, I guess its strongly implied that if the stream is an error state, a read error will occur right away, and the error state is preserved as specified.

If fread is little more then sequential calls to fgetc, I would believe that a stream is in FERROR, FERROR will be preserved, because fgetc wouldn't be able to read a single character. Additionally, as mentioned in another inline, §7.19.8.1.3 talks about the return value of fread:

The fread function returns the number of elements successfully read, which may be less than nmemb if a read error or end-of-file is encountered. If size or nmemb is zero, fread returns zero and the contents of the array and the state of the stream remain unchanged.

so that should be 0.

To conclude, unless I'm wrong (which is totally possible, I'm just throwing standard cites around wanting to prove how I hope these things work), both types of stream errors would be preserved after a call to fread, and the return value would always be zero.

[balazske]

Pre-statement checks for fread and fwrite filter out the cases when the position is "indeterminate", because in those states it is fatal error to call the function. Otherwise if not only the EOF flag is set initially (at start of fread) in ErrorState the fread should not generate state when other errors are possible (or none) (this contains an execution where initially EOF is set, after the function no EOF is set or FERROR is set). So the EOF initial state must be handled separately, probably split from the rest of the error state.

[balazske]

I am not sure if this FEOF thing is such a big problem. The modeling is not exact but practically this does not cause big problems and still better than before this patch. The problem is, if a fread is made and the stream is already in EOF state the (modeled) fread may still succeed or fail again with other error, according to the current way of modeling it. The calling program must handle still every possible error after fread regardless what error was there before. I am not sure if false positives can appear because this issue.

[balazske]

Added state split before fread to make warning for error state possible.

[Szelethus]

I'm sorry for the late review -- please know that this isn't the first time me taking a look, this is a complex issue. I find navigating your phabricator comments a bit difficult, it would help a lot if you would accompany them with a lot of code examples, and a lot more tests in the actual patch.

Error handling is super annoying with streams. Take a look at this:

#include "stdio.h"

int main() {
  FILE *F = fopen("test.cpp", "r");
  putc('c', F);
  printf("error: %i\n", ferror(F));
  char Buf[1024];
  const int READ_COUNT = 5;
  if (READ_COUNT != fread(Buf, sizeof(char), READ_COUNT, F))
    printf("eof: %i\n", ferror(F));
  else
    printf("%s\n", Buf);
  fclose(F);
}

$ clang++ test.cpp && ./a.out 
error: 1
#incl

Should we allow this? It seems like the error flag was set, but the actual problem is unrelated to the read, so the read still works fine.

I feel like we're constantly changing what we're shooting for, and that implies immediate code changes. Let's take a step back, draw a state machine (no error, feof, ferror, indeterminate state), and just define what is an error, what is a code smell, what is perfectly fine, because we don't seem to be on the clear on this. We need to follow and agree on the C standard in a very literal sense, and we're not there just yet. The amount and placement of the state splits seem to cause a lot of confusion and we're not ready for that discussion just yet.

D70470 is a great example for a patch with a state machine.

In D78374#2023195, @balazske wrote:

Pre-statement checks for fread and fwrite filter out the cases when the position is "indeterminate", because in those states it is fatal error to call the function. Otherwise if not only the EOF flag is set initially (at start of fread) in ErrorState the fread should not generate state when other errors are possible (or none) (this contains an execution where initially EOF is set, after the function no EOF is set or FERROR is set). So the EOF initial state must be handled separately, probably split from the rest of the error state.

I read through this many times but I just don't understand what you mean, could you rephrase this please?

In D78374#2031694, @balazske wrote:

Added state split before fread to make warning for error state possible.

Is there a significant gain behind doing that instead of doing a state split immediately? It feels unnatural to delay the split. It doesn't accurately represent how the code would run. Are we supposed to do this at every precall? What about other checkers that may rely on this one?

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

37

Alright, you convinced me.

107

The standard suggests that this happens on FERROR, not FEOF.

If an error occurs, the resulting value of the file position indicator for the stream is indeterminate. If a partial element is read, its value is indeterminate.

It would be wild if the stream wouldn't set the FEOF flag after reaching the end of the file. Also, I would imagine FEOF being usually implemented with the file position indicator.

$ cat test.cpp`

#include "stdio.h"

int main() {
  FILE *F = fopen("test.cpp", "r");
  char Buf[1024];
  const int READ_COUNT = 99999;
  if (READ_COUNT != fread(Buf, sizeof(char), READ_COUNT, F)) {
    printf("%i\n", feof(F));
  }
}

$ build/bin/clang++ test.cpp && ./a.out 
1

Operations on an EOF and indeterminate stream are very different.

[balazske]

I think this is a expected way of how it works, failure of a stream operation does not necessarily depend on result of a previous operation. So any operation can fail or not, independent of the previous error state (the "ferror" may happen because a temporary disk error and it is worth to retry the read, at least a non-fread function where the "indeterminate file position" problem does not happen). This would make things more simple. Only the EOF is probably exception, in EOF state any read operation fails with EOF. (But I do not know if it is possible that a stream in EOF state becomes non-EOF because new data appears at the end, if other program can write into it or it is some form of "pipe" or special file. If this is true even EOF needs no special handling.)

[balazske]

The intent is to model the fread-fwrite function failure by returning the error value and set the stream into error state. The error state is a composite of ferror and feof. The questions are now, at what case do these functions fail and with what error type?

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

107

I wanted to say in that comment that FilePositionIndeterminate should be ignored if the ErrorState is ErrorFEof. In other words the file is never indeterminate if in EOF state.

[Szelethus]

In D78374#2033826, @balazske wrote:

I think this is a expected way of how it works, failure of a stream operation does not necessarily depend on result of a previous operation. So any operation can fail or not, independent of the previous error state (the "ferror" may happen because a temporary disk error and it is worth to retry the read, at least a non-fread function where the "indeterminate file position" problem does not happen). This would make things more simple. Only the EOF is probably exception, in EOF state any read operation fails with EOF.

Right. But, speaking about generality, would it be reasonably to assume that all (or almost all) operations may also result in an indeterminate file indicator? Because if so, we could, and totally should strip those changes from the fread/fwrite ones and make a new revision.

That is by the way true for a lot of other changes, such as the delayed state split or the new error state struct. I mean not to burden you by micromanaging a lot of small patches, but it makes reviewing far faster for me.

(But I do not know if it is possible that a stream in EOF state becomes non-EOF because new data appears at the end, if other program can write into it or it is some form of "pipe" or special file. If this is true even EOF needs no special handling.)

That is a great question, but I strongly believe that it cannot without freopen. I mean, if I were to create an io library, I would set a pointer to the beginning and the end of the file, and know we hit eof is the current pointer hits the end point. Besides, how could anyone change the FILE * object in a non-analyzable way?

In any case, its a reasonable to assume it won't change.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

937–939

Since this isn't a bug but rather a code smell, a note would be a nice compliment.
"Use clearerr() to clear the error flag"

[balazske]

The problem with small changes is that still we (at least I who writes the code) should know the final goal (and this can be hard if I have multiple not related problems to work on and everything goes forward by little pieces spread out in a long time). (Also the reviewer needs to know why a change is made if it is not obvious because it is part of a bigger to-be-added "logic".) Otherwise something already done can turn out to be wrong later, as is the case already now with these changes. My goal was to add the file handling functions one by one and adjust the existing code to work with the current implemented functions (probably something already added must be changed to adapt to the new conditions).

[balazske]

The indeterminate position is mentioned only at fread and fwrite. I do not know if it is reasonable to make the indeterminate position in other cases. Th indeterminate position is separate from error flags because of "clearerr" that clears the error flag but not the indeterminate position (if this is the right way of how it works), and because even ferror does not imply an indeterminate position always like after a write to a file opened in read mode. Then it is a detail question after what functions to set indeterminate position and before what functions check for it, the support for it is still needed. The current way of its handling seems correct to me, indeterminate position is not allowed before read or write but no problem before fseek (at least with an absolute seek value, this is not checked now).

[balazske]

We can assume that size of a file does not change by external (to the analyzed program) events so the EOF state can not disappear. (Is this true for stdin, or it has never an EOF?)

[balazske]

Adding a state chart may help something but does not tell the full picture because conditions on the transitions are not trivial to indicate and follow. And the state chart is too complex to draw it in ASCII-art format.

[balazske]

Created new revisions for parts of this change and a bit improved: D80009 and others in "Stack".

[baloghadamsoftware]

Is there a significant gain behind doing that instead of doing a state split immediately? It feels unnatural to delay the split. It doesn't accurately represent how the code would run. Are we supposed to do this at every precall? What about other checkers that may rely on this one?

The actual gain is that we somewhat reduce the exponential growth of the exploded graph. With such a Schrödinger's cat pattern we delay it to the point where it is actually checked, it checked at all.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

39

bool NoError:1 = true etc. Why not use bit fields for a struct of boolans?

[Szelethus]

We can close this now, right?

[balazske]

Yes, closing it. (Will change StreamErrorState probably to bitfield later.)

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include <functional>
 
 using namespace clang;
 using namespace ento;
+using namespace std::placeholders;
 
 namespace {
 
 struct FnDescription;
 
+/// State of the stream related error flags.
+/// It indicates possibility of different error states.
+/// In other sense, every set flag means a separate execution path.
+/// At least one of the bool flags must be set to have a valid StreamErrorState.
+/// Note: A stream has either FEOF or FERROR set but not both at the same time.
+struct StreamErrorState {
+  /// There is an execution path with none of the error flags set.
+  bool NoError = true;
+  /// There is an execution path with EOF indicator set.
+  bool FEof = false;
+  /// There is an execution path with error indicator set.
+  bool FError = false;
+  // bool FErrorIndeterminate = false;
+  // bool NoErrorIndeterminate = false;
+
+  bool isNoError() const { return NoError && !FEof && !FError; }
+  bool isFEof() const { return !NoError && FEof && !FError; }
+  bool isFError() const { return !NoError && !FEof && FError; }
+
+  bool operator==(const StreamErrorState &ES) const {
+    return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
+  }
+
+  StreamErrorState operator|(const StreamErrorState &E) const {
+    return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
+  }
+
+  StreamErrorState operator&(const StreamErrorState &E) const {
+    return {NoError && E.NoError, FEof && E.FEof, FError && E.FError};
+  }
+
+  StreamErrorState operator~() const { return {!NoError, !FEof, !FError}; }
+
+  /// Returns if the StreamErrorState is a valid object.
+  operator bool() const { return NoError || FEof || FError; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.AddBoolean(NoError);
+    ID.AddBoolean(FEof);
+    ID.AddBoolean(FError);
+  }
+};
+
+const StreamErrorState ErrorNone{true, false, false};
+const StreamErrorState ErrorFEof{false, true, false};
+const StreamErrorState ErrorFError{false, false, true};
+
 /// Full state information about a stream pointer.
+/// ErrorState and FilePositionIndeterminate are only used if the stream is in
+/// Opened state. Otherwise these should be set to the default value.
 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;
+  /// Used to store a bundle of execution paths if more than one error kind is
+  /// set here.
+  StreamErrorState ErrorState;
+
+  /// Indicate if the file has an "indeterminate file position indicator".
+  /// This can be set at a failing read or write or seek operation.
+  /// If it is set no more read or write is allowed until the position is set
+  /// in other way.
+  /// This value is not dependent on the stream error flags:
+  /// The error flag may be cleared with `clearerr` but the file position
+  /// remains still indeterminate.
+  /// This value applies to all execution paths in ErrorState except the FEOF
+  /// case. In more detail, an EOF+indeterminate state is the same as EOF state.
+  bool FilePositionIndeterminate = false;
 
   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 LastOperation == X.LastOperation && State == X.State &&
-           ErrorState == X.ErrorState;
+           ErrorState == X.ErrorState &&
+           FilePositionIndeterminate == X.FilePositionIndeterminate;
   }
 
-  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 getOpened(const FnDescription *L,
+                               const StreamErrorState &ES = ErrorNone,
+                               bool FPI = false) {
+    return StreamState{L, Opened, ES, FPI};
   }
   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 {
+    ErrorState.Profile(ID);
     ID.AddPointer(LastOperation);
     ID.AddInteger(State);
-    ID.AddInteger(ErrorState);
+    ID.AddBoolean(FilePositionIndeterminate);
   }
 };
 
 class StreamChecker;
 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())
ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
   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;
+      BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak,
+      BT_InvalidStreamState, BT_StreamInErrorState;
 
 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,
-        &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, {}}},
+       {&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}},
+      // Note: fgetpos sets errno only.
+      {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+      // Note: fsetpos sets errno only.
+      {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
       {{"clearerr", 1},
-       {&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.
+       {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
       {{"feof", 1},
        {&StreamChecker::preDefault,
-        &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.
+        std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
+        0}},
       {{"ferror", 1},
        {&StreamChecker::preDefault,
-        &StreamChecker::evalFeofFerror<StreamState::FError>,
-        0,
-        {StreamState::FEof, StreamState::NoError}}},
-      {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+        std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
+        0}},
+      {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
   CallDescriptionMap<FnDescription> FnTestDescriptions = {
       {{"StreamTesterChecker_make_feof_stream", 1},
-       {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FEof>, 0}},
+       {nullptr,
+        std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof),
+        0}},
       {{"StreamTesterChecker_make_ferror_stream", 1},
-       {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FError>, 0}},
+       {nullptr,
+        std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
+                  ErrorFError),
+        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 preFread(const FnDescription *Desc, const CallEvent &Call,
+                CheckerContext &C) const;
+
+  void preFwrite(const FnDescription *Desc, const CallEvent &Call,
+                 CheckerContext &C) const;
+
+  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 preDefault(const FnDescription *Desc, const CallEvent &Call,
+  void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
                     CheckerContext &C) const;
 
-  void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
-                    CheckerContext &C) const;
+  void preDefault(const FnDescription *Desc, const CallEvent &Call,
 
-  template <StreamState::ErrorKindTy ErrorKind>
   void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
-                      CheckerContext &C) const;
+                      CheckerContext &C,
+                      const StreamErrorState &ErrorKind) const;
 
-  template <StreamState::ErrorKindTy EK>
   void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
-                         CheckerContext &C) const;
+                         CheckerContext &C,
+                         const StreamErrorState &ErrorKind) 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;
 
   /// 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 that the stream has not an invalid ("indeterminate") file position,
+  /// generate warning for it.
+  /// (EOF is not an invalid position.)
+  /// The returned state can be nullptr if a fatal error was generated.
+  /// It can return non-null state if the stream has not an invalid position or
+  /// there is execution path with non-invalid position.
+  ProgramStateRef
+  ensureNoFilePositionIndeterminate(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;
 
+  /// Create two states where the original error state of the stream is
+  /// differentiated. First member contains error state with bits from
+  /// 'SplitError' and original error state. Second member contains error state
+  /// with bits from complement of 'SplitError' and original error state. Both
+  /// can be null.
+  std::pair<ProgramStateRef, ProgramStateRef>
+  splitErrorState(ProgramStateRef State, SymbolRef StreamSym,
+                  StreamErrorState SplitError) const;
+
+  /// Report a 'BT_StreamInErrorState' type non-fatal error.
+  /// Returns the new error node, or null.
+  ExplodedNode *reportStreamInErrorState(CheckerContext &C,
+                                         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()) {
void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
   // 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(Desc));
 
   C.addTransition(State);
 }
 
+void StreamChecker::preFread(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 = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+  if (!State)
+    return;
+
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (Sym && State->get<StreamMap>(Sym)) {
+    ProgramStateRef StateError, StateNoError;
+    std::tie(StateError, StateNoError) =
+        splitErrorState(State, Sym, ErrorFEof | ErrorFError);
+    if (StateError) {
+      if (!reportStreamInErrorState(C, StateError))
+        C.addTransition(StateError);
+    }
+    if (StateNoError)
+      C.addTransition(StateNoError);
+  } else {
+    C.addTransition(State);
+  }
+}
+
+void StreamChecker::preFwrite(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 = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+  if (!State)
+    return;
+
+  C.addTransition(State);
+}
+
+void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
+                                    const CallEvent &Call, CheckerContext &C,
+                                    bool IsFread) 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;
+
+  Optional<NonLoc> SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
+  if (!SizeVal)
+    return;
+  Optional<NonLoc> CountVal = Call.getArgSVal(2).getAs<NonLoc>();
+  if (!CountVal)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  if (!SS)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  // C'99 standard, §7.19.8.1.3, the return value of fread:
+  // The fread function returns the number of elements successfully read, which
+  // may be less than nmemb if a read error or end-of-file is encountered. If
+  // size or nmemb is zero, fread returns zero and the contents of the array and
+  // the state of the stream remain unchanged.
+
+  if (State->isNull(*SizeVal).isConstrainedTrue() ||
+      State->isNull(*CountVal).isConstrainedTrue()) {
+    // This is the "size or nmemb is zero" case.
+    // Just return 0, do nothing more (not clear the error flags).
+    State = bindInt(0, State, C, CE);
+    C.addTransition(State);
+    return;
+  }
+
+  // Generate a transition for the success state.
+  // The fread call fails if there is already error, in this case make no
+  // success transition.
+  if (!IsFread || SS->ErrorState & ErrorNone) {
+    ProgramStateRef StateNotFailed =
+        State->BindExpr(CE, C.getLocationContext(), *CountVal);
+    if (StateNotFailed) {
+      StateNotFailed = StateNotFailed->set<StreamMap>(
+          StreamSym, StreamState::getOpened(Desc));
+      C.addTransition(StateNotFailed);
+    }
+  }
+
+  // Add transition for the failed state.
+  Optional<NonLoc> RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+  assert(RetVal && "Value should be NonLoc.");
+  ProgramStateRef StateFailed =
+      State->BindExpr(CE, C.getLocationContext(), *RetVal);
+  if (!StateFailed)
+    return;
+  auto Cond = C.getSValBuilder()
+                  .evalBinOpNN(State, BO_LT, *RetVal, *CountVal,
+                               C.getASTContext().IntTy)
+                  .getAs<DefinedOrUnknownSVal>();
+  if (!Cond)
+    return;
+  StateFailed = StateFailed->assume(*Cond, true);
+  if (!StateFailed)
+    return;
+
+  StreamErrorState NewES;
+  if (IsFread)
+    NewES = (SS->ErrorState & ErrorNone) ? (ErrorFEof | ErrorFError)
+                                         : SS->ErrorState;
+  else
+    NewES = ErrorFError;
+  StreamState NewState = StreamState::getOpened(Desc, NewES, true);
+  // If an error occurs, the resulting value of the file position indicator for
+  // the stream is indeterminate. This flag is set now for both the EOF and
+  // FERROR cases. But for EOF it is to be ignored.
+  StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
+  C.addTransition(StateFailed);
+}
+
 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);
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   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));
+  // ErrorNone is there because fseek can fail without setting any error flags.
+  StreamState NewState =
+      StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true);
+  StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
 
   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);
 
-  State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+  // The FilePositionIndeterminate value does not change!
+  State = State->set<StreamMap>(
+      StreamSym,
+      StreamState::getOpened(Desc, ErrorNone, SS->FilePositionIndeterminate));
   C.addTransition(State);
 }
 
-template <StreamState::ErrorKindTy ErrorKind>
 void StreamChecker::evalFeofFerror(const FnDescription *Desc,
-                                   const CallEvent &Call,
-                                   CheckerContext &C) const {
+                                   const CallEvent &Call, CheckerContext &C,
+                                   const StreamErrorState &ErrorKind) 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);
   if (!SS)
     return;
 
   assertStreamStateOpened(SS);
 
-  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 {
-    // 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);
-    }
+  if (SS->ErrorState & ErrorKind) {
+    // Found an execution path with ErrorKind, split state for this and return
+    // "true".
+    ProgramStateRef TrueState = bindAndAssumeTrue(State, C, CE);
+    C.addTransition(TrueState->set<StreamMap>(
+        StreamSym, StreamState::getOpened(Desc, ErrorKind,
+                                          SS->FilePositionIndeterminate)));
+  }
+  if (StreamErrorState NewES = SS->ErrorState & (~ErrorKind)) {
+    // Found execution path(s) with ErrorKind not set, split state for this and
+    // return "false".
+    ProgramStateRef FalseState = bindInt(0, State, C, CE);
+    C.addTransition(FalseState->set<StreamMap>(
+        StreamSym,
+        StreamState::getOpened(Desc, NewES, SS->FilePositionIndeterminate)));
   }
 }
 
 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::ErrorKindTy EK>
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
-                                      const CallEvent &Call,
-                                      CheckerContext &C) const {
+                                      const CallEvent &Call, CheckerContext &C,
+                                      const StreamErrorState &ErrorKind) const {
   ProgramStateRef State = C.getState();
   SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
   assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
   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));
+  State = State->set<StreamMap>(
+      StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind));
   C.addTransition(State);
 }
 
 ProgramStateRef
 StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
                                    ProgramStateRef State) const {
   auto Stream = StreamVal.getAs<DefinedSVal>();
   if (!Stream)
if (N) {
       return nullptr;
     }
     return State;
   }
 
   return State;
 }
 
+ProgramStateRef StreamChecker::ensureNoFilePositionIndeterminate(
+    SVal StreamVal, CheckerContext &C, ProgramStateRef State) const {
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (!Sym)
+    return State;
+
+  const StreamState *SS = State->get<StreamMap>(Sym);
+  if (!SS)
+    return State;
+
+  assert(SS->isOpened() && "First ensure that stream is opened.");
+
+  if (SS->FilePositionIndeterminate) {
+    if (!BT_InvalidStreamState)
+      BT_InvalidStreamState.reset(
+          new BuiltinBug(this, "Invalid stream state",
+                         "Stream might be in indeterminate state after "
+                         "a failed operation. "
+                         "Can cause undefined behaviour."));
+
+    // Clear FilePositionIndeterminate from state.
+    StreamState NewState =
+        StreamState::getOpened(SS->LastOperation, SS->ErrorState, false);
+
+    // If only FEof is possible, report no warning (indeterminate position in
+    // FEof is ignored).
+    if (SS->ErrorState.isFEof())
+      return State->set<StreamMap>(Sym, NewState);
+
+    // If FEof is not possible, warn for indeterminate position and stop
+    // analysis.
+    if (!(SS->ErrorState & ErrorFEof)) {
+      ExplodedNode *N = C.generateErrorNode(State);
+      if (N) {
+        C.emitReport(std::make_unique<PathSensitiveBugReport>(
+            *BT_InvalidStreamState, BT_InvalidStreamState->getDescription(),
+            N));
+        return nullptr;
+      }
+      return State; // FIXME: nullptr?
+    }
+
+    // FEof and other states are possible.
+    // The path with FEof is the one that can continue.
+    // For this reason a non-fatal error is generated to continue the analysis
+    // with only FEof state set.
+    ExplodedNode *N = C.generateNonFatalErrorNode(State);
+    if (N) {
+      C.emitReport(std::make_unique<PathSensitiveBugReport>(
+          *BT_InvalidStreamState, BT_InvalidStreamState->getDescription(), N));
+    }
+    NewState.ErrorState = ErrorFEof;
+    return State->set<StreamMap>(Sym, NewState);
+  }
+
+  return State;
+}
+
 ProgramStateRef
 StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
                                         ProgramStateRef State) const {
   Optional<nonloc::ConcreteInt> CI = WhenceVal.getAs<nonloc::ConcreteInt>();
   if (!CI)
     return State;
 
   int64_t X = CI->getValue().getSExtValue();
if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
         *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
     return nullptr;
   }
 
   return State;
 }
 
+std::pair<ProgramStateRef, ProgramStateRef>
+StreamChecker::splitErrorState(ProgramStateRef State, SymbolRef StreamSym,
+                               StreamErrorState SplitError) const {
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  assert(SS && "Function should be called with stream that has state.");
+
+  ProgramStateRef State1, State2;
+  if (StreamErrorState Error1 = SS->ErrorState & SplitError) {
+    State1 = State->set<StreamMap>(
+        StreamSym, StreamState::getOpened(SS->LastOperation, Error1,
+                                          SS->FilePositionIndeterminate));
+  }
+  if (StreamErrorState Error2 = SS->ErrorState & ~SplitError) {
+    State2 = State->set<StreamMap>(
+        StreamSym, StreamState::getOpened(SS->LastOperation, Error2,
+                                          SS->FilePositionIndeterminate));
+  }
+  return std::make_pair(State1, State2);
+}
+
+ExplodedNode *
+StreamChecker::reportStreamInErrorState(CheckerContext &C,
+                                        ProgramStateRef State) const {
+  ExplodedNode *N = C.generateNonFatalErrorNode(State);
+  if (!N)
+    return nullptr;
+
+  if (!BT_StreamInErrorState)
+    BT_StreamInErrorState.reset(
+        new BuiltinBug(this, "Stream is in error state",
+                       "Function called when stream has already error state "
+                       "flag (EOF or error) set."));
+  C.emitReport(std::make_unique<PathSensitiveBugReport>(
+      *BT_StreamInErrorState, BT_StreamInErrorState->getDescription(), N));
+  return N;
+}
+
 void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // TODO: Clean up the state.
   const StreamMapTy &Map = State->get<StreamMap>();
   for (const auto &I : Map) {
     SymbolRef Sym = I.first;

clang/test/Analysis/stream-error.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-checker=debug.StreamTester,debug.ExprInspection -analyzer-store region -verify %s
 
 #include "Inputs/system-header-simulator.h"
 
 void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
 void StreamTesterChecker_make_feof_stream(FILE *);
 void StreamTesterChecker_make_ferror_stream(FILE *);
 
 void error_fopen() {
   FILE *F = fopen("file", "r");
   if (!F)
     return;
   clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
void stream_error_ferror() {
   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_fread() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int Ret = fread(0, 1, 10, F);
+  if (Ret == 10) {
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+    if (feof(F)) {
+      clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+      fread(0, 1, 10, F);             // expected-warning {{Function called when stream has already error state}}
+    }
+    if (ferror(F)) {
+      clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+      fread(0, 1, 10, F);             // expected-warning {{Stream might be in indeterminate state}}
+    }
+  }
+  fclose(F);
+  Ret = fread(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void error_fwrite() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int Ret = fwrite(0, 1, 10, F);
+  if (Ret == 10) {
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+    fwrite(0, 1, 10, F);            // expected-warning {{Stream might be in indeterminate state}}
+  }
+  fclose(F);
+  Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void freadwrite_zerosize(FILE *F) {
+  fwrite(0, 1, 0, F);
+  fwrite(0, 0, 1, F);
+  fread(0, 1, 0, F);
+  fread(0, 0, 1, F);
+}
+
+void freadwrite_zerosize_errorstate(FILE *F) {
+  fwrite(0, 1, 0, F);
+  fwrite(0, 0, 1, F);
+  fread(0, 1, 0, F); // expected-warning {{Function called when stream has already error state}}
+  fread(0, 0, 1, F); // expected-warning {{Function called when stream has already error state}}
+}
+
+void error_fread_fwrite_zerosize() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+
+  freadwrite_zerosize(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+
+  StreamTesterChecker_make_ferror_stream(F);
+  freadwrite_zerosize_errorstate(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+
+  StreamTesterChecker_make_feof_stream(F);
+  freadwrite_zerosize_errorstate(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{TRUE}}
+  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.
if (rc) {
     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_indeterminate() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+    if (feof(F)) {
+      fwrite(0, 1, 10, F); // no warning
+    } else if (ferror(F)) {
+      fwrite(0, 1, 10, F); // expected-warning {{Stream might be in indeterminate state}}
+    } else {
+      fwrite(0, 1, 10, F); // expected-warning {{Stream might be in indeterminate state}}
+    }
+  }
+  fclose(F);
+}
+
+void error_clearerr_indeterminate() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+    if (!feof(F)) {
+      clearerr(F);
+      fwrite(0, 1, 10, F); // expected-warning {{Stream might be in indeterminate state}}
+    }
+  }
+  fclose(F);
+}