Skip to content

Commit dbaf334

Browse files
balazskebalazske
committed
[clang][analyzer] Fix StreamChecker ftell and fgetpos at indeterminate file position.
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 fec4716 commit dbaf334

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)