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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2eefd19613b8: [clang][analyzer] No end-of-file when seek to 
file begin. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153363/new/

https://reviews.llvm.org/D153363

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


Index: clang/test/Analysis/stream-error.c
===================================================================
--- clang/test/Analysis/stream-error.c
+++ clang/test/Analysis/stream-error.c
@@ -146,7 +146,7 @@
   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 @@
   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)
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -285,7 +285,14 @@
         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 @@
     });
   }
 
-  void initEof(CheckerContext &C) const {
+  void initMacroValues(CheckerContext &C) const {
     if (EofVal)
       return;
 
@@ -441,6 +448,15 @@
       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 @@
 
 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 @@
   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 @@
   // 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 @@
     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)) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits