https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/74296
>From fbfe3492b66492948c9b0220af38d59345c5a793 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/4] [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 fab80da9cf06bd0fa73dfdca1d4f2ed23ad060ba 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/4] [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 39ba7800a804a9be3b177144e62f4f52f7e6539d 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/4] [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) >From 21ad7d0824bb2dfb5242fa3811a49b74751bf70f Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Thu, 14 Dec 2023 14:19:53 +0800 Subject: [PATCH 4/4] [clang][analyzer] Support `fflush` in the StreamChecker --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 64 +++++++++---------- clang/test/Analysis/stream-error.c | 52 +++++++++------ 2 files changed, 64 insertions(+), 52 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index a0dc643500024e..efc38147f49dc1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -1204,41 +1204,33 @@ 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>(); - SymbolRef StreamSym = StreamVal.getAsSymbol(); - if (!Stream || !StreamSym) + if (!Stream) return; - ProgramStateRef StateNotNull, StateNull; - std::tie(StateNotNull, StateNull) = + ConstraintManager::ProgramStatePair SP = C.getConstraintManager().assumeDual(State, *Stream); - 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); - } - } + if (SP.first) + ensureStreamOpened(StreamVal, C, SP.first); } void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol(); - const StreamState *SS = nullptr; + SVal StreamVal = getStreamArg(Desc, Call); + std::optional<DefinedSVal> Stream = StreamVal.getAs<DefinedSVal>(); + if (!Stream) + return; - // Skip if the input stream's state is unknown. - // FIXME: We should skip non-NULL constant, such as `(FILE *) 0x1234`. - if (StreamSym) { - if (!(SS = State->get<StreamMap>(StreamSym))) - return; - assert((SS->isOpened() || SS->isOpenFailed()) && - "Stream is expected to opened or open-failed"); - } + // Skip if the stream can be both NULL and non-NULL. + ProgramStateRef StateNotNull, StateNull; + std::tie(StateNotNull, StateNull) = + C.getConstraintManager().assumeDual(State, *Stream); + if (StateNotNull && StateNull) + return; + if (StateNotNull && !StateNull) + State = StateNotNull; + else + State = StateNull; const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); if (!CE) @@ -1246,10 +1238,9 @@ void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, // `fflush` returns EOF on failure, otherwise returns 0. ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE); - C.addTransition(StateFailed); + ProgramStateRef StateNotFailed = bindInt(0, State, C, CE); // 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) { @@ -1260,12 +1251,18 @@ void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, } }; - if (SS && SS->isOpened()) { - // We only clear current stream's error state. - ClearError(StreamSym, SS); - C.addTransition(StateNotFailed); + if (StateNotNull && !StateNull) { + // Skip if the input stream's state is unknown, open-failed or closed. + if (SymbolRef StreamSym = StreamVal.getAsSymbol()) { + const StreamState *SS = State->get<StreamMap>(StreamSym); + if (SS && SS->isOpened()) { + ClearError(StreamSym, SS); + C.addTransition(StateNotFailed); + C.addTransition(StateFailed); + } + } } else { - // We clear error states for all streams. + // Clear error states for all streams. const StreamMapTy &Map = StateNotFailed->get<StreamMap>(); for (const auto &I : Map) { SymbolRef Sym = I.first; @@ -1274,6 +1271,7 @@ void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call, ClearError(Sym, &SS); } C.addTransition(StateNotFailed); + C.addTransition(StateFailed); } } diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 1c5ffb19a5a9cd..c696e4bd4c3103 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -299,23 +299,33 @@ void error_fseek_0(void) { fclose(F); } -void error_fflush_0(void) { +void error_fflush_after_fclose(void) { FILE *F = tmpfile(); int Ret; fflush(NULL); // no-warning - if (!F) { - if ((Ret = fflush(F)) != EOF) - clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}} + if (!F) 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}} } -void error_fflush_1(void) { - FILE *F0 = tmpfile(), *F1 = tmpfile(), *F2 = tmpfile(), *F3 = tmpfile(); +void error_fflush_on_open_failed_stream(void) { + FILE *F = tmpfile(); + if (!F) { + fflush(F); // no-warning + return; + } + fclose(F); +} + +void error_fflush_on_unknown_stream(FILE *F) { + fflush(F); // no-warning +} + +void error_fflush_on_non_null_stream_clear_error_states(void) { + FILE *F0 = tmpfile(), *F1 = tmpfile(); // `fflush` clears a non-EOF stream's error state. if (F0) { StreamTesterChecker_make_ferror_stream(F0); @@ -335,22 +345,26 @@ void error_fflush_1(void) { } fclose(F1); } +} + +void error_fflush_on_null_stream_clear_error_states(void) { + FILE *F0 = tmpfile(), *F1 = tmpfile(); // `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 (F0 && F1) { + StreamTesterChecker_make_ferror_stream(F0); + StreamTesterChecker_make_ferror_stream(F1); + StreamTesterChecker_make_feof_stream(F1); 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}} + clang_analyzer_eval(ferror(F0)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F0)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F1)); // expected-warning {{FALSE}} + clang_analyzer_eval(feof(F1)); // expected-warning {{TRUE}} } } - if (F2) - fclose(F2); - if (F3) - fclose(F3); + if (F0) + fclose(F0); + if (F1) + fclose(F1); } void error_indeterminate(void) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits