[Analyzer][StreamChecker] Introduction of stream error handling.

Summary:
Store the error flags (EOF or error) of a stream.
Support the functions feof, ferror, clearerr.
Added a test checker for setting the error flags.

Reviewers: Szelethus, NoQ, Charusso, baloghadamsoftware, xazax.hun

Reviewed By: Szelethus

Subscribers: steakhal, ASDenysPetrov, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75682

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -1400,6 +1400,11 @@ def TaintTesterChecker : Checker<"TaintTest">,
   HelpText<"Mark tainted symbols as such.">,
   Documentation<NotDocumented>;
 
+def StreamTesterChecker : Checker<"StreamTester">,
+  HelpText<"Add test functions to StreamChecker for test and debugging purposes.">,
+  Dependencies<[StreamChecker]>,
+  Documentation<NotDocumented>;
+
 def ExprInspectionChecker : Checker<"ExprInspection">,
   HelpText<"Check the analyzer's understanding of expressions">,
   Documentation<NotDocumented>;

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

@@ -19,33 +19,67 @@
 #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 {
 
+/// Full state information about a stream pointer.
 struct StreamState {
-  enum Kind { Opened, Closed, OpenFailed, Escaped } K;
+  /// 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;
 
-  StreamState(Kind k) : K(k) {}
+  /// 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,
+  } ErrorState = NoError;
 
-  bool isOpened() const { return K == Opened; }
-  bool isClosed() const { return K == Closed; }
-  bool isOpenFailed() const { return K == OpenFailed; }
-  //bool isEscaped() const { return K == Escaped; }
+  bool isOpened() const { return State == Opened; }
+  bool isClosed() const { return State == Closed; }
+  bool isOpenFailed() const { return State == OpenFailed; }
 
-  bool operator==(const StreamState &X) const { return K == X.K; }
+  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;
+  }
 
-  static StreamState getOpened() { return StreamState(Opened); }
-  static StreamState getClosed() { return StreamState(Closed); }
-  static StreamState getOpenFailed() { return StreamState(OpenFailed); }
-  static StreamState getEscaped() { return StreamState(Escaped); }
+  bool operator==(const StreamState &X) const {
+    // In not opened state error should always NoError.
+    return 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};
+  }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
-    ID.AddInteger(K);
+    ID.AddInteger(State);
+    ID.AddInteger(ErrorState);
   }
 };
 
@@ -71,6 +105,16 @@ SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
   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>();
+}
+
 class StreamChecker
     : public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
   mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
@@ -81,6 +125,9 @@ public:
   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}},
@@ -96,12 +143,27 @@ private:
       {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
       {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
       {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+      {{"clearerr", 1},
+       {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
+      {{"feof", 1},
+       {&StreamChecker::preDefault,
+        &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
+      {{"ferror", 1},
+       {&StreamChecker::preDefault,
+        &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
       {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
+  CallDescriptionMap<FnDescription> FnTestDescriptions = {
+      {{"StreamTesterChecker_make_feof_stream", 1},
+       {nullptr,
+        &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}},
+      {{"StreamTesterChecker_make_ferror_stream", 1},
+       {nullptr,
+        &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>,
+        0}},
+  };
+
   void evalFopen(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
 
@@ -119,6 +181,17 @@ private:
   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>
+  void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
+                      CheckerContext &C) const;
+
+  template <StreamState (*GetState)()>
+  void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
+                         CheckerContext &C) const;
+
   /// Check that the stream (in StreamVal) is not NULL.
   /// If it can only be NULL a fatal error is emitted and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
@@ -160,6 +233,11 @@ private:
 
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
+inline void assertStreamStateOpened(const StreamState *SS) {
+  assert(SS->isOpened() &&
+         "Previous create of error node for non-opened stream failed?");
+}
+
 void StreamChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
   const FnDescription *Desc = lookupFn(Call);
@@ -171,6 +249,8 @@ void StreamChecker::checkPreCall(const CallEvent &Call,
 
 bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
   const FnDescription *Desc = lookupFn(Call);
+  if (!Desc && TestMode)
+    Desc = FnTestDescriptions.lookup(Call);
   if (!Desc || !Desc->EvalFn)
     return false;
 
@@ -182,15 +262,11 @@ bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
 void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
                               CheckerContext &C) const {
   ProgramStateRef State = C.getState();
-  SValBuilder &SVB = C.getSValBuilder();
-  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
-
-  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
 
-  DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
-                           .castAs<DefinedSVal>();
+  DefinedSVal RetVal = makeRetVal(C, CE);
   SymbolRef RetSym = RetVal.getAsSymbol();
   assert(RetSym && "RetVal must be a symbol here.");
 
@@ -271,6 +347,8 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
   if (!SS)
     return;
 
+  assertStreamStateOpened(SS);
+
   // Close the File Descriptor.
   // Regardless if the close fails or not, stream becomes "closed"
   // and can not be used any more.
@@ -296,6 +374,61 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(State);
 }
 
+void StreamChecker::evalClearerr(const FnDescription *Desc,
+                                 const CallEvent &Call,
+                                 CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(StreamSym);
+  if (!SS)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  if (SS->isNoError())
+    return;
+
+  State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+  C.addTransition(State);
+}
+
+template <bool (StreamState::*IsOfError)() const>
+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;
+
+  assertStreamStateOpened(SS);
+
+  if ((SS->*IsOfError)()) {
+    // Function returns nonzero.
+    DefinedSVal RetVal = makeRetVal(C, CE);
+    State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+    State = State->assume(RetVal, true);
+    assert(State && "Assumption on new value should not fail.");
+  } else {
+    // Return zero.
+    State = State->BindExpr(CE, C.getLocationContext(),
+                            C.getSValBuilder().makeIntVal(0, false));
+  }
+  C.addTransition(State);
+}
+
 void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   ProgramStateRef State = C.getState();
@@ -310,6 +443,17 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(State);
 }
 
+template <StreamState (*GetState)()>
+void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
+                                      const CallEvent &Call,
+                                      CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
+  State = State->set<StreamMap>(StreamSym, (*GetState)());
+  C.addTransition(State);
+}
+
 ProgramStateRef
 StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
                                    ProgramStateRef State) const {
@@ -437,10 +581,20 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   }
 }
 
-void ento::registerStreamChecker(CheckerManager &mgr) {
-  mgr.registerChecker<StreamChecker>();
+void ento::registerStreamChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<StreamChecker>();
 }
 
-bool ento::shouldRegisterStreamChecker(const CheckerManager &mgr) {
+bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) {
   return true;
 }
+
+void ento::registerStreamTesterChecker(CheckerManager &Mgr) {
+  auto *Checker = Mgr.getChecker<StreamChecker>();
+  Checker->TestMode = true;
+}
+
+bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) {
+  return true;
+}
+

clang/test/Analysis/Inputs/system-header-simulator.h

@@ -9,7 +9,16 @@
 #define restrict /*restrict*/
 #endif
 
+typedef __typeof(sizeof(int)) size_t;
+typedef long long __int64_t;
+typedef __int64_t __darwin_off_t;
+typedef __darwin_off_t fpos_t;
+
 typedef struct _FILE FILE;
+#define SEEK_SET 0 /* Seek from beginning of file. */
+#define SEEK_CUR 1 /* Seek from current position. */
+#define SEEK_END 2 /* Seek from end of file. */
+
 extern FILE *stdin;
 extern FILE *stdout;
 extern FILE *stderr;
@@ -24,11 +33,35 @@ int printf(const char *restrict format, ...);
 int fprintf(FILE *restrict, const char *restrict, ...);
 int getchar(void);
 
+void setbuf(FILE *restrict, char *restrict);
+int setvbuf(FILE *restrict, char *restrict, int, size_t);
+
+FILE *funopen(const void *,
+              int (*)(void *, char *, int),
+              int (*)(void *, const char *, int),
+              fpos_t (*)(void *, fpos_t, int),
+              int (*)(void *));
+
+FILE *fopen(const char *path, const char *mode);
+FILE *tmpfile(void);
+FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+int fclose(FILE *fp);
+size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
+int fputc(int ch, FILE *stream);
+int fseek(FILE *__stream, long int __off, int __whence);
+long int ftell(FILE *__stream);
+void rewind(FILE *__stream);
+int fgetpos(FILE *stream, fpos_t *pos);
+int fsetpos(FILE *stream, const fpos_t *pos);
+void clearerr(FILE *stream);
+int feof(FILE *stream);
+int ferror(FILE *stream);
+int fileno(FILE *stream);
+
 // Note, on some platforms errno macro gets replaced with a function call.
 extern int errno;
 
-typedef __typeof(sizeof(int)) size_t;
-
 size_t strlen(const char *);
 
 char *strcpy(char *restrict, const char *restrict);
@@ -39,21 +72,6 @@ typedef unsigned long __darwin_pthread_key_t;
 typedef __darwin_pthread_key_t pthread_key_t;
 int pthread_setspecific(pthread_key_t, const void *);
 
-typedef long long __int64_t;
-typedef __int64_t __darwin_off_t;
-typedef __darwin_off_t fpos_t;
-
-void setbuf(FILE * restrict, char * restrict);
-int setvbuf(FILE * restrict, char * restrict, int, size_t);
-
-FILE *fopen(const char * restrict, const char * restrict);
-int fclose(FILE *);
-FILE *funopen(const void *,
-                 int (*)(void *, char *, int),
-                 int (*)(void *, const char *, int),
-                 fpos_t (*)(void *, fpos_t, int),
-                 int (*)(void *));
-
 int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));
 
 typedef void (*freeCallback) (void*);
@@ -117,5 +135,6 @@ void _Exit(int status) __attribute__ ((__noreturn__));
 #define __DARWIN_NULL 0
 #define NULL __DARWIN_NULL
 #endif
+#define EOF (-1)
 
-#define offsetof(t, d) __builtin_offsetof(t, d)
+#define offsetof(t, d) __builtin_offsetof(t, d)

clang/test/Analysis/stream-error.c

@@ -0,0 +1,54 @@
+// 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 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}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}
+
+void error_freopen() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  F = freopen(0, "w", F);
+  if (!F)
+    return;
+  clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}
+
+void stream_error_feof() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  StreamTesterChecker_make_feof_stream(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{TRUE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  clearerr(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}
+
+void stream_error_ferror() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  StreamTesterChecker_make_ferror_stream(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+  clearerr(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}

clang/test/Analysis/stream.c

@@ -1,26 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
 
-typedef __typeof__(sizeof(int)) size_t;
-typedef __typeof__(sizeof(int)) fpos_t;
-typedef struct _IO_FILE FILE;
-#define SEEK_SET  0  /* Seek from beginning of file.  */
-#define SEEK_CUR  1  /* Seek from current position.  */
-#define SEEK_END  2  /* Seek from end of file.  */
-extern FILE *fopen(const char *path, const char *mode);
-extern FILE *tmpfile(void);
-extern int fclose(FILE *fp);
-extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern int fseek (FILE *__stream, long int __off, int __whence);
-extern long int ftell (FILE *__stream);
-extern void rewind (FILE *__stream);
-extern int fgetpos(FILE *stream, fpos_t *pos);
-extern int fsetpos(FILE *stream, const fpos_t *pos);
-extern void clearerr(FILE *stream);
-extern int feof(FILE *stream);
-extern int ferror(FILE *stream);
-extern int fileno(FILE *stream);
-extern FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+#include "Inputs/system-header-simulator.h"
 
 void check_fread() {
   FILE *fp = tmpfile();