https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/74296
>From 65ce18117f99056aafcf58151b64f4243f4d5e26 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Mon, 4 Dec 2023 15:51:20 +0800 Subject: [PATCH 1/3] [clang][analyzer] Support `fflush` in the StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 16 ++++++++++++++++ .../Analysis/Inputs/system-header-simulator.h | 1 + clang/test/Analysis/stream-error.c | 9 +++++++++ 3 files changed, 26 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 925fc90e355431..2c725c01dc285b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -266,6 +266,7 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}}, + {{{"fflush"}, 1}, {&StreamChecker::preFflush, nullptr, 0}}, {{{"rewind"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}}, {{{"fgetpos"}, 2}, @@ -360,6 +361,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, CheckerContext &C, const StreamErrorState &ErrorKind) const; + void preFflush(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + /// Check that the stream (in StreamVal) is not NULL. /// If it can only be NULL a fatal error is emitted and nullptr returned. /// Otherwise the return value is a new state where the stream is constrained @@ -1191,6 +1195,18 @@ void StreamChecker::evalSetFeofFerror(const FnDescription *Desc, C.addTransition(State); } +void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { + // Skip if the stream is NULL/nullptr, which means flush all streams. + if (!Call.getArgExpr(Desc->StreamArgNo) + ->isNullPointerConstant(C.getASTContext(), + Expr::NPC_ValueDependentIsNotNull)) { + ProgramStateRef State = C.getState(); + if (State = ensureStreamOpened(getStreamArg(Desc, Call), C, State)) + C.addTransition(State); + } +} + ProgramStateRef StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE, CheckerContext &C, diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h index 7089bd8bfc9d98..409a969a0d4cce 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator.h +++ b/clang/test/Analysis/Inputs/system-header-simulator.h @@ -61,6 +61,7 @@ void clearerr(FILE *stream); int feof(FILE *stream); int ferror(FILE *stream); int fileno(FILE *stream); +int fflush(FILE *stream); size_t strlen(const char *); diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index c8332bcbfa8ca7..aa5b6be851773a 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -299,6 +299,15 @@ void error_fseek_0(void) { fclose(F); } +void error_fflush(void) { + FILE *F = tmpfile(); + if (!F) + return; + fclose(F); + fflush(F); // expected-warning {{Stream might be already closed}} + fflush(NULL); // no-warning +} + void error_indeterminate(void) { FILE *F = fopen("file", "r+"); if (!F) >From dcb766b468a2f29df30451f8f196d7a2371fd038 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Wed, 6 Dec 2023 15:47:35 +0800 Subject: [PATCH 2/3] [clang][analyzer] Support `fflush` in the StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 49 ++++++++++++++++--- clang/test/Analysis/stream-error.c | 12 +++-- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 2c725c01dc285b..a368619fd37d22 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -266,7 +266,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}}, {{{"ftell"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}}, - {{{"fflush"}, 1}, {&StreamChecker::preFflush, nullptr, 0}}, + {{{"fflush"}, 1}, + {&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}}, {{{"rewind"}, 1}, {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}}, {{{"fgetpos"}, 2}, @@ -364,6 +365,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, void preFflush(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; + void evalFflush(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + /// Check that the stream (in StreamVal) is not NULL. /// If it can only be NULL a fatal error is emitted and nullptr returned. /// Otherwise the return value is a new state where the stream is constrained @@ -1197,14 +1201,45 @@ void StreamChecker::evalSetFeofFerror(const FnDescription *Desc, void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { - // Skip if the stream is NULL/nullptr, which means flush all streams. - if (!Call.getArgExpr(Desc->StreamArgNo) - ->isNullPointerConstant(C.getASTContext(), - Expr::NPC_ValueDependentIsNotNull)) { - ProgramStateRef State = C.getState(); - if (State = ensureStreamOpened(getStreamArg(Desc, Call), C, State)) + ProgramStateRef State = C.getState(); + SVal StreamVal = getStreamArg(Desc, Call); + std::optional<DefinedSVal> Stream = StreamVal.getAs<DefinedSVal>(); + if (!Stream) + return; + + ConstraintManager::ProgramStatePair SP = + C.getConstraintManager().assumeDual(State, *Stream); + if (State = SP.first) + if (State = ensureStreamOpened(StreamVal, C, State)) C.addTransition(State); +} + +void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // We will check the result even if the input is `NULL`, + // but do nothing if the input state is unknown. + SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); + if (StreamSym) { + const StreamState *OldSS = State->get<StreamMap>(StreamSym); + if (!OldSS) + return; + assertStreamStateOpened(OldSS); } + + const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); + if (!CE) + return; + + // `fflush` returns 0 on success, otherwise returns EOF. + ProgramStateRef StateNotFailed = bindInt(0, State, C, CE); + ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE); + + // This function does not affect the stream state. + // Still we add success and failure state with the appropriate return value. + C.addTransition(StateNotFailed); + C.addTransition(StateFailed); } ProgramStateRef diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index aa5b6be851773a..94787874cf8396 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -301,11 +301,17 @@ void error_fseek_0(void) { void error_fflush(void) { FILE *F = tmpfile(); - if (!F) + int Ret; + fflush(NULL); // no-warning + if (!F) { + if ((Ret = fflush(F)) != EOF) // no-warning + clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}} return; + } + if ((Ret = fflush(F)) != 0) + clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}} fclose(F); - fflush(F); // expected-warning {{Stream might be already closed}} - fflush(NULL); // no-warning + fflush(F); // expected-warning {{Stream might be already closed}} } void error_indeterminate(void) { >From aa4713ba5ebdf6970a0d352f5c2028cc65ca1747 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Thu, 7 Dec 2023 16:30:13 +0800 Subject: [PATCH 3/3] [clang][analyzer] Support `fflush` in the StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 69 ++++++++++++++----- clang/test/Analysis/stream-error.c | 43 +++++++++++- 2 files changed, 93 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index a368619fd37d22..a0dc643500024e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -1204,42 +1204,77 @@ void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call, ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); std::optional<DefinedSVal> Stream = StreamVal.getAs<DefinedSVal>(); - if (!Stream) + SymbolRef StreamSym = StreamVal.getAsSymbol(); + if (!Stream || !StreamSym) return; - ConstraintManager::ProgramStatePair SP = + ProgramStateRef StateNotNull, StateNull; + std::tie(StateNotNull, StateNull) = C.getConstraintManager().assumeDual(State, *Stream); - if (State = SP.first) - if (State = ensureStreamOpened(StreamVal, C, State)) - C.addTransition(State); + if (StateNotNull) { + if (StateNotNull = ensureStreamOpened(StreamVal, C, StateNotNull)) + C.addTransition(StateNotNull); + } else if (StateNull) { + const StreamState *SS = StateNull->get<StreamMap>(StreamSym); + if (!SS || !SS->isOpenFailed()) { + StateNull = StateNull->set<StreamMap>(StreamSym, + StreamState::getOpenFailed(Desc)); + if (StateNull) + C.addTransition(StateNull); + } + } } void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - - // We will check the result even if the input is `NULL`, - // but do nothing if the input state is unknown. SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); + const StreamState *SS = nullptr; + + // Skip if the input stream's state is unknown. + // FIXME: We should skip non-NULL constant, such as `(FILE *) 0x1234`. if (StreamSym) { - const StreamState *OldSS = State->get<StreamMap>(StreamSym); - if (!OldSS) + if (!(SS = State->get<StreamMap>(StreamSym))) return; - assertStreamStateOpened(OldSS); + assert((SS->isOpened() || SS->isOpenFailed()) && + "Stream is expected to opened or open-failed"); } const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); if (!CE) return; - // `fflush` returns 0 on success, otherwise returns EOF. - ProgramStateRef StateNotFailed = bindInt(0, State, C, CE); + // `fflush` returns EOF on failure, otherwise returns 0. ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE); - - // This function does not affect the stream state. - // Still we add success and failure state with the appropriate return value. - C.addTransition(StateNotFailed); C.addTransition(StateFailed); + + // Clear error states if `fflush` returns 0, but retain their EOF flags. + ProgramStateRef StateNotFailed = bindInt(0, State, C, CE); + auto ClearError = [&StateNotFailed, Desc](SymbolRef Sym, + const StreamState *SS) { + if (SS->ErrorState & ErrorFError) { + StreamErrorState NewES = + (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone; + StreamState NewSS = StreamState::getOpened(Desc, NewES, false); + StateNotFailed = StateNotFailed->set<StreamMap>(Sym, NewSS); + } + }; + + if (SS && SS->isOpened()) { + // We only clear current stream's error state. + ClearError(StreamSym, SS); + C.addTransition(StateNotFailed); + } else { + // We clear error states for all streams. + const StreamMapTy &Map = StateNotFailed->get<StreamMap>(); + for (const auto &I : Map) { + SymbolRef Sym = I.first; + const StreamState &SS = I.second; + if (SS.isOpened()) + ClearError(Sym, &SS); + } + C.addTransition(StateNotFailed); + } } ProgramStateRef diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 94787874cf8396..1c5ffb19a5a9cd 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -299,12 +299,12 @@ void error_fseek_0(void) { fclose(F); } -void error_fflush(void) { +void error_fflush_0(void) { FILE *F = tmpfile(); int Ret; fflush(NULL); // no-warning if (!F) { - if ((Ret = fflush(F)) != EOF) // no-warning + if ((Ret = fflush(F)) != EOF) clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}} return; } @@ -314,6 +314,45 @@ void error_fflush(void) { fflush(F); // expected-warning {{Stream might be already closed}} } +void error_fflush_1(void) { + FILE *F0 = tmpfile(), *F1 = tmpfile(), *F2 = tmpfile(), *F3 = tmpfile(); + // `fflush` clears a non-EOF stream's error state. + if (F0) { + StreamTesterChecker_make_ferror_stream(F0); + if (fflush(F0) == 0) { // no-warning + clang_analyzer_eval(ferror(F0)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F0)); // expected-warning {{FALSE}} + } + fclose(F0); + } + // `fflush` clears an EOF stream's error state. + if (F1) { + StreamTesterChecker_make_ferror_stream(F1); + StreamTesterChecker_make_feof_stream(F1); + if (fflush(F1) == 0) { // no-warning + clang_analyzer_eval(ferror(F1)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F1)); // expected-warning {{TRUE}} + } + fclose(F1); + } + // `fflush` clears all stream's error states, while retains their EOF states. + if (F2 && F3) { + StreamTesterChecker_make_ferror_stream(F2); + StreamTesterChecker_make_ferror_stream(F3); + StreamTesterChecker_make_feof_stream(F3); + if (fflush(NULL) == 0) { // no-warning + clang_analyzer_eval(ferror(F2)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F2)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F3)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F3)); // expected-warning {{TRUE}} + } + } + if (F2) + fclose(F2); + if (F3) + fclose(F3); +} + void error_indeterminate(void) { FILE *F = fopen("file", "r+"); if (!F) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits