balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some of the stream handling functions are modeled in StreamChecker.
These are now added to StdLibraryFunctionsChecker where additional
checks are performed on these functions. This includes setting of
'errno' related state. In this way the errno state is set for these
functions (almost) completely in StdLibraryFunctionsChecker.
The two checkers work together and 'apiModeling.StdCLibraryFunctions'
and its 'ModelPOSIX=true' option should be now a dependency of
checker 'alpha.unix.Stream'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135247

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-errno.c
  clang/test/Analysis/stream-noopen.c

Index: clang/test/Analysis/stream-noopen.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-noopen.c
@@ -0,0 +1,97 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+// enable only StdCLibraryFunctions checker
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_var.h"
+
+void clang_analyzer_eval(int);
+
+const char *WBuf = "123456789";
+char RBuf[10];
+
+void test_freopen(FILE *F) {
+  F = freopen("xxx", "w", F);
+  if (F) {
+    if (errno) {} // expected-warning{{undefined}}
+  } else {
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+}
+
+void test_fread(FILE *F) {
+  size_t Ret = fread(RBuf, 1, 10, F);
+  if (Ret == 10) {
+    if (errno) {} // expected-warning{{undefined}}
+  } else {
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fwrite(FILE *F) {
+  size_t Ret = fwrite(WBuf, 1, 10, F);
+  if (Ret == 10) {
+    if (errno) {} // expected-warning{{undefined}}
+  } else {
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fclose(FILE *F) {
+  int Ret = fclose(F);
+  if (Ret == 0) {
+    if (errno) {} // expected-warning{{undefined}}
+  } else {
+    clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fseek(FILE *F) {
+  int Ret = fseek(F, SEEK_SET, 1);
+  if (Ret == 0) {
+    if (errno) {} // expected-warning{{undefined}}
+  } else {
+    clang_analyzer_eval(Ret == -1); // expected-warning {{TRUE}}
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void freadwrite_zerosize(FILE *F) {
+  fwrite(WBuf, 1, 0, F);
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+  if (errno) {} // no-warning
+  fwrite(WBuf, 0, 1, F);
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+  if (errno) {} // no-warning
+  fread(RBuf, 1, 0, F);
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+  if (errno) {} // no-warning
+  fread(RBuf, 0, 1, F);
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+  if (errno) {} // no-warning
+}
Index: clang/test/Analysis/stream-errno.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-errno.c
@@ -0,0 +1,138 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+extern void clang_analyzer_eval(int);
+extern void clang_analyzer_dump(int);
+extern void clang_analyzer_printState();
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  if (!F) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  if (!F) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+
+void check_freopen(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  F = freopen("xxx", "w", F);
+  if (!F) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fclose(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int Ret = fclose(F);
+  if (Ret == EOF) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+  char Buf[10];
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  fread(Buf, 0, 1, F);
+  if (errno) {} // no-warning
+  fread(Buf, 1, 0, F);
+  if (errno) {} // no-warning
+
+  int R = fread(Buf, 1, 10, F);
+  if (R < 10) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    fclose(F);
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fwrite(void) {
+  char Buf[] = "0123456789";
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  fwrite(Buf, 0, 1, F);
+  if (errno) {} // no-warning
+  fwrite(Buf, 1, 0, F);
+  if (errno) {} // no-warning
+
+  int R = fwrite(Buf, 1, 10, F);
+  if (R < 10) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    fclose(F);
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_fseek(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int S = fseek(F, 11, SEEK_SET);
+  if (S != 0) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    fclose(F);
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
+
+void check_no_errno_change(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 1;
+  clearerr(F);
+  if (errno) {} // no-warning
+  feof(F);
+  if (errno) {} // no-warning
+  ferror(F);
+  if (errno) {} // no-warning
+  clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}}
+  fclose(F);
+}
+
+void check_fileno(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int N = fileno(F);
+  if (N == -1) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {} // no-warning
+    fclose(F);
+    return;
+  }
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
Index: clang/test/Analysis/stream-errno-note.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-errno-note.c
@@ -0,0 +1,112 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+void check_fopen(void) {
+  FILE *F = fopen("xxx", "r");
+  // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  fclose(F);
+}
+
+void check_tmpfile(void) {
+  FILE *F = tmpfile();
+  // expected-note@-1{{Assuming that function 'tmpfile' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  fclose(F);
+}
+
+void check_freopen(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  F = freopen("xxx", "w", F);
+  // expected-note@-1{{Assuming that function 'freopen' is successful}}
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  fclose(F);
+}
+
+void check_fclose(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  (void)fclose(F);
+  // expected-note@-1{{Assuming that function 'fclose' is successful}}
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+  char Buf[10];
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  (void)fread(Buf, 1, 10, F);
+  // expected-note@-1{{Assuming that function 'fread' is successful}}
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  (void)fclose(F);
+}
+
+void check_fwrite(void) {
+  char Buf[] = "0123456789";
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  int R = fwrite(Buf, 1, 10, F);
+  // expected-note@-1{{Assuming that function 'fwrite' is successful}}
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  (void)fclose(F);
+}
+
+void check_fseek(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  (void)fseek(F, 11, SEEK_SET);
+  // expected-note@-1{{Assuming that function 'fseek' is successful}}
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  (void)fclose(F);
+}
+
+void check_fileno(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  fileno(F);
+  // expected-note@-1{{Assuming that function 'fileno' is successful}}
+  if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  // expected-note@-1{{An undefined value may be read from 'errno'}}
+  (void)fclose(F);
+}
Index: clang/test/Analysis/Inputs/system-header-simulator.h
===================================================================
--- clang/test/Analysis/Inputs/system-header-simulator.h
+++ clang/test/Analysis/Inputs/system-header-simulator.h
@@ -42,9 +42,9 @@
               fpos_t (*)(void *, fpos_t, int),
               int (*)(void *));
 
-FILE *fopen(const char *path, const char *mode);
+FILE *fopen(const char *restrict path, const char *restrict mode);
 FILE *tmpfile(void);
-FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+FILE *freopen(const char *restrict pathname, const char *restrict mode, FILE *restrict stream);
 int fclose(FILE *fp);
 size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
@@ -52,7 +52,7 @@
 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 fgetpos(FILE *restrict stream, fpos_t *restrict pos);
 int fsetpos(FILE *stream, const fpos_t *pos);
 void clearerr(FILE *stream);
 int feof(FILE *stream);
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "ErrnoModeling.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -17,6 +18,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -85,10 +87,10 @@
 /// Full state information about a stream pointer.
 struct StreamState {
   /// The last file operation called in the stream.
+  /// Can be nullptr.
   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).
@@ -202,7 +204,7 @@
 ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
                         CheckerContext &C, const CallExpr *CE) {
   State = State->BindExpr(CE, C.getLocationContext(),
-                          C.getSValBuilder().makeIntVal(Value, false));
+                          C.getSValBuilder().makeIntVal(Value, CE->getType()));
   return State;
 }
 
@@ -249,10 +251,6 @@
        {&StreamChecker::preFwrite,
         std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
       {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
-      {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
       {{"clearerr", 1},
        {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
       {{"feof", 1},
@@ -278,6 +276,8 @@
         0}},
   };
 
+  mutable Optional<int> EofVal;
+
   void evalFopen(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
 
@@ -411,6 +411,17 @@
     });
   }
 
+  void initEof(CheckerContext &C) const {
+    if (EofVal)
+      return;
+
+    if (const llvm::Optional<int> OptInt =
+            tryExpandAsInteger("EOF", C.getPreprocessor()))
+      EofVal = *OptInt;
+    else
+      EofVal = -1;
+  }
+
   /// Searches for the ExplodedNode where the file descriptor was acquired for
   /// StreamSym.
   static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
@@ -426,8 +437,7 @@
 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?");
+  assert(SS->isOpened() && "Stream is expected to be opened");
 }
 
 const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
@@ -457,6 +467,8 @@
 
 void StreamChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
+  initEof(C);
+
   const FnDescription *Desc = lookupFn(Call);
   if (!Desc || !Desc->PreFn)
     return;
@@ -574,6 +586,10 @@
   if (!SS)
     return;
 
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
   assertStreamStateOpened(SS);
 
   // Close the File Descriptor.
@@ -581,7 +597,16 @@
   // and can not be used any more.
   State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
 
-  C.addTransition(State);
+  // Return 0 on success, EOF on failure.
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef StateSuccess = State->BindExpr(
+      CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy));
+  ProgramStateRef StateFailure =
+      State->BindExpr(CE, C.getLocationContext(),
+                      SVB.makeIntVal(*EofVal, C.getASTContext().IntTy));
+
+  C.addTransition(StateSuccess);
+  C.addTransition(StateFailure);
 }
 
 void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
@@ -663,6 +688,8 @@
     // 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);
+    // It is unspecified what can happen with 'errno'.
+    State = errno_modeling::setErrnoState(State, errno_modeling::Irrelevant);
     C.addTransition(State);
     return;
   }
@@ -780,6 +807,8 @@
 
   assertStreamStateOpened(SS);
 
+  // According to POSIX no change to 'errno' shall happen.
+
   // FilePositionIndeterminate is not cleared.
   State = State->set<StreamMap>(
       StreamSym,
@@ -805,6 +834,8 @@
 
   assertStreamStateOpened(SS);
 
+  // According to POSIX no change to 'errno' shall happen.
+
   if (SS->ErrorState & ErrorKind) {
     // Execution path with error of ErrorKind.
     // Function returns true.
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -244,6 +244,8 @@
     bool CannotBeNull = true;
 
   public:
+    NotNullConstraint(ArgNo ArgN, bool CannotBeNull = true)
+        : ValueConstraint(ArgN), CannotBeNull(CannotBeNull) {}
     std::string describe(ProgramStateRef State,
                          const Summary &Summary) const override;
     StringRef getName() const override { return "NonNull"; }
@@ -275,6 +277,45 @@
     }
   };
 
+  class NotZeroConstraint : public ValueConstraint {
+    using ValueConstraint::ValueConstraint;
+    // This variable has a role when we negate the constraint.
+    bool CannotBeNull = true;
+
+  public:
+    NotZeroConstraint(ArgNo ArgN, bool CannotBeNull = true)
+        : ValueConstraint(ArgN), CannotBeNull(CannotBeNull) {}
+    std::string describe(ProgramStateRef State,
+                         const Summary &Summary) const override;
+    StringRef getName() const override { return "NonZero"; }
+    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
+      SVal V = getArgSVal(Call, getArgNo());
+      if (V.isUndef())
+        return State;
+
+      DefinedOrUnknownSVal L = V.castAs<DefinedOrUnknownSVal>();
+      if (isa<Loc>(L))
+        return State;
+
+      return State->assume(L, CannotBeNull);
+    }
+
+    ValueConstraintPtr negate() const override {
+      NotZeroConstraint Tmp(*this);
+      Tmp.CannotBeNull = !this->CannotBeNull;
+      return std::make_shared<NotZeroConstraint>(Tmp);
+    }
+
+    bool checkSpecificValidity(const FunctionDecl *FD) const override {
+      const bool ValidArg = !getArgType(FD, ArgN)->isPointerType();
+      assert(ValidArg &&
+             "This constraint should be applied only on a non-pointer type");
+      return ValidArg;
+    }
+  };
+
   // Represents a buffer argument with an additional size constraint. The
   // constraint may be a concrete value, or a symbolic value in an argument.
   // Example 1. Concrete value as the minimum buffer size.
@@ -409,6 +450,31 @@
     static int Tag;
   };
 
+  /// Reset errno constraints to irrelevant.
+  /// This is applicable to functions that may change 'errno' and are not
+  /// modeled elsewhere.
+  class ResetErrnoConstraint : public ErrnoConstraintBase {
+  public:
+    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
+      return errno_modeling::setErrnoState(State, errno_modeling::Irrelevant);
+    }
+  };
+
+  /// Do not change errno constraints.
+  /// This is applicable to functions that are modeled in another checker
+  /// and the already set errno constraints should not be changed in the
+  /// post-call event.
+  class NoErrnoConstraint : public ErrnoConstraintBase {
+  public:
+    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+                          const Summary &Summary,
+                          CheckerContext &C) const override {
+      return State;
+    }
+  };
+
   /// Set errno constraint at failure cases of standard functions.
   /// Failure case: 'errno' becomes not equal to 0 and may or may not be checked
   /// by the program. \c ErrnoChecker does not emit a bug report after such a
@@ -446,17 +512,6 @@
     }
   };
 
-  /// Set errno constraints if use of 'errno' is irrelevant to the
-  /// modeled function or modeling is not possible.
-  class NoErrnoConstraint : public ErrnoConstraintBase {
-  public:
-    ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
-                          const Summary &Summary,
-                          CheckerContext &C) const override {
-      return errno_modeling::setErrnoState(State, errno_modeling::Irrelevant);
-    }
-  };
-
   /// A single branch of a function summary.
   ///
   /// A branch is defined by a series of constraints - "assumptions" -
@@ -716,7 +771,8 @@
   /// Usually if a failure return value exists for function, that function
   /// needs different cases for success and failure with different errno
   /// constraints (and different return value constraints).
-  const NoErrnoConstraint ErrnoIrrelevant{};
+  const NoErrnoConstraint ErrnoUnchanged{};
+  const ResetErrnoConstraint ErrnoIrrelevant{};
   const SuccessErrnoConstraint ErrnoMustNotBeChecked{};
   const FailureErrnoConstraint ErrnoNEZeroIrrelevant{};
 };
@@ -743,6 +799,15 @@
   return Result.c_str();
 }
 
+std::string StdLibraryFunctionsChecker::NotZeroConstraint::describe(
+    ProgramStateRef State, const Summary &Summary) const {
+  SmallString<48> Result;
+  Result += "The ";
+  Result += getArgDesc(ArgN);
+  Result += " should not be zero";
+  return Result.c_str();
+}
+
 std::string StdLibraryFunctionsChecker::RangeConstraint::describe(
     ProgramStateRef State, const Summary &Summary) const {
 
@@ -975,8 +1040,10 @@
     if (NewState && NewState != State) {
       if (Case.getNote().empty()) {
         const NoteTag *NT = nullptr;
-        if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
+        if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl())) {
           NT = Case.getErrnoConstraint().describe(C, D->getNameAsString());
+          // llvm::errs()<<D->getNameAsString();
+        }
         C.addTransition(NewState, NT);
       } else {
         StringRef Note = Case.getNote();
@@ -990,6 +1057,11 @@
             /*IsPrunable=*/true);
         C.addTransition(NewState, Tag);
       }
+    } else if (NewState == State) {
+      if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
+        if (const NoteTag *NT =
+                Case.getErrnoConstraint().describe(C, D->getNameAsString()))
+          C.addTransition(NewState, NT);
     }
   }
 }
@@ -1324,6 +1396,9 @@
   auto NotNull = [&](ArgNo ArgN) {
     return std::make_shared<NotNullConstraint>(ArgN);
   };
+  auto IsNull = [&](ArgNo ArgN) {
+    return std::make_shared<NotNullConstraint>(ArgN, false);
+  };
 
   Optional<QualType> FileTy = lookupTy("FILE");
   Optional<QualType> FilePtrTy = getPointerTy(FileTy);
@@ -1553,9 +1628,12 @@
   // read()-like functions that never return more than buffer size.
   auto FreadSummary =
       Summary(NoEvalCall)
-          .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+          .Case({ReturnValueCondition(BO_LT, ArgNo(2)),
                  ReturnValueCondition(WithinRange, Range(0, SizeMax))},
-                ErrnoIrrelevant)
+                ErrnoNEZeroIrrelevant)
+          .Case({ReturnValueCondition(BO_EQ, ArgNo(2)),
+                 ReturnValueCondition(WithinRange, Range(1, SizeMax))},
+                ErrnoMustNotBeChecked)
           .ArgConstraint(NotNull(ArgNo(0)))
           .ArgConstraint(NotNull(ArgNo(3)))
           .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1),
@@ -1643,6 +1721,80 @@
   }
 
   if (ModelPOSIX) {
+    const auto ReturnsZeroOrMinusOne =
+        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
+    const auto ReturnsZero =
+        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(0))};
+    const auto ReturnsMinusOne =
+        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(-1))};
+    const auto ReturnsNonnegative =
+        ConstraintSet{ReturnValueCondition(WithinRange, Range(0, IntMax))};
+    const auto ReturnsFileDescriptor =
+        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, IntMax))};
+    const auto &ReturnsValidFileDescriptor = ReturnsNonnegative;
+
+    // FILE *fopen(const char *restrict pathname, const char *restrict mode);
+    addToFunctionSummaryMap(
+        "fopen",
+        Signature(ArgTypes{ConstCharPtrRestrictTy, ConstCharPtrRestrictTy},
+                  RetType{FilePtrTy}),
+        Summary(NoEvalCall)
+            .Case({NotNull(Ret)}, ErrnoMustNotBeChecked)
+            .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0)))
+            .ArgConstraint(NotNull(ArgNo(1))));
+
+    // FILE *tmpfile(void);
+    addToFunctionSummaryMap("tmpfile",
+                            Signature(ArgTypes{}, RetType{FilePtrTy}),
+                            Summary(NoEvalCall)
+                                .Case({NotNull(Ret)}, ErrnoMustNotBeChecked)
+                                .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant));
+
+    // FILE *freopen(const char *restrict pathname, const char *restrict mode,
+    //               FILE *restrict stream);
+    addToFunctionSummaryMap(
+        "freopen",
+        Signature(ArgTypes{ConstCharPtrRestrictTy, ConstCharPtrRestrictTy,
+                           FilePtrRestrictTy},
+                  RetType{FilePtrTy}),
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(BO_EQ, ArgNo(2))},
+                  ErrnoMustNotBeChecked)
+            .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(1)))
+            .ArgConstraint(NotNull(ArgNo(2))));
+
+    // int fclose(FILE *stream);
+    addToFunctionSummaryMap(
+        "fclose", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(0))},
+                  ErrnoMustNotBeChecked)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv))},
+                  ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0))));
+
+    // int fseek(FILE *stream, long offset, int whence);
+    // FIXME: It is possible to get the 'SEEK_' values (like EOFv) for arg 2
+    // condition.
+    addToFunctionSummaryMap(
+        "fseek", Signature(ArgTypes{FilePtrTy, LongTy, IntTy}, RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(0))},
+                  ErrnoMustNotBeChecked)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(-1))},
+                  ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0)))
+            .ArgConstraint(ArgumentCondition(2, WithinRange, {{0, 2}})));
+
+    // int fileno(FILE *stream);
+    addToFunctionSummaryMap(
+        "fileno", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+            .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(NotNull(ArgNo(0))));
 
     // long a64l(const char *str64);
     addToFunctionSummaryMap(
@@ -1656,18 +1808,6 @@
                                 .ArgConstraint(ArgumentCondition(
                                     0, WithinRange, Range(0, LongMax))));
 
-    const auto ReturnsZeroOrMinusOne =
-        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
-    const auto ReturnsZero =
-        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(0))};
-    const auto ReturnsMinusOne =
-        ConstraintSet{ReturnValueCondition(WithinRange, SingleValue(-1))};
-    const auto ReturnsNonnegative =
-        ConstraintSet{ReturnValueCondition(WithinRange, Range(0, IntMax))};
-    const auto ReturnsFileDescriptor =
-        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, IntMax))};
-    const auto &ReturnsValidFileDescriptor = ReturnsNonnegative;
-
     // int access(const char *pathname, int amode);
     addToFunctionSummaryMap(
         "access", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{IntTy}),
@@ -2154,14 +2294,6 @@
         "rand_r", Signature(ArgTypes{UnsignedIntPtrTy}, RetType{IntTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
-    // int fileno(FILE *stream);
-    addToFunctionSummaryMap(
-        "fileno", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
-        Summary(NoEvalCall)
-            .Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
-            .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
-            .ArgConstraint(NotNull(ArgNo(0))));
-
     // int fseeko(FILE *stream, off_t offset, int whence);
     addToFunctionSummaryMap(
         "fseeko",
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -67,6 +67,9 @@
 /// Set the errno check state, do not modify the errno value.
 ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState);
 
+/// Clear state of errno (make it irrelevant).
+ProgramStateRef clearErrnoState(ProgramStateRef State);
+
 /// Determine if a `Decl` node related to 'errno'.
 /// This is true if the declaration is the errno variable or a function
 /// that returns a pointer to the 'errno' value (usually the 'errno' macro is
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -246,12 +246,16 @@
   return loc::MemRegionVal{ErrnoR};
 }
 
+ErrnoCheckState getErrnoState(ProgramStateRef State) {
+  return State->get<ErrnoState>();
+}
+
 ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState) {
   return State->set<ErrnoState>(EState);
 }
 
-ErrnoCheckState getErrnoState(ProgramStateRef State) {
-  return State->get<ErrnoState>();
+ProgramStateRef clearErrnoState(ProgramStateRef State) {
+  return setErrnoState(State, Irrelevant);
 }
 
 bool isErrno(const Decl *D) {
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
@@ -227,12 +227,12 @@
   // If 'errno' is invalidated we can not know if it is checked or written into,
   // allow read and write without bug reports.
   if (llvm::is_contained(Regions, ErrnoRegion))
-    return setErrnoStateIrrelevant(State);
+    return clearErrnoState(State);
 
   // Always reset errno state when the system memory space is invalidated.
   // The ErrnoRegion is not always found in the list in this case.
   if (llvm::is_contained(Regions, ErrnoRegion->getMemorySpace()))
-    return setErrnoStateIrrelevant(State);
+    return clearErrnoState(State);
 
   return State;
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to