[clang] [clang][analyzer] Add function 'ungetc' to StreamChecker. (PR #77331)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 9 01:03:57 PST 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/77331
>From 9bcc43b5c62ba969f91c495d4d570c5c4337aca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 8 Jan 2024 16:42:58 +0100
Subject: [PATCH 1/2] [clang][analyzer] Add 'ungetc' to StreamChecker.
---
clang/docs/ReleaseNotes.rst | 4 +-
.../Checkers/StdLibraryFunctionsChecker.cpp | 19 ++++++++
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 44 +++++++++++++++++++
.../Analysis/Inputs/system-header-simulator.h | 1 +
clang/test/Analysis/stream-error.c | 16 +++++++
clang/test/Analysis/stream-noopen.c | 25 +++++++++++
clang/test/Analysis/stream.c | 6 +++
7 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
(`c3a87ddad62a <https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c>`_,
`0954dc3fb921 <https://github.com/llvm/llvm-project/commit/0954dc3fb9214b994623f5306473de075f8e3593>`_)
-- Improved the ``alpha.unix.Stream`` checker by modeling more functions like,
- ``fflush``, ``fputs``, ``fgetc``, ``fputc``, ``fopen``, ``fdopen``, ``fgets``, ``tmpfile``.
+- Improved the ``alpha.unix.Stream`` checker by modeling more functions:
+ ``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``.
(`#76776 <https://github.com/llvm/llvm-project/pull/76776>`_,
`#74296 <https://github.com/llvm/llvm-project/pull/74296>`_,
`#73335 <https://github.com/llvm/llvm-project/pull/73335>`_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2201,6 +2201,25 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(0))));
+ // int ungetc(int c, FILE *stream);
+ addToFunctionSummaryMap(
+ "ungetc", Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(BO_EQ, ArgNo(0)),
+ ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
+ ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
+ ArgumentCondition(0, WithinRange, {{EOFv, EOFv}})},
+ ErrnoNEZeroIrrelevant,
+ "Assuming that 'ungetc' fails because EOF was passed as "
+ "character")
+ .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
+ ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
+ ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(ArgumentCondition(
+ 0, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}}))
+ .ArgConstraint(NotNull(ArgNo(1))));
+
// int fseek(FILE *stream, long offset, int whence);
// FIXME: It can be possible to get the 'SEEK_' values (like EOFv) and use
// these for condition of arg 2.
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 25da3c18e8519f..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
{{{"fputs"}, 2},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
+ {{{"ungetc"}, 2},
+ {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+ std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
{{{"fseek"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"ftell"}, 1},
@@ -332,6 +335,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFputx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsSingleChar) const;
+ void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -916,6 +922,44 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ 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);
+
+ // Generate a transition for the success state.
+ std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
+ if (!PutVal)
+ return;
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), *PutVal);
+ StateNotFailed =
+ StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+
+ // Add transition for the failed state.
+ // Failure of 'ungetc' does not result in feof or ferror state.
+ // If the PutVal has value of EofVal the function should "fail", but this is
+ // the same transition as the success state.
+ // FIXME: Is it possible that StateFailed == StateNotFailed ?
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ StreamState NewSS = StreamState::getOpened(Desc);
+ StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
+ C.addTransition(StateFailed);
+}
+
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index 8c43c48c6a3e45..caae59c38a4c8e 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -53,6 +53,7 @@ int fgetc(FILE *stream);
char *fgets(char *restrict str, int count, FILE *restrict stream);
int fputc(int ch, FILE *stream);
int fputs(const char *restrict s, FILE *restrict stream);
+int ungetc(int c, FILE *stream);
int fseek(FILE *__stream, long int __off, int __whence);
long int ftell(FILE *__stream);
void rewind(FILE *__stream);
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 13c6684b5840af..c038348e799d29 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -191,6 +191,22 @@ void error_fputs(void) {
fputs("ABC", F); // expected-warning {{Stream might be already closed}}
}
+void error_ungetc() {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ int Ret = ungetc('X', F);
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ if (Ret == EOF) {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ } else {
+ clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
+ }
+ fputc('Y', F); // no-warning
+ fclose(F);
+ ungetc('A', F); // expected-warning {{Stream might be already closed}}
+}
+
void write_after_eof_is_allowed(void) {
FILE *F = tmpfile();
if (!F)
diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..8ad101ee1e8c13 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -138,6 +138,31 @@ void test_rewind(FILE *F) {
rewind(F);
}
+void test_ungetc(FILE *F) {
+ int Ret = ungetc('X', F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ if (Ret == 'X') {
+ if (errno) {} // expected-warning {{undefined}}
+ } else {
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_ungetc_EOF(FILE *F, int C) {
+ int Ret = ungetc(EOF, F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ Ret = ungetc(C, F);
+ if (Ret == EOF) {
+ clang_analyzer_eval(C == EOF); // expected-warning {{TRUE}}
+ // expected-warning at -1{{FALSE}}
+ }
+}
+
void test_feof(FILE *F) {
errno = 0;
feof(F);
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 060d561c1fe1c2..d8026247697a50 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -39,6 +39,12 @@ void check_fputs(void) {
fclose(fp);
}
+void check_ungetc(void) {
+ FILE *fp = tmpfile();
+ ungetc('A', fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
void check_fseek(void) {
FILE *fp = tmpfile();
fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
>From 3081d968435de1a3ac948153a4fc6dc22c630655 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 9 Jan 2024 10:03:23 +0100
Subject: [PATCH 2/2] updated release notes and code comment
---
clang/docs/ReleaseNotes.rst | 8 +++++---
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3bfcd7997faa19..64b80a96ef8b44 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,9 @@ Improvements
(`c3a87ddad62a <https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c>`_,
`0954dc3fb921 <https://github.com/llvm/llvm-project/commit/0954dc3fb9214b994623f5306473de075f8e3593>`_)
-- Improved the ``alpha.unix.Stream`` checker by modeling more functions:
- ``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``.
+- Improved the ``alpha.unix.Stream`` checker by modeling more functions
+ ``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``
+ and no not recognize alternative ``fopen`` and ``tmpfile`` implementations.
(`#76776 <https://github.com/llvm/llvm-project/pull/76776>`_,
`#74296 <https://github.com/llvm/llvm-project/pull/74296>`_,
`#73335 <https://github.com/llvm/llvm-project/pull/73335>`_,
@@ -1171,7 +1172,8 @@ Improvements
`#71518 <https://github.com/llvm/llvm-project/pull/71518>`_,
`#72016 <https://github.com/llvm/llvm-project/pull/72016>`_,
`#70540 <https://github.com/llvm/llvm-project/pull/70540>`_,
- `#73638 <https://github.com/llvm/llvm-project/pull/73638>`_)
+ `#73638 <https://github.com/llvm/llvm-project/pull/73638>`_,
+ `#77331 <https://github.com/llvm/llvm-project/pull/77331>`_)
- The ``alpha.security.taint.TaintPropagation`` checker no longer propagates
taint on ``strlen`` and ``strnlen`` calls, unless these are marked
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a3d1f34d3d1a9a..35ef953deafeec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -953,7 +953,8 @@ void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
// Failure of 'ungetc' does not result in feof or ferror state.
// If the PutVal has value of EofVal the function should "fail", but this is
// the same transition as the success state.
- // FIXME: Is it possible that StateFailed == StateNotFailed ?
+ // In this case only one state transition is added by the analyzer (the two
+ // new states may be similar).
ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
StreamState NewSS = StreamState::getOpened(Desc);
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
More information about the cfe-commits
mailing list