[clang] f7c9f77 - [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Wed May 20 00:40:28 PDT 2020
Author: Balázs Kéri
Date: 2020-05-20T09:40:57+02:00
New Revision: f7c9f77ef3721c956499b0ccf5e585de105aae4e
URL: https://github.com/llvm/llvm-project/commit/f7c9f77ef3721c956499b0ccf5e585de105aae4e
DIFF: https://github.com/llvm/llvm-project/commit/f7c9f77ef3721c956499b0ccf5e585de105aae4e.diff
LOG: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.
Summary:
Stream functions `fread` and `fwrite` are evaluated
and preconditions checked.
A new bug type is added for a (non fatal) warning if `fread`
is called in EOF state.
Reviewers: Szelethus, NoQ, dcoughlin, baloghadamsoftware, martong, xazax.hun
Reviewed By: Szelethus
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80015
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index c94cae045239..2e90be4350a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -51,6 +51,8 @@ struct StreamErrorState {
return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
}
+ bool operator!=(const StreamErrorState &ES) const { return !(*this == ES); }
+
StreamErrorState operator|(const StreamErrorState &E) const {
return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
}
@@ -171,7 +173,7 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
- BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak;
+ BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
public:
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -189,8 +191,12 @@ class StreamChecker
{{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
{{"fclose", 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
- {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
- {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+ {{"fread", 4},
+ {&StreamChecker::preFread,
+ std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
+ {{"fwrite", 4},
+ {&StreamChecker::preFwrite,
+ std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
{{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
{{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
@@ -232,6 +238,15 @@ class StreamChecker
void evalFclose(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void preFread(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void preFwrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C, bool IsFread) const;
+
void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -271,6 +286,12 @@ class StreamChecker
ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
ProgramStateRef State) const;
+ /// Generate warning about stream in EOF state.
+ /// There will be always a state transition into the passed State,
+ /// by the new non-fatal error node or (if failed) a normal transition,
+ /// to ensure uniform handling.
+ void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const;
+
/// Find the description data of the function called by a call event.
/// Returns nullptr if no function is recognized.
const FnDescription *lookupFn(const CallEvent &Call) const {
@@ -418,6 +439,120 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(State);
}
+void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ State = ensureStreamNonNull(StreamVal, C, State);
+ if (!State)
+ return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+ return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get<StreamMap>(Sym)) {
+ const StreamState *SS = State->get<StreamMap>(Sym);
+ if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(C, State);
+ } else {
+ C.addTransition(State);
+ }
+}
+
+void StreamChecker::preFwrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ State = ensureStreamNonNull(StreamVal, C, State);
+ if (!State)
+ return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+ return;
+
+ C.addTransition(State);
+}
+
+void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
+ const CallEvent &Call, CheckerContext &C,
+ bool IsFread) 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;
+
+ Optional<NonLoc> SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
+ if (!SizeVal)
+ return;
+ Optional<NonLoc> NMembVal = Call.getArgSVal(2).getAs<NonLoc>();
+ if (!NMembVal)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ // C'99 standard, §7.19.8.1.3, the return value of fread:
+ // The fread function returns the number of elements successfully read, which
+ // may be less than nmemb if a read error or end-of-file is encountered. If
+ // size or nmemb is zero, fread returns zero and the contents of the array and
+ // the state of the stream remain unchanged.
+
+ if (State->isNull(*SizeVal).isConstrainedTrue() ||
+ State->isNull(*NMembVal).isConstrainedTrue()) {
+ // This is the "size or nmemb is zero" case.
+ // Just return 0, do nothing more (not clear the error flags).
+ State = bindInt(0, State, C, CE);
+ C.addTransition(State);
+ return;
+ }
+
+ // Generate a transition for the success state.
+ // If we know the state to be FEOF at fread, do not add a success state.
+ if (!IsFread || (SS->ErrorState != ErrorFEof)) {
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), *NMembVal);
+ if (StateNotFailed) {
+ StateNotFailed = StateNotFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+ }
+ }
+
+ // Add transition for the failed state.
+ Optional<NonLoc> RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ assert(RetVal && "Value should be NonLoc.");
+ ProgramStateRef StateFailed =
+ State->BindExpr(CE, C.getLocationContext(), *RetVal);
+ if (!StateFailed)
+ return;
+ auto Cond = C.getSValBuilder()
+ .evalBinOpNN(State, BO_LT, *RetVal, *NMembVal,
+ C.getASTContext().IntTy)
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return;
+ StateFailed = StateFailed->assume(*Cond, true);
+ if (!StateFailed)
+ return;
+
+ StreamErrorState NewES;
+ if (IsFread)
+ NewES = (SS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError;
+ else
+ NewES = ErrorFError;
+ StreamState NewState = StreamState::getOpened(Desc, NewES);
+ StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
+ C.addTransition(StateFailed);
+}
+
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -657,6 +792,21 @@ StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
return State;
}
+void StreamChecker::reportFEofWarning(CheckerContext &C,
+ ProgramStateRef State) const {
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
+ if (!BT_StreamEof)
+ BT_StreamEof.reset(
+ new BuiltinBug(this, "Stream already in EOF",
+ "Read function called when stream is in EOF state. "
+ "Function has no effect."));
+ C.emitReport(std::make_unique<PathSensitiveBugReport>(
+ *BT_StreamEof, BT_StreamEof->getDescription(), N));
+ return;
+ }
+ C.addTransition(State);
+}
+
void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 2302f3efc2f1..cc0147deafdf 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -7,6 +7,7 @@
#include "Inputs/system-header-simulator.h"
void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
void StreamTesterChecker_make_feof_stream(FILE *);
void StreamTesterChecker_make_ferror_stream(FILE *);
@@ -57,6 +58,84 @@ void stream_error_ferror() {
fclose(F);
}
+void error_fread() {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ char Buf[10];
+ int Ret = fread(Buf, 1, 10, F);
+ if (Ret == 10) {
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ } else {
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+ if (feof(F)) {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ fread(Buf, 1, 10, F); // expected-warning {{Read function called when stream is in EOF state}}
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ }
+ if (ferror(F)) {
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ fread(Buf, 1, 10, F); // no warning
+ }
+ }
+ fclose(F);
+ Ret = fread(Buf, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void error_fwrite() {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ const char *Buf = "123456789";
+ int Ret = fwrite(Buf, 1, 10, F);
+ if (Ret == 10) {
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ } else {
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ fwrite(0, 1, 10, F); // no warning
+ }
+ fclose(F);
+ Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void freadwrite_zerosize(FILE *F) {
+ fwrite(0, 1, 0, F);
+ fwrite(0, 0, 1, F);
+ fread(0, 1, 0, F);
+ fread(0, 0, 1, F);
+}
+
+void freadwrite_zerosize_eofstate(FILE *F) {
+ fwrite(0, 1, 0, F);
+ fwrite(0, 0, 1, F);
+ fread(0, 1, 0, F); // expected-warning {{Read function called when stream is in EOF state}}
+ fread(0, 0, 1, F); // expected-warning {{Read function called when stream is in EOF state}}
+}
+
+void error_fread_fwrite_zerosize() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+
+ freadwrite_zerosize(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+
+ StreamTesterChecker_make_ferror_stream(F);
+ freadwrite_zerosize(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+
+ StreamTesterChecker_make_feof_stream(F);
+ freadwrite_zerosize_eofstate(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+
+ fclose(F);
+}
+
void error_fseek() {
FILE *F = fopen("file", "r");
if (!F)
More information about the cfe-commits
mailing list