[clang] [clang][analyzer] Fix StreamChecker `ftell` and `fgetpos` at indeterminate file position. (PR #84191)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 6 08:00:13 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/84191.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+53-30)
- (modified) clang/test/Analysis/stream-error.c (+23-4)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..346d4ae8db2dfe 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),
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),
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),
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),
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);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/84191
More information about the cfe-commits
mailing list