[clang] [clang][analyzer] Fix StreamChecker `ftell` and `fgetpos` at indeterminate file position. (PR #84191)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 7 00:50:07 PST 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/84191
>From dbaf3348510582c013254ed48b69663b42816be0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 6 Mar 2024 16:01:01 +0100
Subject: [PATCH] [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.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 91 ++++++++++++-------
clang/test/Analysis/stream-error.c | 27 +++++-
2 files changed, 80 insertions(+), 38 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..10972158f39862 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -307,64 +307,64 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
{{{"fclose"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
{{{"fread"}, 4},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
{{{"fwrite"}, 4},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
{{{"fgetc"}, 1},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
{{{"fgets"}, 3},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}},
{{{"getc"}, 1},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
{{{"fputc"}, 2},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
{{{"fputs"}, 2},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
{{{"putc"}, 2},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
{{{"fprintf"}},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
{{{"vfprintf"}, 3},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
{{{"fscanf"}},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
{{{"vfscanf"}, 3},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
{{{"ungetc"}, 2},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ {&StreamChecker::preWrite,
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
{{{"getdelim"}, 4},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}},
{{{"getline"}, 3},
- {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ {&StreamChecker::preRead,
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}},
{{{"fseek"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"fseeko"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"ftell"}, 1},
- {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+ {&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
{{{"ftello"}, 1},
- {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+ {&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
{{{"fflush"}, 1},
{&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}},
{{{"rewind"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
{{{"fgetpos"}, 2},
- {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
+ {&StreamChecker::preWrite, &StreamChecker::evalFgetpos, 0}},
{{{"fsetpos"}, 2},
{&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
{{{"clearerr"}, 1},
@@ -384,12 +384,18 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
CallDescriptionMap<FnDescription> FnTestDescriptions = {
{{{"StreamTesterChecker_make_feof_stream"}, 1},
{nullptr,
- std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof),
+ std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof,
+ false),
0}},
{{{"StreamTesterChecker_make_ferror_stream"}, 1},
{nullptr,
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
- ErrorFError),
+ ErrorFError, false),
+ 0}},
+ {{{"StreamTesterChecker_make_ferror_indeterminate_stream"}, 1},
+ {nullptr,
+ std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
+ ErrorFError, true),
0}},
};
@@ -415,8 +421,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFclose(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- void preReadWrite(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C, bool IsRead) const;
+ void preRead(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preWrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsFread) const;
@@ -467,8 +476,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
const StreamErrorState &ErrorKind) const;
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C,
- const StreamErrorState &ErrorKind) const;
+ CheckerContext &C, const StreamErrorState &ErrorKind,
+ bool Indeterminate) const;
void preFflush(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -849,9 +858,8 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(E.bindReturnValue(State, C, *EofVal));
}
-void StreamChecker::preReadWrite(const FnDescription *Desc,
- const CallEvent &Call, CheckerContext &C,
- bool IsRead) const {
+void StreamChecker::preRead(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
SVal StreamVal = getStreamArg(Desc, Call);
State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
@@ -865,11 +873,6 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
if (!State)
return;
- if (!IsRead) {
- C.addTransition(State);
- return;
- }
-
SymbolRef Sym = StreamVal.getAsSymbol();
if (Sym && State->get<StreamMap>(Sym)) {
const StreamState *SS = State->get<StreamMap>(Sym);
@@ -880,6 +883,24 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
}
}
+void StreamChecker::preWrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+ return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+ return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+ return;
+
+ C.addTransition(State);
+}
+
void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
const CallEvent &Call, CheckerContext &C,
bool IsFread) const {
@@ -1496,14 +1517,16 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
const CallEvent &Call, CheckerContext &C,
- const StreamErrorState &ErrorKind) const {
+ const StreamErrorState &ErrorKind,
+ bool Indeterminate) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
const StreamState *SS = State->get<StreamMap>(StreamSym);
assert(SS && "Stream should be tracked by the checker.");
State = State->set<StreamMap>(
- StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind));
+ StreamSym,
+ StreamState::getOpened(SS->LastOperation, ErrorKind, Indeterminate));
C.addTransition(State);
}
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index ac31083bfc691f..88f7de4234ffb4 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -11,6 +11,7 @@ void clang_analyzer_dump(int);
void clang_analyzer_warnIfReached(void);
void StreamTesterChecker_make_feof_stream(FILE *);
void StreamTesterChecker_make_ferror_stream(FILE *);
+void StreamTesterChecker_make_ferror_indeterminate_stream(FILE *);
void error_fopen(void) {
FILE *F = fopen("file", "r");
@@ -52,6 +53,8 @@ void stream_error_feof(void) {
clearerr(F);
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
fclose(F);
}
@@ -65,6 +68,8 @@ void stream_error_ferror(void) {
clearerr(F);
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
fclose(F);
}
@@ -233,7 +238,7 @@ void error_fscanf(int *A) {
fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}}
}
-void error_ungetc() {
+void error_ungetc(int TestIndeterminate) {
FILE *F = tmpfile();
if (!F)
return;
@@ -245,8 +250,12 @@ void error_ungetc() {
clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
}
fputc('Y', F); // no-warning
+ if (TestIndeterminate) {
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ ungetc('X', F); // expected-warning {{might be 'indeterminate'}}
+ }
fclose(F);
- ungetc('A', F); // expected-warning {{Stream might be already closed}}
+ ungetc('A', F); // expected-warning {{Stream might be already closed}}
}
void error_getdelim(char *P, size_t Sz) {
@@ -449,7 +458,7 @@ void error_fseeko_0(void) {
fclose(F);
}
-void error_ftell(void) {
+void error_ftell(int TestIndeterminate) {
FILE *F = fopen("file", "r");
if (!F)
return;
@@ -467,10 +476,14 @@ void error_ftell(void) {
rc = ftell(F);
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ if (TestIndeterminate) {
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ ftell(F); // expected-warning {{might be 'indeterminate'}}
+ }
fclose(F);
}
-void error_ftello(void) {
+void error_ftello(int TestIndeterminate) {
FILE *F = fopen("file", "r");
if (!F)
return;
@@ -488,6 +501,10 @@ void error_ftello(void) {
rc = ftello(F);
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ if (TestIndeterminate) {
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ ftell(F); // expected-warning {{might be 'indeterminate'}}
+ }
fclose(F);
}
@@ -506,6 +523,8 @@ void error_fileno(void) {
N = fileno(F);
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ StreamTesterChecker_make_ferror_indeterminate_stream(F);
+ fileno(F); // no warning
fclose(F);
}
More information about the cfe-commits
mailing list