[clang] [clang][analyzer] Simplify code of StreamChecker (NFC). (PR #79312)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 31 01:01:09 PST 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/79312
>From 62dc7fdb2f86c81af501f7f1255a98d97ede303f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 24 Jan 2024 16:28:57 +0100
Subject: [PATCH 1/2] [clang][analyzer] Simplify code of StreamChecker (NFC).
Common parts of some "eval" functions are moved into one function.
The non-common parts of the "eval" functions are passed through
lambda parameters to the new function.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 438 +++++++++---------
1 file changed, 230 insertions(+), 208 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 07727b339d967..95e7e5d9f0912 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -400,6 +400,51 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFflush(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ // Contains function parameters to 'evalRWCommon'.
+ struct EvalRWCommonFn {
+ using InitFn = std::function<ProgramStateRef(SymbolRef, const CallExpr *,
+ ASTContext &, SValBuilder &)>;
+ using GetSuccessStateFn = std::function<ProgramStateRef(
+ SymbolRef, const CallExpr *, ASTContext &, SValBuilder &)>;
+ using GetFailureStateFn =
+ std::function<std::pair<ProgramStateRef, StreamErrorState>(
+ SymbolRef, const CallExpr *, ASTContext &, SValBuilder &)>;
+
+ EvalRWCommonFn(InitFn I, GetSuccessStateFn GSS, GetFailureStateFn GFS)
+ : Init(I), GetSuccessState(GSS), GetFailureState(GFS) {}
+ EvalRWCommonFn(GetSuccessStateFn GSS, GetFailureStateFn GFS)
+ : Init([](SymbolRef, const CallExpr *, ASTContext &, SValBuilder &) {
+ return nullptr;
+ }),
+ GetSuccessState(GSS), GetFailureState(GFS) {}
+
+ // Called at start of the evaluation.
+ // Should return 'nullptr' to continue evaluation, or a program state that
+ // is added and evaluation stops here.
+ // This is used to handle special cases when no success or failure state
+ // is added.
+ InitFn Init;
+ // Create the state for the "success" case of the function and return it.
+ // Can return 'nullptr' to not add a success state.
+ GetSuccessStateFn GetSuccessState;
+ // Create the state for the "failure" case of the function.
+ // Return the state and the new stream error state (combination of possible
+ // errors).
+ GetFailureStateFn GetFailureState;
+ };
+
+ /// Common part of many eval functions in the checker.
+ /// This can be used for simple stream read and write functions.
+ /// This function handles full modeling of such functions, by passing a
+ /// 'GetSuccessState' and 'GetFailureState' function. These functions should
+ /// create the program state for the success and failure cases. 'IsRead'
+ /// indicates if the modeled function is a stream read or write operation.
+ /// If the evaluated function is a read operation and the stream is already
+ /// in EOF, the new error will always be EOF and no success state is added.
+ /// This case is handled here and 'GetFailureState' should not care about it.
+ void evalRWCommon(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C, bool IsRead, EvalRWCommonFn Fn) 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
@@ -726,225 +771,140 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
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;
-
- std::optional<NonLoc> SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
- if (!SizeVal)
- return;
- std::optional<NonLoc> NMembVal = Call.getArgSVal(2).getAs<NonLoc>();
- if (!NMembVal)
- return;
-
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
- return;
-
- assertStreamStateOpened(OldSS);
-
- // 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 || (OldSS->ErrorState != ErrorFEof)) {
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), *NMembVal);
- StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- C.addTransition(StateNotFailed);
- }
-
- // Add transition for the failed state.
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef StateFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto Cond =
- SVB.evalBinOpNN(State, BO_LT, RetVal, *NMembVal, SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateFailed = StateFailed->assume(*Cond, true);
- if (!StateFailed)
- return;
-
- StreamErrorState NewES;
- if (IsFread)
- NewES =
- (OldSS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError;
- else
- NewES = ErrorFError;
- // If a (non-EOF) error occurs, the resulting value of the file position
- // indicator for the stream is indeterminate.
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (IsFread && OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
- else
- C.addTransition(StateFailed);
+ std::optional<NonLoc> SizeVal;
+ std::optional<NonLoc> NMembVal;
+
+ EvalRWCommonFn Fn{
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) -> ProgramStateRef {
+ SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
+ NMembVal = Call.getArgSVal(2).getAs<NonLoc>();
+ if (!SizeVal || !NMembVal)
+ return C.getState();
+
+ // 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 (C.getState()->isNull(*SizeVal).isConstrainedTrue() ||
+ C.getState()->isNull(*NMembVal).isConstrainedTrue())
+ // This is the "size or nmemb is zero" case.
+ // Just return 0, do nothing more (not clear the error flags).
+ return bindInt(0, C.getState(), C, CE);
+ return nullptr;
+ },
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) {
+ return C.getState()->BindExpr(CE, C.getLocationContext(), *NMembVal);
+ },
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) -> std::pair<ProgramStateRef, StreamErrorState> {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef State =
+ C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
+ auto Cond = SVB.evalBinOpNN(State, BO_LT, RetVal, *NMembVal,
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return {nullptr, ErrorNone};
+ State = State->assume(*Cond, true);
+ StreamErrorState NewES =
+ (IsFread ? (ErrorFEof | ErrorFError) : ErrorFError);
+ return {State, NewES};
+ }};
+
+ evalRWCommon(Desc, Call, C, IsFread, Fn);
}
void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool SingleChar) 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);
-
// `fgetc` returns the read character on success, otherwise returns EOF.
// `fgets` returns the read buffer address on success, otherwise returns NULL.
-
- if (OldSS->ErrorState != ErrorFEof) {
- if (SingleChar) {
- // Generate a transition for the success state of `fgetc`.
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- ASTContext &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);
- } else {
- // Generate a transition for the success state of `fgets`.
- std::optional<DefinedSVal> GetBuf =
- Call.getArgSVal(0).getAs<DefinedSVal>();
- if (!GetBuf)
- return;
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), *GetBuf);
- StateNotFailed = StateNotFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc));
- C.addTransition(StateNotFailed);
- }
- }
-
- // Add transition for the failed state.
- ProgramStateRef StateFailed;
- if (SingleChar)
- StateFailed = bindInt(*EofVal, State, C, CE);
- else
- StateFailed =
- State->BindExpr(CE, C.getLocationContext(),
- C.getSValBuilder().makeNullWithType(CE->getType()));
-
- // If a (non-EOF) error occurs, the resulting value of the file position
- // indicator for the stream is indeterminate.
- StreamErrorState NewES =
- OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError;
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
- else
- C.addTransition(StateFailed);
+ EvalRWCommonFn Fn{
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) -> ProgramStateRef {
+ if (SingleChar) {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef State =
+ C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
+ // 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(ACtx.IntTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ auto CondHigh =
+ SVB.evalBinOp(State, BO_LE, RetVal,
+ SVB.makeIntVal(SVB.getBasicValueFactory()
+ .getMaxValue(ACtx.UnsignedCharTy)
+ .getLimitedValue(),
+ ACtx.IntTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!CondLow || !CondHigh)
+ return nullptr;
+ State = State->assume(*CondLow, true);
+ if (!State)
+ return nullptr;
+ return State->assume(*CondHigh, true);
+ } else {
+ // Generate a transition for the success state of `fgets`.
+ std::optional<DefinedSVal> GetBuf =
+ Call.getArgSVal(0).getAs<DefinedSVal>();
+ if (!GetBuf)
+ return nullptr;
+ return C.getState()->BindExpr(CE, C.getLocationContext(), *GetBuf);
+ }
+ },
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) -> std::pair<ProgramStateRef, StreamErrorState> {
+ StreamErrorState NewES = ErrorFEof | ErrorFError;
+ if (SingleChar)
+ return std::make_pair(bindInt(*EofVal, C.getState(), C, CE), NewES);
+ else
+ return std::make_pair(
+ C.getState()->BindExpr(
+ CE, C.getLocationContext(),
+ C.getSValBuilder().makeNullWithType(CE->getType())),
+ NewES);
+ }};
+
+ evalRWCommon(Desc, Call, C, /*IsRead=*/true, Fn);
}
void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsSingleChar) 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);
-
// `fputc` returns the written character on success, otherwise returns EOF.
- // `fputs` returns a non negative value on sucecess, otherwise returns EOF.
-
- if (IsSingleChar) {
- // Generate a transition for the success state of `fputc`.
- 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);
- } else {
- // Generate a transition for the success state of `fputs`.
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto &ASTC = C.getASTContext();
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ASTC.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateNotFailed = StateNotFailed->assume(*Cond, true);
- if (!StateNotFailed)
- return;
- StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- C.addTransition(StateNotFailed);
- }
-
- // Add transition for the failed state. 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, true);
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- C.addTransition(StateFailed);
+ // `fputs` returns a nonnegative value on success, otherwise returns EOF.
+ EvalRWCommonFn Fn{
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) -> ProgramStateRef {
+ if (IsSingleChar) {
+ std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
+ if (!PutVal)
+ return nullptr;
+ return C.getState()->BindExpr(CE, C.getLocationContext(), *PutVal);
+ } else {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef State =
+ C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
+ auto Cond =
+ SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return nullptr;
+ return State->assume(*Cond, true);
+ }
+ },
+ [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
+ SValBuilder &SVB) {
+ return std::make_pair(bindInt(*EofVal, C.getState(), C, CE),
+ ErrorFError);
+ }};
+
+ evalRWCommon(Desc, Call, C, /*IsRead=*/false, Fn);
}
void StreamChecker::evalFprintf(const FnDescription *Desc,
@@ -1510,6 +1470,68 @@ void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+void StreamChecker::evalRWCommon(const FnDescription *Desc,
+ const CallEvent &Call, CheckerContext &C,
+ bool IsRead, EvalRWCommonFn Fn) 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);
+
+ ASTContext &ACtx = C.getASTContext();
+ SValBuilder &SVB = C.getSValBuilder();
+
+ if (ProgramStateRef State = Fn.Init(StreamSym, CE, ACtx, SVB)) {
+ C.addTransition(State);
+ return;
+ }
+
+ // A read operation will aways fail with EOF if the stream is already in EOF
+ // state, no success case is added.
+ if (!IsRead || OldSS->ErrorState != ErrorFEof) {
+ ProgramStateRef StateSuccess = Fn.GetSuccessState(StreamSym, CE, ACtx, SVB);
+ if (StateSuccess) {
+ StateSuccess =
+ StateSuccess->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(StateSuccess);
+ }
+ }
+
+ ProgramStateRef StateFailed;
+ StreamErrorState NewES;
+ std::tie(StateFailed, NewES) = Fn.GetFailureState(StreamSym, CE, ACtx, SVB);
+ if (!StateFailed)
+ return;
+ if (OldSS->ErrorState == ErrorFEof && NewES.FEof) {
+ assert(IsRead && "Stream write function must not produce EOF error.");
+ // If state was EOF before the call and it is possible to get EOF from this
+ // call, create always EOF error.
+ // (Read from an EOF stream results in EOF error, no other error.)
+ StateFailed = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorFEof, false));
+ C.addTransition(StateFailed);
+ } else {
+ // Assume that at any non-EOF read or write error the file position
+ // indicator for the stream becomes indeterminate.
+ StateFailed = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, NewES, NewES.FError));
+ if (OldSS->ErrorState != ErrorFEof && NewES.FEof)
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ else
+ C.addTransition(StateFailed);
+ }
+}
+
ProgramStateRef
StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
CheckerContext &C,
>From 67e14fcee917302a1caba84ac273995c77ef8769 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 31 Jan 2024 09:55:57 +0100
Subject: [PATCH 2/2] Another kind of simplification. `evalFreadFwrite`,
`evalFgetx`, `evalFputx`, `evalFclose` is changed.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 485 ++++++++----------
1 file changed, 225 insertions(+), 260 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 95e7e5d9f0912..354217c1e5292 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -155,6 +155,11 @@ struct StreamState {
} // namespace
+// This map holds the state of a stream.
+// The stream is identified with a SymbolRef that is created when a stream
+// opening function is modeled by the checker.
+REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+
//===----------------------------------------------------------------------===//
// StreamChecker class and utility functions.
//===----------------------------------------------------------------------===//
@@ -208,6 +213,75 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
return State;
}
+inline void assertStreamStateOpened(const StreamState *SS) {
+ assert(SS->isOpened() && "Stream is expected to be opened");
+}
+
+struct StreamOperationEvaluator {
+ SValBuilder &SVB;
+ const ASTContext &ACtx;
+
+ SymbolRef StreamSym;
+ const StreamState *SS = nullptr;
+ const CallExpr *CE = nullptr;
+
+ StreamOperationEvaluator(CheckerContext &C)
+ : SVB(C.getSValBuilder()), ACtx(C.getASTContext()) {
+ ;
+ }
+
+ bool Init(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C,
+ ProgramStateRef State) {
+ StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return false;
+ SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return false;
+ CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return false;
+
+ assertStreamStateOpened(SS);
+
+ return true;
+ }
+
+ bool isStreamEof() const { return SS->ErrorState == ErrorFEof; }
+
+ ProgramStateRef setStreamState(ProgramStateRef State,
+ const StreamState &NewSS) {
+ return State->set<StreamMap>(StreamSym, NewSS);
+ }
+
+ ProgramStateRef makeAndBindRetVal(ProgramStateRef State, CheckerContext &C) {
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ return State->BindExpr(CE, C.getLocationContext(), RetVal);
+ }
+
+ ProgramStateRef bindReturnValue(ProgramStateRef State, CheckerContext &C,
+ uint64_t Val) {
+ return State->BindExpr(CE, C.getLocationContext(),
+ SVB.makeIntVal(Val, CE->getCallReturnType(ACtx)));
+ }
+
+ ProgramStateRef bindNullReturnValue(ProgramStateRef State,
+ CheckerContext &C) {
+ return State->BindExpr(CE, C.getLocationContext(),
+ C.getSValBuilder().makeNullWithType(CE->getType()));
+ }
+
+ ProgramStateRef assumeBinOpNN(ProgramStateRef State,
+ BinaryOperator::Opcode Op, NonLoc LHS,
+ NonLoc RHS) {
+ auto Cond = SVB.evalBinOpNN(State, Op, LHS, RHS, SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return nullptr;
+ return State->assume(*Cond, true);
+ }
+};
+
class StreamChecker : public Checker<check::PreCall, eval::Call,
check::DeadSymbols, check::PointerEscape> {
BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
@@ -400,51 +474,6 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
void evalFflush(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
- // Contains function parameters to 'evalRWCommon'.
- struct EvalRWCommonFn {
- using InitFn = std::function<ProgramStateRef(SymbolRef, const CallExpr *,
- ASTContext &, SValBuilder &)>;
- using GetSuccessStateFn = std::function<ProgramStateRef(
- SymbolRef, const CallExpr *, ASTContext &, SValBuilder &)>;
- using GetFailureStateFn =
- std::function<std::pair<ProgramStateRef, StreamErrorState>(
- SymbolRef, const CallExpr *, ASTContext &, SValBuilder &)>;
-
- EvalRWCommonFn(InitFn I, GetSuccessStateFn GSS, GetFailureStateFn GFS)
- : Init(I), GetSuccessState(GSS), GetFailureState(GFS) {}
- EvalRWCommonFn(GetSuccessStateFn GSS, GetFailureStateFn GFS)
- : Init([](SymbolRef, const CallExpr *, ASTContext &, SValBuilder &) {
- return nullptr;
- }),
- GetSuccessState(GSS), GetFailureState(GFS) {}
-
- // Called at start of the evaluation.
- // Should return 'nullptr' to continue evaluation, or a program state that
- // is added and evaluation stops here.
- // This is used to handle special cases when no success or failure state
- // is added.
- InitFn Init;
- // Create the state for the "success" case of the function and return it.
- // Can return 'nullptr' to not add a success state.
- GetSuccessStateFn GetSuccessState;
- // Create the state for the "failure" case of the function.
- // Return the state and the new stream error state (combination of possible
- // errors).
- GetFailureStateFn GetFailureState;
- };
-
- /// Common part of many eval functions in the checker.
- /// This can be used for simple stream read and write functions.
- /// This function handles full modeling of such functions, by passing a
- /// 'GetSuccessState' and 'GetFailureState' function. These functions should
- /// create the program state for the success and failure cases. 'IsRead'
- /// indicates if the modeled function is a stream read or write operation.
- /// If the evaluated function is a read operation and the stream is already
- /// in EOF, the new error will always be EOF and no success state is added.
- /// This case is handled here and 'GetFailureState' should not care about it.
- void evalRWCommon(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C, bool IsRead, EvalRWCommonFn Fn) 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
@@ -559,15 +588,6 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
} // end anonymous namespace
-// This map holds the state of a stream.
-// The stream is identified with a SymbolRef that is created when a stream
-// opening function is modeled by the checker.
-REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
-
-inline void assertStreamStateOpened(const StreamState *SS) {
- assert(SS->isOpened() && "Stream is expected to be opened");
-}
-
const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
SymbolRef StreamSym,
CheckerContext &C) {
@@ -706,35 +726,18 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
- if (!Sym)
- return;
-
- const StreamState *SS = State->get<StreamMap>(Sym);
- if (!SS)
- return;
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(SS);
-
// Close the File Descriptor.
// Regardless if the close fails or not, stream becomes "closed"
// and can not be used any more.
- State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
+ State = E.setStreamState(State, StreamState::getClosed(Desc));
// Return 0 on success, EOF on failure.
- SValBuilder &SVB = C.getSValBuilder();
- ProgramStateRef StateSuccess = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy));
- ProgramStateRef StateFailure =
- State->BindExpr(CE, C.getLocationContext(),
- SVB.makeIntVal(*EofVal, C.getASTContext().IntTy));
-
- C.addTransition(StateSuccess);
- C.addTransition(StateFailure);
+ C.addTransition(E.bindReturnValue(State, C, 0));
+ C.addTransition(E.bindReturnValue(State, C, *EofVal));
}
void StreamChecker::preReadWrite(const FnDescription *Desc,
@@ -771,140 +774,164 @@ void StreamChecker::preReadWrite(const FnDescription *Desc,
void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
const CallEvent &Call, CheckerContext &C,
bool IsFread) const {
- std::optional<NonLoc> SizeVal;
- std::optional<NonLoc> NMembVal;
-
- EvalRWCommonFn Fn{
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) -> ProgramStateRef {
- SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
- NMembVal = Call.getArgSVal(2).getAs<NonLoc>();
- if (!SizeVal || !NMembVal)
- return C.getState();
-
- // 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 (C.getState()->isNull(*SizeVal).isConstrainedTrue() ||
- C.getState()->isNull(*NMembVal).isConstrainedTrue())
- // This is the "size or nmemb is zero" case.
- // Just return 0, do nothing more (not clear the error flags).
- return bindInt(0, C.getState(), C, CE);
- return nullptr;
- },
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) {
- return C.getState()->BindExpr(CE, C.getLocationContext(), *NMembVal);
- },
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) -> std::pair<ProgramStateRef, StreamErrorState> {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef State =
- C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond = SVB.evalBinOpNN(State, BO_LT, RetVal, *NMembVal,
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return {nullptr, ErrorNone};
- State = State->assume(*Cond, true);
- StreamErrorState NewES =
- (IsFread ? (ErrorFEof | ErrorFError) : ErrorFError);
- return {State, NewES};
- }};
-
- evalRWCommon(Desc, Call, C, IsFread, Fn);
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
+ return;
+
+ std::optional<NonLoc> SizeVal = Call.getArgSVal(1).getAs<NonLoc>();
+ if (!SizeVal)
+ return;
+ std::optional<NonLoc> NMembVal = Call.getArgSVal(2).getAs<NonLoc>();
+ if (!NMembVal)
+ return;
+
+ // 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).
+ C.addTransition(E.bindReturnValue(State, C, 0));
+ 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 || !E.isStreamEof()) {
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), *NMembVal);
+ StateNotFailed =
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+ }
+
+ // Add transition for the failed state.
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
+ ProgramStateRef StateFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateFailed = E.assumeBinOpNN(StateFailed, BO_LT, RetVal, *NMembVal);
+ if (!StateFailed)
+ return;
+
+ StreamErrorState NewES;
+ if (IsFread)
+ NewES = E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
+ else
+ NewES = ErrorFError;
+ // If a (non-EOF) error occurs, the resulting value of the file position
+ // indicator for the stream is indeterminate.
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (IsFread && !E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
+ else
+ C.addTransition(StateFailed);
}
void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool SingleChar) const {
// `fgetc` returns the read character on success, otherwise returns EOF.
// `fgets` returns the read buffer address on success, otherwise returns NULL.
- EvalRWCommonFn Fn{
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) -> ProgramStateRef {
- if (SingleChar) {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef State =
- C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
- // 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(ACtx.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- auto CondHigh =
- SVB.evalBinOp(State, BO_LE, RetVal,
- SVB.makeIntVal(SVB.getBasicValueFactory()
- .getMaxValue(ACtx.UnsignedCharTy)
- .getLimitedValue(),
- ACtx.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!CondLow || !CondHigh)
- return nullptr;
- State = State->assume(*CondLow, true);
- if (!State)
- return nullptr;
- return State->assume(*CondHigh, true);
- } else {
- // Generate a transition for the success state of `fgets`.
- std::optional<DefinedSVal> GetBuf =
- Call.getArgSVal(0).getAs<DefinedSVal>();
- if (!GetBuf)
- return nullptr;
- return C.getState()->BindExpr(CE, C.getLocationContext(), *GetBuf);
- }
- },
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) -> std::pair<ProgramStateRef, StreamErrorState> {
- StreamErrorState NewES = ErrorFEof | ErrorFError;
- if (SingleChar)
- return std::make_pair(bindInt(*EofVal, C.getState(), C, CE), NewES);
- else
- return std::make_pair(
- C.getState()->BindExpr(
- CE, C.getLocationContext(),
- C.getSValBuilder().makeNullWithType(CE->getType())),
- NewES);
- }};
-
- evalRWCommon(Desc, Call, C, /*IsRead=*/true, Fn);
+
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
+ return;
+
+ if (!E.isStreamEof()) {
+ if (SingleChar) {
+ // Generate a transition for the success state of `fgetc`.
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ // The returned 'unsigned char' of `fgetc` is converted to 'int',
+ // so we need to check if it is in range [0, 255].
+ StateNotFailed = StateNotFailed->assumeInclusiveRange(
+ RetVal,
+ E.SVB.getBasicValueFactory().getValue(0, E.ACtx.UnsignedCharTy),
+ E.SVB.getBasicValueFactory().getMaxValue(E.ACtx.UnsignedCharTy),
+ true);
+ if (!StateNotFailed)
+ return;
+ C.addTransition(StateNotFailed);
+ } else {
+ // Generate a transition for the success state of `fgets`.
+ std::optional<DefinedSVal> GetBuf =
+ Call.getArgSVal(0).getAs<DefinedSVal>();
+ if (!GetBuf)
+ return;
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), *GetBuf);
+ StateNotFailed =
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+ }
+ }
+
+ // Add transition for the failed state.
+ ProgramStateRef StateFailed;
+ if (SingleChar)
+ StateFailed = E.bindReturnValue(State, C, *EofVal);
+ else
+ StateFailed = E.bindNullReturnValue(State, C);
+
+ // If a (non-EOF) error occurs, the resulting value of the file position
+ // indicator for the stream is indeterminate.
+ StreamErrorState NewES =
+ E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (!E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
+ else
+ C.addTransition(StateFailed);
}
void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C, bool IsSingleChar) const {
// `fputc` returns the written character on success, otherwise returns EOF.
// `fputs` returns a nonnegative value on success, otherwise returns EOF.
- EvalRWCommonFn Fn{
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) -> ProgramStateRef {
- if (IsSingleChar) {
- std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
- if (!PutVal)
- return nullptr;
- return C.getState()->BindExpr(CE, C.getLocationContext(), *PutVal);
- } else {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- ProgramStateRef State =
- C.getState()->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond =
- SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return nullptr;
- return State->assume(*Cond, true);
- }
- },
- [&](SymbolRef StreamSym, const CallExpr *CE, ASTContext &ACtx,
- SValBuilder &SVB) {
- return std::make_pair(bindInt(*EofVal, C.getState(), C, CE),
- ErrorFError);
- }};
-
- evalRWCommon(Desc, Call, C, /*IsRead=*/false, Fn);
+
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
+ return;
+
+ if (IsSingleChar) {
+ // Generate a transition for the success state of `fputc`.
+ std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
+ if (!PutVal)
+ return;
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), *PutVal);
+ StateNotFailed =
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+ } else {
+ // Generate a transition for the success state of `fputs`.
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed =
+ E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ *E.SVB.makeZeroVal(E.ACtx.IntTy).getAs<NonLoc>());
+ if (!StateNotFailed)
+ return;
+ StateNotFailed =
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+ }
+
+ // Add transition for the failed state. The resulting value of the file
+ // position indicator for the stream is indeterminate.
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, ErrorFError, true));
+ C.addTransition(StateFailed);
}
void StreamChecker::evalFprintf(const FnDescription *Desc,
@@ -1470,68 +1497,6 @@ void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
-void StreamChecker::evalRWCommon(const FnDescription *Desc,
- const CallEvent &Call, CheckerContext &C,
- bool IsRead, EvalRWCommonFn Fn) 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);
-
- ASTContext &ACtx = C.getASTContext();
- SValBuilder &SVB = C.getSValBuilder();
-
- if (ProgramStateRef State = Fn.Init(StreamSym, CE, ACtx, SVB)) {
- C.addTransition(State);
- return;
- }
-
- // A read operation will aways fail with EOF if the stream is already in EOF
- // state, no success case is added.
- if (!IsRead || OldSS->ErrorState != ErrorFEof) {
- ProgramStateRef StateSuccess = Fn.GetSuccessState(StreamSym, CE, ACtx, SVB);
- if (StateSuccess) {
- StateSuccess =
- StateSuccess->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- C.addTransition(StateSuccess);
- }
- }
-
- ProgramStateRef StateFailed;
- StreamErrorState NewES;
- std::tie(StateFailed, NewES) = Fn.GetFailureState(StreamSym, CE, ACtx, SVB);
- if (!StateFailed)
- return;
- if (OldSS->ErrorState == ErrorFEof && NewES.FEof) {
- assert(IsRead && "Stream write function must not produce EOF error.");
- // If state was EOF before the call and it is possible to get EOF from this
- // call, create always EOF error.
- // (Read from an EOF stream results in EOF error, no other error.)
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, ErrorFEof, false));
- C.addTransition(StateFailed);
- } else {
- // Assume that at any non-EOF read or write error the file position
- // indicator for the stream becomes indeterminate.
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, NewES, NewES.FError));
- if (OldSS->ErrorState != ErrorFEof && NewES.FEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
- else
- C.addTransition(StateFailed);
- }
-}
-
ProgramStateRef
StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
CheckerContext &C,
More information about the cfe-commits
mailing list