[clang][analyzer] No end-of-file when seek to file begin.

[balazske]

Summary

If fseek is used with 0 position and SEEK_SET it sets the position
to the start of the file. This should not cause FEOF (end of file) error.
The case of an empty file is not handled for simplification.
It is not exactly defined in what cases fseek produces the different
error states. Normally feof should not happen at all because it is
possible to set the position after the end of file, but previous tests
showed that still feof (and any other error cases) can happen.

[balazske]

This case is an example of what can be fixed by the change.

[donat.nagy]

LGTM.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

CallDescriptionMap<FnDescription> FnTestDescriptions = {
         0}},
       {{{"StreamTesterChecker_make_ferror_stream"}, 1},
        {nullptr,
         std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
                   ErrorFError),
         0}},
   };
 
+  /// Expanded value of EOF, empty before initialization.
   mutable std::optional<int> EofVal;
+  /// Expanded value of SEEK_SET, 0 if not found.
+  mutable int SeekSetVal = 0;
+  /// Expanded value of SEEK_CUR, 1 if not found.
+  mutable int SeekCurVal = 1;
+  /// Expanded value of SEEK_END, 2 if not found.
+  mutable int SeekEndVal = 2;
 
   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;
return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) {
         return "";
 
       BR.markNotInteresting(StreamSym);
 
       return "Assuming stream reaches end-of-file here";
     });
   }
 
-  void initEof(CheckerContext &C) const {
+  void initMacroValues(CheckerContext &C) const {
     if (EofVal)
       return;
 
     if (const std::optional<int> OptInt =
             tryExpandAsInteger("EOF", C.getPreprocessor()))
       EofVal = *OptInt;
     else
       EofVal = -1;
+    if (const std::optional<int> OptInt =
+            tryExpandAsInteger("SEEK_SET", C.getPreprocessor()))
+      SeekSetVal = *OptInt;
+    if (const std::optional<int> OptInt =
+            tryExpandAsInteger("SEEK_END", C.getPreprocessor()))
+      SeekEndVal = *OptInt;
+    if (const std::optional<int> OptInt =
+            tryExpandAsInteger("SEEK_CUR", C.getPreprocessor()))
+      SeekCurVal = *OptInt;
   }
 
   /// Searches for the ExplodedNode where the file descriptor was acquired for
   /// StreamSym.
   static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
                                                 SymbolRef StreamSym,
                                                 CheckerContext &C);
 };

 }
 
 //===----------------------------------------------------------------------===//
 // Methods of StreamChecker.
 //===----------------------------------------------------------------------===//
 
 void StreamChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
-  initEof(C);
+  initMacroValues(C);
 
   const FnDescription *Desc = lookupFn(Call);
   if (!Desc || !Desc->PreFn)
     return;
 
   Desc->PreFn(this, Desc, Call, C);
 }
 
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return;
 
   // Ignore the call if the stream is not tracked.
   if (!State->get<StreamMap>(StreamSym))
     return;
 
+  const llvm::APSInt *PosV =
+      C.getSValBuilder().getKnownValue(State, Call.getArgSVal(1));
+  const llvm::APSInt *WhenceV =
+      C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
+
   DefinedSVal RetVal = makeRetVal(C, CE);
 
   // Make expression result.
   State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
   // Bifurcate the state into failed and non-failed.
   // Return zero on success, nonzero on error.
   ProgramStateRef StateNotFailed, StateFailed;
   std::tie(StateFailed, StateNotFailed) =
       C.getConstraintManager().assumeDual(State, RetVal);
 
   // Reset the state to opened with no error.
   StateNotFailed =
       StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
   // We get error.
   // It is possible that fseek fails but sets none of the error flags.
   // If fseek failed, assume that the file position becomes indeterminate in any
   // case.
+  StreamErrorState NewErrS = ErrorNone | ErrorFError;
+  // Setting the position to start of file never produces EOF error.
+  if (!(PosV && *PosV == 0 && WhenceV && *WhenceV == SeekSetVal))
+    NewErrS = NewErrS | ErrorFEof;
   StateFailed = StateFailed->set<StreamMap>(
-      StreamSym,
-      StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
+      StreamSym, StreamState::getOpened(Desc, NewErrS, true));
 
   C.addTransition(StateNotFailed);
   C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
 }
 
 void StreamChecker::evalFgetpos(const FnDescription *Desc,
                                 const CallEvent &Call,
                                 CheckerContext &C) const {

 StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
                                         ProgramStateRef State) const {
   std::optional<nonloc::ConcreteInt> CI =
       WhenceVal.getAs<nonloc::ConcreteInt>();
   if (!CI)
     return State;
 
   int64_t X = CI->getValue().getSExtValue();
-  if (X >= 0 && X <= 2)
+  if (X == SeekSetVal || X == SeekCurVal || X == SeekEndVal)
     return State;
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
         BT_IllegalWhence,
         "The whence argument to fseek() should be "
         "SEEK_SET, SEEK_END, or SEEK_CUR.",
         N));

clang/test/Analysis/stream-error.c

void error_fread_fwrite_zerosize(void) {
 
   fclose(F);
 }
 
 void error_fseek(void) {
   FILE *F = fopen("file", "r");
   if (!F)
     return;
-  int rc = fseek(F, 0, SEEK_SET);
+  int rc = fseek(F, 1, SEEK_SET);
   if (rc) {
     int IsFEof = feof(F), IsFError = ferror(F);
     // Get feof or ferror or no error.
     clang_analyzer_eval(IsFEof || IsFError);
     // expected-warning@-1 {{FALSE}}
     // expected-warning@-2 {{TRUE}}
     clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
     // Error flags should not change.
if (rc) {
     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_0(void) {
+  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 ferror or no error, but not feof.
+    clang_analyzer_eval(IsFError);
+    // expected-warning@-1 {{FALSE}}
+    // expected-warning@-2 {{TRUE}}
+    clang_analyzer_eval(IsFEof);
+    // expected-warning@-1 {{FALSE}}
+    // Error flags should not change.
+    clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+    if (IsFError)
+      clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+    else
+      clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+    // Error flags should not change.
+    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  }
+  fclose(F);
+}
+
 void error_indeterminate(void) {
   FILE *F = fopen("file", "r+");
   if (!F)
     return;
   const char *Buf = "123456789";
   int rc = fseek(F, 0, SEEK_SET);
   if (rc) {
     if (feof(F)) {