Skip to content

Commit d72b7f9

Browse files
balazskebalazske
authored
[clang][analyzer] Fix StreamChecker ftell and fgetpos at indeterminate file position. (#84191)
These functions should not be allowed if the file position is indeterminate (they return the file position). This condition is now checked, and tests are improved to check it.
1 parent df9be01 commit d72b7f9

File tree

2 files changed

+80
-38
lines changed

2 files changed

+80
-38
lines changed

‎clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -307,64 +307,64 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
307307
{{{"fclose"}, 1},
308308
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
309309
{{{"fread"}, 4},
310-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
310+
{&StreamChecker::preRead,
311311
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
312312
{{{"fwrite"}, 4},
313-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
313+
{&StreamChecker::preWrite,
314314
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
315315
{{{"fgetc"}, 1},
316-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
316+
{&StreamChecker::preRead,
317317
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
318318
{{{"fgets"}, 3},
319-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
319+
{&StreamChecker::preRead,
320320
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}},
321321
{{{"getc"}, 1},
322-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
322+
{&StreamChecker::preRead,
323323
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
324324
{{{"fputc"}, 2},
325-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
325+
{&StreamChecker::preWrite,
326326
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
327327
{{{"fputs"}, 2},
328-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
328+
{&StreamChecker::preWrite,
329329
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
330330
{{{"putc"}, 2},
331-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
331+
{&StreamChecker::preWrite,
332332
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
333333
{{{"fprintf"}},
334-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
334+
{&StreamChecker::preWrite,
335335
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
336336
{{{"vfprintf"}, 3},
337-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
337+
{&StreamChecker::preWrite,
338338
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
339339
{{{"fscanf"}},
340-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
340+
{&StreamChecker::preRead,
341341
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
342342
{{{"vfscanf"}, 3},
343-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
343+
{&StreamChecker::preRead,
344344
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
345345
{{{"ungetc"}, 2},
346-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
346+
{&StreamChecker::preWrite,
347347
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
348348
{{{"getdelim"}, 4},
349-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
349+
{&StreamChecker::preRead,
350350
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}},
351351
{{{"getline"}, 3},
352-
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
352+
{&StreamChecker::preRead,
353353
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}},
354354
{{{"fseek"}, 3},
355355
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
356356
{{{"fseeko"}, 3},
357357
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
358358
{{{"ftell"}, 1},
359-
{&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
359+
{&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
360360
{{{"ftello"}, 1},
361-
{&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
361+
{&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
362362
{{{"fflush"}, 1},
363363
{&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}},
364364
{{{"rewind"}, 1},
365365
{&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
366366
{{{"fgetpos"}, 2},
367-
{&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
367+
{&StreamChecker::preWrite, &StreamChecker::evalFgetpos, 0}},
368368
{{{"fsetpos"}, 2},
369369
{&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
370370
{{{"clearerr"}, 1},
@@ -384,12 +384,18 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
384384
CallDescriptionMap<FnDescription> FnTestDescriptions = {
385385
{{{"StreamTesterChecker_make_feof_stream"}, 1},
386386
{nullptr,
387-
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof),
387+
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof,
388+
false),
388389
0}},
389390
{{{"StreamTesterChecker_make_ferror_stream"}, 1},
390391
{nullptr,
391392
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
392-
ErrorFError),
393+
ErrorFError, false),
394+
0}},
395+
{{{"StreamTesterChecker_make_ferror_indeterminate_stream"}, 1},
396+
{nullptr,
397+
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
398+
ErrorFError, true),
393399
0}},
394400
};
395401

@@ -415,8 +421,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
415421
void evalFclose(const FnDescription *Desc, const CallEvent &Call,
416422
CheckerContext &C) const;
417423

418-
void preReadWrite(const FnDescription *Desc, const CallEvent &Call,
419-
CheckerContext &C, bool IsRead) const;
424+
void preRead(const FnDescription *Desc, const CallEvent &Call,
425+
CheckerContext &C) const;
426+
427+
void preWrite(const FnDescription *Desc, const CallEvent &Call,
428+
CheckerContext &C) const;
420429

421430
void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
422431
CheckerContext &C, bool IsFread) const;
@@ -467,8 +476,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
467476
const StreamErrorState &ErrorKind) const;
468477

469478
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
470-
CheckerContext &C,
471-
const StreamErrorState &ErrorKind) const;
479+
CheckerContext &C, const StreamErrorState &ErrorKind,
480+
bool Indeterminate) const;
472481

473482
void preFflush(const FnDescription *Desc, const CallEvent &Call,
474483
CheckerContext &C) const;
@@ -849,9 +858,8 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
849858
C.addTransition(E.bindReturnValue(State, C, *EofVal));
850859
}
851860

852-
void StreamChecker::preReadWrite(const FnDescription *Desc,
853-
const CallEvent &Call, CheckerContext &C,
854-
bool IsRead) const {
861+
void StreamChecker::preRead(const FnDescription *Desc, const CallEvent &Call,
862+
CheckerContext &C) const {
855863
ProgramStateRef State = C.getState();
856864
SVal StreamVal = getStreamArg(Desc, Call);
857865
State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
@@ -865,11 +873,6 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
865873
if (!State)
866874
return;
867875

868-
if (!IsRead) {
869-
C.addTransition(State);
870-
return;
871-
}
872-
873876
SymbolRef Sym = StreamVal.getAsSymbol();
874877
if (Sym && State->get<StreamMap>(Sym)) {
875878
const StreamState *SS = State->get<StreamMap>(Sym);
@@ -880,6 +883,24 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
880883
}
881884
}
882885

886+
void StreamChecker::preWrite(const FnDescription *Desc, const CallEvent &Call,
887+
CheckerContext &C) const {
888+
ProgramStateRef State = C.getState();
889+
SVal StreamVal = getStreamArg(Desc, Call);
890+
State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
891+
State);
892+
if (!State)
893+
return;
894+
State = ensureStreamOpened(StreamVal, C, State);
895+
if (!State)
896+
return;
897+
State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
898+
if (!State)
899+
return;
900+
901+
C.addTransition(State);
902+
}
903+
883904
void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
884905
const CallEvent &Call, CheckerContext &C,
885906
bool IsFread) const {
@@ -1496,14 +1517,16 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
14961517

14971518
void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
14981519
const CallEvent &Call, CheckerContext &C,
1499-
const StreamErrorState &ErrorKind) const {
1520+
const StreamErrorState &ErrorKind,
1521+
bool Indeterminate) const {
15001522
ProgramStateRef State = C.getState();
15011523
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
15021524
assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
15031525
const StreamState *SS = State->get<StreamMap>(StreamSym);
15041526
assert(SS && "Stream should be tracked by the checker.");
15051527
State = State->set<StreamMap>(
1506-
StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind));
1528+
StreamSym,
1529+
StreamState::getOpened(SS->LastOperation, ErrorKind, Indeterminate));
15071530
C.addTransition(State);
15081531
}
15091532

‎clang/test/Analysis/stream-error.c

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ void clang_analyzer_dump(int);
1111
void clang_analyzer_warnIfReached(void);
1212
void StreamTesterChecker_make_feof_stream(FILE *);
1313
void StreamTesterChecker_make_ferror_stream(FILE *);
14+
void StreamTesterChecker_make_ferror_indeterminate_stream(FILE *);
1415

1516
void error_fopen(void) {
1617
FILE *F = fopen("file", "r");
@@ -52,6 +53,8 @@ void stream_error_feof(void) {
5253
clearerr(F);
5354
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
5455
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
56+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
57+
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
5558
fclose(F);
5659
}
5760

@@ -65,6 +68,8 @@ void stream_error_ferror(void) {
6568
clearerr(F);
6669
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
6770
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
71+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
72+
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
6873
fclose(F);
6974
}
7075

@@ -233,7 +238,7 @@ void error_fscanf(int *A) {
233238
fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}}
234239
}
235240

236-
void error_ungetc() {
241+
void error_ungetc(int TestIndeterminate) {
237242
FILE *F = tmpfile();
238243
if (!F)
239244
return;
@@ -245,8 +250,12 @@ void error_ungetc() {
245250
clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
246251
}
247252
fputc('Y', F); // no-warning
253+
if (TestIndeterminate) {
254+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
255+
ungetc('X', F); // expected-warning {{might be 'indeterminate'}}
256+
}
248257
fclose(F);
249-
ungetc('A', F); // expected-warning {{Stream might be already closed}}
258+
ungetc('A', F); // expected-warning {{Stream might be already closed}}
250259
}
251260

252261
void error_getdelim(char *P, size_t Sz) {
@@ -449,7 +458,7 @@ void error_fseeko_0(void) {
449458
fclose(F);
450459
}
451460

452-
void error_ftell(void) {
461+
void error_ftell(int TestIndeterminate) {
453462
FILE *F = fopen("file", "r");
454463
if (!F)
455464
return;
@@ -467,10 +476,14 @@ void error_ftell(void) {
467476
rc = ftell(F);
468477
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
469478
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
479+
if (TestIndeterminate) {
480+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
481+
ftell(F); // expected-warning {{might be 'indeterminate'}}
482+
}
470483
fclose(F);
471484
}
472485

473-
void error_ftello(void) {
486+
void error_ftello(int TestIndeterminate) {
474487
FILE *F = fopen("file", "r");
475488
if (!F)
476489
return;
@@ -488,6 +501,10 @@ void error_ftello(void) {
488501
rc = ftello(F);
489502
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
490503
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
504+
if (TestIndeterminate) {
505+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
506+
ftell(F); // expected-warning {{might be 'indeterminate'}}
507+
}
491508
fclose(F);
492509
}
493510

@@ -506,6 +523,8 @@ void error_fileno(void) {
506523
N = fileno(F);
507524
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
508525
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
526+
StreamTesterChecker_make_ferror_indeterminate_stream(F);
527+
fileno(F); // no warning
509528
fclose(F);
510529
}
511530

0 commit comments

Comments
 (0)