Skip to content

Commit

Permalink
[clang][analyzer] Change modeling of fseek in StreamChecker. (#86919)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
balazske committed Apr 2, 2024
1 parent 24d528c commit 93c387d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 40 deletions.
21 changes: 7 additions & 14 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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));
}

Expand Down
43 changes: 18 additions & 25 deletions clang/test/Analysis/stream-error.c
Original file line number Diff line number Diff line change
Expand Up @@ -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@-1 {{FALSE}}
// expected-warning@-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);
}
Expand All @@ -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@-1 {{FALSE}}
// expected-warning@-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);
}
Expand All @@ -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);
Expand Down
31 changes: 30 additions & 1 deletion clang/test/Analysis/stream-note.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 93c387d

Please sign in to comment.