[clang] [clang][analyzer] Change modeling of `fseek` in StreamChecker. (PR #86919)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 28 00:56:18 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/86919.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+7-14)
- (modified) clang/test/Analysis/stream-error.c (+18-25)
- (modified) clang/test/Analysis/stream-note.c (+30-1)
``````````diff
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);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/86919
More information about the cfe-commits
mailing list