[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)
Ben Shi via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 23 05:50:55 PST 2023
https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/72627
>From 81973987254f037f1682b7f0cd7a970177051f04 Mon Sep 17 00:00:00 2001
From: Ben Shi <bennshi at tencent.com>
Date: Fri, 17 Nov 2023 17:22:10 +0800
Subject: [PATCH] [clang][analyzer] Support `fgetc` in StreamChecker
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 84 +++++++++++++++----
.../Analysis/Inputs/system-header-simulator.h | 1 +
clang/test/Analysis/stream-error.c | 40 +++++----
clang/test/Analysis/stream.c | 6 ++
4 files changed, 96 insertions(+), 35 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 1d53e59ca067c27..8eca989d7bcdea4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -250,9 +250,12 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
{{{"fwrite"}, 4},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
+ {{{"fgetc"}, 1},
+ {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+ std::bind(&StreamChecker::evalFgetcFputc, _1, _2, _3, _4, true), 0}},
{{{"fputc"}, 2},
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
- &StreamChecker::evalFputc, 1}},
+ std::bind(&StreamChecker::evalFgetcFputc, _1, _2, _3, _4, false), 1}},
{{{"fseek"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"ftell"}, 1},
@@ -314,8 +317,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsFread) const;
- void evalFputc(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const;
+ void evalFgetcFputc(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C, bool IsRead) const;
void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -751,8 +754,9 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
C.addTransition(StateFailed);
}
-void StreamChecker::evalFputc(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C) const {
+void StreamChecker::evalFgetcFputc(const FnDescription *Desc,
+ const CallEvent &Call, CheckerContext &C,
+ bool IsRead) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
if (!StreamSym)
@@ -768,26 +772,70 @@ void StreamChecker::evalFputc(const FnDescription *Desc, const CallEvent &Call,
assertStreamStateOpened(OldSS);
+ // `fgetc` returns the read character on success, otherwise returns EOF.
// `fputc` returns the written character on success, otherwise returns EOF.
- // 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);
+ // Generate a transition for the success state of fputc.
+ if (!IsRead) {
+ 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);
+ }
+ // Generate a transition for the success state of fgetc.
+ // If we know the state to be FEOF at fgetc, do not add a success state.
+ else if (OldSS->ErrorState != ErrorFEof) {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), RetVal);
+ SValBuilder &SVB = C.getSValBuilder();
+ auto &ASTC = C.getASTContext();
+ // The returned 'unsigned char' of `fgetc` is converted to 'int',
+ // so we need to check if it is in range [0, 255].
+ auto CondLow =
+ SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ASTC.IntTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ auto CondHigh =
+ SVB.evalBinOp(State, BO_LE, RetVal,
+ SVB.makeIntVal(SVB.getBasicValueFactory()
+ .getMaxValue(ASTC.UnsignedCharTy)
+ .getLimitedValue(),
+ ASTC.IntTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!CondLow || !CondHigh)
+ return;
+ StateNotFailed = StateNotFailed->assume(*CondLow, true);
+ if (!StateNotFailed)
+ return;
+ StateNotFailed = StateNotFailed->assume(*CondHigh, true);
+ if (!StateNotFailed)
+ return;
+ C.addTransition(StateNotFailed);
+ }
// Add transition for the failed state.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+
// If a (non-EOF) error occurs, the resulting value of the file position
// indicator for the stream is indeterminate.
- ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
- StreamState NewSS = StreamState::getOpened(
- Desc, ErrorFError, /*IsFilePositionIndeterminate*/ true);
+ StreamErrorState NewES;
+ if (IsRead)
+ NewES =
+ OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError;
+ else
+ NewES = ErrorFError;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- C.addTransition(StateFailed);
+ if (IsRead && OldSS->ErrorState != ErrorFEof)
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ else
+ C.addTransition(StateFailed);
}
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index 8924103f5046ea2..fc57e8bdc3d30c3 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -48,6 +48,7 @@ FILE *freopen(const char *restrict pathname, const char *restrict mode, FILE *re
int fclose(FILE *fp);
size_t fread(void *restrict, size_t, size_t, FILE *restrict);
size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
+int fgetc(FILE *stream);
int fputc(int ch, FILE *stream);
int fseek(FILE *__stream, long int __off, int __whence);
long int ftell(FILE *__stream);
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 5ebdc32bb1b92ff..51bb600c6d024e3 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -101,6 +101,29 @@ void error_fwrite(void) {
Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
}
+void error_fgetc(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ int Ret = fgetc(F);
+ if (0 <= Ret && Ret <= 255) {
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ } else {
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+ if (feof(F)) {
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fgetc(F); // expected-warning {{Read function called when stream is in EOF state}}
+ }
+ if (ferror(F)) {
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ fgetc(F); // expected-warning {{might be 'indeterminate'}}
+ }
+ }
+ fclose(F);
+ fgetc(F); // expected-warning {{Stream might be already closed}}
+}
+
void error_fputc(void) {
FILE *F = tmpfile();
if (!F)
@@ -259,23 +282,6 @@ void error_indeterminate_clearerr(void) {
fclose(F);
}
-void error_indeterminate_fputc(void) {
- FILE *F = fopen("file", "r+");
- if (!F)
- return;
- int rc = fseek(F, 0, SEEK_SET);
- if (rc) {
- if (feof(F)) {
- fputc('X', F); // no warning
- } else if (ferror(F)) {
- fputc('C', F); // expected-warning {{might be 'indeterminate'}}
- } else {
- fputc('E', F); // expected-warning {{might be 'indeterminate'}}
- }
- }
- fclose(F);
-}
-
void error_indeterminate_feof1(void) {
FILE *F = fopen("file", "r+");
if (!F)
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 9b6a31738d6e0b8..a7ee9982478bb96 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -14,6 +14,12 @@ void check_fwrite(void) {
fclose(fp);
}
+void check_fgetc(void) {
+ FILE *fp = tmpfile();
+ fgetc(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
void check_fputc(void) {
FILE *fp = tmpfile();
fputc('A', fp); // expected-warning {{Stream pointer might be NULL}}
More information about the cfe-commits
mailing list