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

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2023-06-30T10:29:49+02:00
New Revision: 2eefd19613b8ea1f4cf59a4f0321bdc5f5ab0841

URL: https://github.com/llvm/llvm-project/commit/2eefd19613b8ea1f4cf59a4f0321bdc5f5ab0841
DIFF: https://github.com/llvm/llvm-project/commit/2eefd19613b8ea1f4cf59a4f0321bdc5f5ab0841.diff

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

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.

Reviewed By: donat.nagy

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

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
    clang/test/Analysis/stream-error.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 5081ff63102b31..bad86682c91e25 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -285,7 +285,14 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
         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;
@@ -432,7 +439,7 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
     });
   }
 
-  void initEof(CheckerContext &C) const {
+  void initMacroValues(CheckerContext &C) const {
     if (EofVal)
       return;
 
@@ -441,6 +448,15 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
       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
@@ -488,7 +504,7 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
 
 void StreamChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
-  initEof(C);
+  initMacroValues(C);
 
   const FnDescription *Desc = lookupFn(Call);
   if (!Desc || !Desc->PreFn)
@@ -786,6 +802,11 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   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.
@@ -804,9 +825,12 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   // 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));
@@ -1153,7 +1177,7 @@ StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
     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)) {

diff  --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index cfb642926a3ff0..3c00e59bb6bd19 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -146,7 +146,7 @@ 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.
@@ -173,6 +173,35 @@ void error_fseek(void) {
   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 at -1 {{FALSE}}
+    // expected-warning at -2 {{TRUE}}
+    clang_analyzer_eval(IsFEof);
+    // expected-warning at -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)