[clang] 93c387d - [clang][analyzer] Change modeling of `fseek` in StreamChecker. (#86919)

via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 1 23:55:24 PDT 2024


Author: Balázs Kéri
Date: 2024-04-02T08:55:20+02:00
New Revision: 93c387df908923f17875ab9cf0463d5f181318bb

URL: https://github.com/llvm/llvm-project/commit/93c387df908923f17875ab9cf0463d5f181318bb
DIFF: https://github.com/llvm/llvm-project/commit/93c387df908923f17875ab9cf0463d5f181318bb.diff

LOG: [clang][analyzer] Change modeling of `fseek` in StreamChecker. (#86919)

Until now function `fseek` returned nonzero on error, this is changed to
-1 only. And it does not produce EOF error any more.
This complies better with the POSIX standard.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 902c42a2799be4..069e3a633c1214 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1264,15 +1264,10 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   if (!E.Init(Desc, Call, C, State))
     return;
 
-  const llvm::APSInt *PosV =
-      C.getSValBuilder().getKnownValue(State, Call.getArgSVal(1));
-  const llvm::APSInt *WhenceV =
-      C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
-
   // Bifurcate the state into failed and non-failed.
-  // Return zero on success, nonzero on error.
-  ProgramStateRef StateNotFailed, StateFailed;
-  std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
+  // Return zero on success, -1 on error.
+  ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, 0);
+  ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
 
   // No failure: Reset the state to opened with no error.
   StateNotFailed =
@@ -1282,12 +1277,10 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   // At 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 = E.setStreamState(StateFailed,
-                                 StreamState::getOpened(Desc, NewErrS, true));
+  // It is allowed to set the position beyond the end of the file. EOF error
+  // should not occur.
+  StateFailed = E.setStreamState(
+      StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
   C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
 }
 

diff  --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 88f7de4234ffb4..7f9116ff401445 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -365,27 +365,22 @@ void error_fseek(void) {
     return;
   int rc = fseek(F, 1, SEEK_SET);
   if (rc) {
+    clang_analyzer_eval(rc == -1);     // expected-warning {{TRUE}}
     int IsFEof = feof(F), IsFError = ferror(F);
-    // Get feof or ferror or no error.
-    clang_analyzer_eval(IsFEof || IsFError);
-    // expected-warning at -1 {{FALSE}}
-    // expected-warning at -2 {{TRUE}}
-    clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
+    // Get ferror or no error.
+    clang_analyzer_eval(IsFError);     // expected-warning {{FALSE}} \
+                                       // expected-warning {{TRUE}}
+    clang_analyzer_eval(IsFEof);       // expected-warning {{FALSE}}
     // Error flags should not change.
-    if (IsFEof)
-      clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
-    else
-      clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+    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}}
+      clang_analyzer_eval(ferror(F));  // expected-warning {{TRUE}}
   } else {
-    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
-    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+    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}}
+    clang_analyzer_eval(feof(F));      // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F));    // expected-warning {{FALSE}}
   }
   fclose(F);
 }
@@ -396,15 +391,13 @@ void error_fseeko(void) {
     return;
   int rc = fseeko(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 at -1 {{FALSE}}
-    // expected-warning at -2 {{TRUE}}
-    clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
+    // Get ferror or no error.
+    clang_analyzer_eval(ferror(F));  // expected-warning {{FALSE}} \
+                                     // expected-warning {{TRUE}}
+    clang_analyzer_eval(feof(F));    // expected-warning {{FALSE}}
   } else {
-    clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
-    clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+    clang_analyzer_eval(feof(F));    // expected-warning {{FALSE}}
+    clang_analyzer_eval(ferror(F));  // expected-warning {{FALSE}}
   }
   fclose(F);
 }
@@ -414,7 +407,7 @@ void error_fseek_0(void) {
   if (!F)
     return;
   int rc = fseek(F, 0, SEEK_SET);
-  if (rc) {
+  if (rc == -1) {
     int IsFEof = feof(F), IsFError = ferror(F);
     // Get ferror or no error, but not feof.
     clang_analyzer_eval(IsFError);

diff  --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index f77cd4aa62841d..54ea699f46674e 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -226,10 +226,39 @@ void check_indeterminate_fseek(void) {
     return;
   int Ret = fseek(F, 1, SEEK_SET);  // expected-note {{Assuming this stream operation fails}}
   if (Ret) {                        // expected-note {{Taking true branch}} \
-                                    // expected-note {{'Ret' is not equal to 0}}
+                                    // expected-note {{'Ret' is -1}}
     char Buf[2];
     fwrite(Buf, 1, 2, F);           // expected-warning {{might be 'indeterminate'}} \
                                     // expected-note {{might be 'indeterminate'}}
   }
   fclose(F);
 }
+
+void error_fseek_ftell(void) {
+  FILE *F = fopen("file", "r");
+  if (!F)                 // expected-note {{Taking false branch}} \
+                          // expected-note {{'F' is non-null}}
+    return;
+  fseek(F, 0, SEEK_END);  // expected-note {{Assuming this stream operation fails}}
+  long size = ftell(F);   // expected-warning {{might be 'indeterminate'}} \
+                          // expected-note {{might be 'indeterminate'}}
+  if (size == -1) {
+    fclose(F);
+    return;
+  }
+  if (size == 1)
+    fprintf(F, "abcd");
+  fclose(F);
+}
+
+void error_fseek_read_eof(void) {
+  FILE *F = fopen("file", "r");
+  if (!F)
+    return;
+  if (fseek(F, 22, SEEK_SET) == -1) {
+    fclose(F);
+    return;
+  }
+  fgetc(F); // no warning
+  fclose(F);
+}


        


More information about the cfe-commits mailing list