Skip to content

Commit 93c387d

Browse files
authored
[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.
1 parent 24d528c commit 93c387d

File tree

3 files changed

+55
-40
lines changed

3 files changed

+55
-40
lines changed

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,15 +1264,10 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
12641264
if (!E.Init(Desc, Call, C, State))
12651265
return;
12661266

1267-
const llvm::APSInt *PosV =
1268-
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(1));
1269-
const llvm::APSInt *WhenceV =
1270-
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
1271-
12721267
// Bifurcate the state into failed and non-failed.
1273-
// Return zero on success, nonzero on error.
1274-
ProgramStateRef StateNotFailed, StateFailed;
1275-
std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
1268+
// Return zero on success, -1 on error.
1269+
ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, 0);
1270+
ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
12761271

12771272
// No failure: Reset the state to opened with no error.
12781273
StateNotFailed =
@@ -1282,12 +1277,10 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
12821277
// At error it is possible that fseek fails but sets none of the error flags.
12831278
// If fseek failed, assume that the file position becomes indeterminate in any
12841279
// case.
1285-
StreamErrorState NewErrS = ErrorNone | ErrorFError;
1286-
// Setting the position to start of file never produces EOF error.
1287-
if (!(PosV && *PosV == 0 && WhenceV && *WhenceV == SeekSetVal))
1288-
NewErrS = NewErrS | ErrorFEof;
1289-
StateFailed = E.setStreamState(StateFailed,
1290-
StreamState::getOpened(Desc, NewErrS, true));
1280+
// It is allowed to set the position beyond the end of the file. EOF error
1281+
// should not occur.
1282+
StateFailed = E.setStreamState(
1283+
StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
12911284
C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
12921285
}
12931286

clang/test/Analysis/stream-error.c

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -365,27 +365,22 @@ void error_fseek(void) {
365365
return;
366366
int rc = fseek(F, 1, SEEK_SET);
367367
if (rc) {
368+
clang_analyzer_eval(rc == -1); // expected-warning {{TRUE}}
368369
int IsFEof = feof(F), IsFError = ferror(F);
369-
// Get feof or ferror or no error.
370-
clang_analyzer_eval(IsFEof || IsFError);
371-
// expected-warning@-1 {{FALSE}}
372-
// expected-warning@-2 {{TRUE}}
373-
clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
370+
// Get ferror or no error.
371+
clang_analyzer_eval(IsFError); // expected-warning {{FALSE}} \
372+
// expected-warning {{TRUE}}
373+
clang_analyzer_eval(IsFEof); // expected-warning {{FALSE}}
374374
// Error flags should not change.
375-
if (IsFEof)
376-
clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
377-
else
378-
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
375+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
379376
if (IsFError)
380-
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
381-
else
382-
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
377+
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
383378
} else {
384-
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
385-
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
379+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
380+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
386381
// Error flags should not change.
387-
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
388-
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
382+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
383+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
389384
}
390385
fclose(F);
391386
}
@@ -396,15 +391,13 @@ void error_fseeko(void) {
396391
return;
397392
int rc = fseeko(F, 1, SEEK_SET);
398393
if (rc) {
399-
int IsFEof = feof(F), IsFError = ferror(F);
400-
// Get feof or ferror or no error.
401-
clang_analyzer_eval(IsFEof || IsFError);
402-
// expected-warning@-1 {{FALSE}}
403-
// expected-warning@-2 {{TRUE}}
404-
clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}}
394+
// Get ferror or no error.
395+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} \
396+
// expected-warning {{TRUE}}
397+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
405398
} else {
406-
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
407-
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
399+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
400+
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
408401
}
409402
fclose(F);
410403
}
@@ -414,7 +407,7 @@ void error_fseek_0(void) {
414407
if (!F)
415408
return;
416409
int rc = fseek(F, 0, SEEK_SET);
417-
if (rc) {
410+
if (rc == -1) {
418411
int IsFEof = feof(F), IsFError = ferror(F);
419412
// Get ferror or no error, but not feof.
420413
clang_analyzer_eval(IsFError);

clang/test/Analysis/stream-note.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,39 @@ void check_indeterminate_fseek(void) {
226226
return;
227227
int Ret = fseek(F, 1, SEEK_SET); // expected-note {{Assuming this stream operation fails}}
228228
if (Ret) { // expected-note {{Taking true branch}} \
229-
// expected-note {{'Ret' is not equal to 0}}
229+
// expected-note {{'Ret' is -1}}
230230
char Buf[2];
231231
fwrite(Buf, 1, 2, F); // expected-warning {{might be 'indeterminate'}} \
232232
// expected-note {{might be 'indeterminate'}}
233233
}
234234
fclose(F);
235235
}
236+
237+
void error_fseek_ftell(void) {
238+
FILE *F = fopen("file", "r");
239+
if (!F) // expected-note {{Taking false branch}} \
240+
// expected-note {{'F' is non-null}}
241+
return;
242+
fseek(F, 0, SEEK_END); // expected-note {{Assuming this stream operation fails}}
243+
long size = ftell(F); // expected-warning {{might be 'indeterminate'}} \
244+
// expected-note {{might be 'indeterminate'}}
245+
if (size == -1) {
246+
fclose(F);
247+
return;
248+
}
249+
if (size == 1)
250+
fprintf(F, "abcd");
251+
fclose(F);
252+
}
253+
254+
void error_fseek_read_eof(void) {
255+
FILE *F = fopen("file", "r");
256+
if (!F)
257+
return;
258+
if (fseek(F, 22, SEEK_SET) == -1) {
259+
fclose(F);
260+
return;
261+
}
262+
fgetc(F); // no warning
263+
fclose(F);
264+
}

0 commit comments

Comments
 (0)