[clang] [clang][analyzer] Add function 'fscanf' to StreamChecker. (PR #78180)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 02:28:22 PST 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/78180
>From aacfc3f06ee51ede08464cb23ec32b210e703b6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 15 Jan 2024 16:41:34 +0100
Subject: [PATCH 1/2] [clang][analyzer] Add function 'fscanf' to StreamChecker.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 61 +++++++++++++++++++
clang/test/Analysis/stream-error.c | 25 ++++++++
clang/test/Analysis/stream.c | 6 ++
3 files changed, 92 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 95c7503e49e0d3..ab23a428e9397f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
{{{"fprintf"}},
{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),
+ std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
{{{"ungetc"}, 2},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
@@ -345,6 +348,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFprintf(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFscanf(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -975,6 +981,61 @@ void StreamChecker::evalFprintf(const FnDescription *Desc,
C.addTransition(StateFailed);
}
+void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (Call.getNumArgs() < 2)
+ return;
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ const StreamState *OldSS = State->get<StreamMap>(StreamSym);
+ if (!OldSS)
+ return;
+
+ assertStreamStateOpened(OldSS);
+
+ SValBuilder &SVB = C.getSValBuilder();
+ ASTContext &ACtx = C.getASTContext();
+
+ if (OldSS->ErrorState != ErrorFEof) {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), RetVal);
+ auto RetGeZero =
+ SVB.evalBinOp(StateNotFailed, BO_GE, RetVal,
+ SVB.makeZeroVal(ACtx.IntTy), SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!RetGeZero)
+ return;
+ StateNotFailed = StateNotFailed->assume(*RetGeZero, true);
+
+ C.addTransition(StateNotFailed);
+ }
+
+ // Add transition for the failed state.
+ // Error occurs if nothing is matched yet and reading the input fails.
+ // Error can be EOF, or other error. At "other error" FERROR or 'errno' can
+ // be set but it is not further specified if all are required to be set.
+ // Documentation does not mention, but file position will be set to
+ // indeterminate similarly as at 'fread'.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ StreamErrorState NewES = (OldSS->ErrorState == ErrorFEof)
+ ? ErrorFEof
+ : ErrorNone | ErrorFEof | ErrorFError;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
+ StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
+ if (OldSS->ErrorState != ErrorFEof)
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ else
+ C.addTransition(StateFailed);
+}
+
void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 0f7fdddc0dd4cd..2cf46e1d4ad51f 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -208,6 +208,31 @@ void error_fprintf(void) {
fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}}
}
+void error_fscanf(int *A) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ int Ret = fscanf(F, "a%ib", A);
+ if (Ret >= 0) {
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ fscanf(F, "bbb"); // no-warning
+ } else {
+ if (ferror(F)) {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ fscanf(F, "bbb"); // expected-warning {{might be 'indeterminate'}}
+ } else if (feof(F)) {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ fscanf(F, "bbb"); // expected-warning {{is in EOF state}}
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ } else {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ fscanf(F, "bbb"); // expected-warning {{might be 'indeterminate'}}
+ }
+ }
+ fclose(F);
+ fscanf(F, "ccc"); // expected-warning {{Stream might be already closed}}
+}
+
void error_ungetc() {
FILE *F = tmpfile();
if (!F)
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index e8f06922bdb2f3..36a9b4e26b07a2 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -45,6 +45,12 @@ void check_fprintf(void) {
fclose(fp);
}
+void check_fscanf(void) {
+ FILE *fp = tmpfile();
+ fscanf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
void check_ungetc(void) {
FILE *fp = tmpfile();
ungetc('A', fp); // expected-warning {{Stream pointer might be NULL}}
>From 128b59ebe046994f38f2e488ce44e891c0b1b0d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 16 Jan 2024 11:27:44 +0100
Subject: [PATCH 2/2] added clarifying comment
---
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index ab23a428e9397f..84923732fe1b28 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1003,6 +1003,14 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
SValBuilder &SVB = C.getSValBuilder();
ASTContext &ACtx = C.getASTContext();
+ // Add the success state.
+ // In this context "success" means there is not an EOF or other read error
+ // before any item is matched in 'fscanf'. But there may be match failure,
+ // therefore return value can be 0 or greater.
+ // It is not specified what happens if some items (not all) are matched and
+ // then EOF or read error happens. Now this case is handled like a "success"
+ // case, and no error flags are set on the stream. This is probably not
+ // accurate, and the POSIX documentation does not tell more.
if (OldSS->ErrorState != ErrorFEof) {
NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
More information about the cfe-commits
mailing list