[clang][analyzer] Add some functions to StreamChecker with errno modeling.

[balazske]

Summary

Functions fsetpos, fgetpos, ftell, rewind are added.
Function fileno is handled by StdLibraryFunctionsChecker too,
tests are added for it but eval function is not needed.

[martong]

clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h

110

Why do you consider important the Std part in the naming?

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

880

Could you please elaborate in the comment why don't we have to use errno_modeling::setErrnoForStdSuccess here?

clang/test/Analysis/stream-error.c

246

This might be TRUE as well isn't it? Because only the "successful call to the fseek() function clears the end-of-file indicator". I'd expect this to be TRUE only in the block of if (rc).

[balazske]

See D135247 and related for a new solution.

clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h

 /// Set \c errno value to be not equal to zero and \c ErrnoCheckState to
 /// \c Irrelevant . The irrelevant errno state ensures that no related bug
 /// report is emitted later and no note tag is needed.
 /// \arg \c ErrnoSym Value to be used for \c errno and constrained to be
 /// non-zero.
 ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
                                       NonLoc ErrnoSym);
 
+/// Set errno state for the common case when a standard function indicates
+/// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
+/// invalidates the errno region (clear of previous value).
+/// At the state transition a note tag created by
+/// \c getNoteTagForStdMustBeChecked can be used.
+/// \arg \c InvalE Expression that causes invalidation of \c errno.
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+                                         CheckerContext &C, const Expr *InvalE);
+
 /// Generate the note tag that can be applied at the state generated by
 /// \c setErrnoForStdSuccess .
 /// \arg \c Fn Name of the (standard) function that is modeled.
 const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
 
+/// Generate the note tag that can be applied at the state generated by
+/// \c setErrnoStdMustBeChecked .
+/// \arg \c Fn Name of the (standard) function that is modeled.
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+                                             llvm::StringRef Fn);
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
 
 #endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ERRNOMODELING_H

clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp

DefinedOrUnknownSVal Cond =
       SVB.evalBinOp(State, BO_NE, ErrnoSym, ZeroVal, SVB.getConditionType())
           .castAs<DefinedOrUnknownSVal>();
   State = State->assume(Cond, true);
   if (!State)
     return nullptr;
   return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
 }
 
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+                                         CheckerContext &C,
+                                         const Expr *InvalE) {
+  const MemRegion *ErrnoR = State->get<ErrnoRegion>();
+  if (!ErrnoR)
+    return State;
+  State = State->invalidateRegions(ErrnoR, InvalE, C.blockCount(),
+                                   C.getLocationContext(), false);
+  if (!State)
+    return nullptr;
+  return setErrnoState(State, MustBeChecked);
+}
+
 const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
   return getErrnoNoteTag(
       C, (Twine("Assuming that function '") + Twine(Fn) +
           Twine("' is successful, in this case the value 'errno' ") +
           Twine(describeErrnoCheckState(MustNotBeChecked)))
              .str());
 }
 
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+                                             llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+      C, (Twine("Function '") + Twine(Fn) +
+          Twine("' indicates failure only by setting of 'errno'"))
+             .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
 
 void ento::registerErrnoModeling(CheckerManager &mgr) {
   mgr.registerChecker<ErrnoModeling>();
 }
 
 bool ento::shouldRegisterErrnoModeling(const CheckerManager &mgr) {
   return true;
 }

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

CallDescriptionMap<FnDescription> FnDescriptions = {
        {&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}},
-      {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
-      {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+      {{"ftell", 1},
+       {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+      {{"rewind", 1},
+       {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
+      {{"fgetpos", 2},
+       {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
+      {{"fsetpos", 2},
+       {&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
       {{"clearerr", 1},
        {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
       {{"feof", 1},
        {&StreamChecker::preDefault,
         std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
         0}},
       {{"ferror", 1},
        {&StreamChecker::preDefault,
         std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
         0}},
+      // 'fileno' is checked sufficiently by StdLibraryFunctionsChecker.
+      // This check additionally checks if the file is in opened state.
       {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
   CallDescriptionMap<FnDescription> FnTestDescriptions = {
       {{"StreamTesterChecker_make_feof_stream", 1},
        {nullptr,
         std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof),
         0}},
private:
   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 evalFgetpos(const FnDescription *Desc, const CallEvent &Call,
+                   CheckerContext &C) const;
+
+  void evalFsetpos(const FnDescription *Desc, const CallEvent &Call,
+                   CheckerContext &C) const;
+
+  void evalFtell(const FnDescription *Desc, const CallEvent &Call,
+                 CheckerContext &C) const;
+
+  void evalRewind(const FnDescription *Desc, const CallEvent &Call,
+                  CheckerContext &C) const;
+
   void preDefault(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
   void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
                     CheckerContext &C) const;
 
   void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
                       CheckerContext &C,
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
                                                       getErrnoSVal(C, Call));
 
   C.addTransition(StateNotFailed,
                   errno_modeling::getNoteTagForStdSuccess(C, "fseek"));
   C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
 }
 
+void StreamChecker::evalFgetpos(const FnDescription *Desc,
+                                const CallEvent &Call,
+                                CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!Sym)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(Sym);
+  if (!SS)
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  DefinedSVal RetVal = makeRetVal(C, CE);
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateFailed, StateNotFailed) =
+      C.getConstraintManager().assumeDual(State, RetVal);
+
+  // In the success case, do not change 'errno' and errno state.
+  StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+                                                      getErrnoSVal(C, Call));
+  if (!StateFailed)
+    return;
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFsetpos(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;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  DefinedSVal RetVal = makeRetVal(C, CE);
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateFailed, StateNotFailed) =
+      C.getConstraintManager().assumeDual(State, RetVal);
+
+  // In the success case, do not change 'errno' and errno state.
+  StateNotFailed = StateNotFailed->set<StreamMap>(
+      StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+
+  // At failure it may happen that ferror is set but not always.
+  // The standards do not tell what happens with the file position at failure.
+  // But we can assume that it is dangerous to make a next I/O operation after
+  // the position was not set correctly (similar to 'fseek').
+  StateFailed = StateFailed->set<StreamMap>(
+      StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
+  StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+                                                      getErrnoSVal(C, Call));
+  if (!StateFailed)
+    return;
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
+                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!Sym)
+    return;
+
+  const StreamState *SS = State->get<StreamMap>(Sym);
+  if (!SS)
+    return;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+  ProgramStateRef StateNotFailed =
+      State->BindExpr(CE, C.getLocationContext(), RetVal);
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
+                            SVB.makeZeroVal(C.getASTContext().LongTy),
+                            SVB.getConditionType())
+                  .getAs<DefinedOrUnknownSVal>();
+  if (!Cond)
+    return;
+  StateNotFailed = StateNotFailed->assume(*Cond, true);
+  if (!StateNotFailed)
+    return;
+
+  ProgramStateRef StateFailed = State->BindExpr(
+      CE, C.getLocationContext(), SVB.makeIntVal(-1, C.getASTContext().LongTy));
+
+  StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+                                                      getErrnoSVal(C, Call));
+  if (!StateFailed)
+    return;
+
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalRewind(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;
+
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  assertStreamStateOpened(SS);
+
+  State = State->set<StreamMap>(StreamSym,
+                                StreamState::getOpened(Desc, ErrorNone, false));
+
+  State =
+      errno_modeling::setErrnoStdMustBeChecked(State, C, Call.getOriginExpr());
+
+  C.addTransition(State,
+                  errno_modeling::getNoteTagForStdMustBeChecked(C, "rewind"));
+}
+
 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;
 

clang/test/Analysis/stream-errno-note.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions \
-// RUN:   -analyzer-output text -verify %s
+// 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}}
void check_fseek(void) {
   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_rewind_errnocheck(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+    return;
+  errno = 0;
+  rewind(F); // expected-note{{Function 'rewind' indicates failure only by setting of 'errno'}}
+  fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
+  // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
+}
+
+// 'fileno' is modeled by StdCLibraryFunctions checker (the errno related parts)
+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);
+}

clang/test/Analysis/stream-errno.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection \
-// RUN:   -verify %s
+// 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);
 
 void check_fopen(void) {
void check_no_errno_change(void) {
   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_fgetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_fsetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_ftell(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+    clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_rewind(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  errno = 0;
+  rewind(F);
+  clang_analyzer_eval(errno == 0);
+  // expected-warning@-1{{FALSE}}
+  // expected-warning@-2{{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'}}
+}

clang/test/Analysis/stream-error.c

 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN: -analyzer-checker=core \
 // RUN: -analyzer-checker=alpha.unix.Stream \
 // RUN: -analyzer-checker=debug.StreamTester \
 // RUN: -analyzer-checker=debug.ExprInspection
 
 #include "Inputs/system-header-simulator.h"
 
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached(void);
 void StreamTesterChecker_make_feof_stream(FILE *);
 void StreamTesterChecker_make_ferror_stream(FILE *);
 
+const char *WBuf = "123456789";
+char RBuf[10];
+
 void error_fopen(void) {
   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_indeterminate_feof2(void) {
   if (fread(Buf, 1, 10, F) < 10) {
     if (ferror(F) == 0) {
       // error is feof, should be non-indeterminate
       fwrite("1", 1, 1, F); // no warning
     }
   }
   fclose(F);
 }
+
+void error_fsetpos(void) {
+  FILE *F = fopen("file", "w");
+  if (!F)
+    return;
+  fpos_t Pos;
+  int rc = fsetpos(F, &Pos);
+  clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}}
+  if (rc) {
+    if (ferror(F))
+      clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+    else
+      clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  } else {
+    clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}}
+  }
+  fwrite(WBuf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
+  fclose(F);
+}
+
+void error_rewind(void) {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  fread(RBuf, 1, 10, F);
+  if (feof(F)) {
+    rewind(F);
+    clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}}
+  } else if (ferror(F)) {
+    rewind(F);
+    clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}}
+  }
+  fclose(F);
+}