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.k...@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.k...@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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits