diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -94,15 +94,15 @@ void evalFreopen(const CallEvent &Call, CheckerContext &C) const; void evalFclose(const CallEvent &Call, CheckerContext &C) const; void evalFseek(const CallEvent &Call, CheckerContext &C) const; - void checkArgNullStream(const CallEvent &Call, CheckerContext &C, unsigned ArgI) const; - bool checkNullStream(SVal SV, CheckerContext &C, - ProgramStateRef &State) const; - void checkFseekWhence(SVal SV, CheckerContext &C, - ProgramStateRef &State) const; - bool checkDoubleClose(const CallEvent &Call, CheckerContext &C, - ProgramStateRef &State) const; + + ProgramStateRef checkNullStream(SVal SV, CheckerContext &C, + ProgramStateRef State) const; + ProgramStateRef checkFseekWhence(SVal SV, CheckerContext &C, + ProgramStateRef State) const; + ProgramStateRef checkDoubleClose(const CallEvent &Call, CheckerContext &C, + ProgramStateRef State) const; }; } // end anonymous namespace @@ -135,31 +135,32 @@ } void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const { - ProgramStateRef state = C.getState(); - SValBuilder &svalBuilder = C.getSValBuilder(); + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); if (!CE) return; DefinedSVal RetVal = - svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()) + SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()) .castAs(); - state = state->BindExpr(CE, C.getLocationContext(), RetVal); + SymbolRef RetSym = RetVal.getAsSymbol(); + assert(RetSym && "RetVal must be a symbol here."); + + State = State->BindExpr(CE, C.getLocationContext(), RetVal); - ConstraintManager &CM = C.getConstraintManager(); // Bifurcate the state into two: one with a valid FILE* pointer, the other // with a NULL. - ProgramStateRef stateNotNull, stateNull; - std::tie(stateNotNull, stateNull) = CM.assumeDual(state, RetVal); + ProgramStateRef StateNotNull, StateNull; + std::tie(StateNotNull, StateNull) = C.getConstraintManager().assumeDual(State, RetVal); - SymbolRef Sym = RetVal.getAsSymbol(); - assert(Sym && "RetVal must be a symbol here."); - stateNotNull = stateNotNull->set(Sym, StreamState::getOpened()); - stateNull = stateNull->set(Sym, StreamState::getOpenFailed()); + StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened()); + StateNull = StateNull->set(RetSym, StreamState::getOpenFailed()); - C.addTransition(stateNotNull); - C.addTransition(stateNull); + C.addTransition(StateNotNull); + C.addTransition(StateNull); } void StreamChecker::evalFreopen(const CallEvent &Call, @@ -175,9 +176,8 @@ return; // Do not allow NULL as passed stream pointer. // This is not specified in the man page but may crash on some system. - checkNullStream(*StreamVal, C, State); - // Check if error was generated. - if (C.isDifferent()) + State = checkNullStream(*StreamVal, C, State); + if (!State) return; SymbolRef StreamSym = StreamVal->getAsSymbol(); @@ -207,7 +207,8 @@ void StreamChecker::evalFclose(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - if (checkDoubleClose(Call, C, State)) + State = checkDoubleClose(Call, C, State); + if (State) C.addTransition(State); } @@ -218,32 +219,31 @@ ProgramStateRef State = C.getState(); - bool StateChanged = checkNullStream(Call.getArgSVal(0), C, State); - // Check if error was generated. - if (C.isDifferent()) + State = checkNullStream(Call.getArgSVal(0), C, State); + if (!State) return; - // Check the legality of the 'whence' argument of 'fseek'. - checkFseekWhence(State->getSVal(AE2, C.getLocationContext()), C, State); - - if (!C.isDifferent() && StateChanged) - C.addTransition(State); + State = + checkFseekWhence(State->getSVal(AE2, C.getLocationContext()), C, State); + if (!State) + return; - return; + C.addTransition(State); } void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C, unsigned ArgI) const { ProgramStateRef State = C.getState(); - if (checkNullStream(Call.getArgSVal(ArgI), C, State)) + State = checkNullStream(Call.getArgSVal(ArgI), C, State); + if (State) C.addTransition(State); } -bool StreamChecker::checkNullStream(SVal SV, CheckerContext &C, - ProgramStateRef &State) const { +ProgramStateRef StreamChecker::checkNullStream(SVal SV, CheckerContext &C, + ProgramStateRef State) const { Optional DV = SV.getAs(); if (!DV) - return false; + return State; ConstraintManager &CM = C.getConstraintManager(); ProgramStateRef StateNotNull, StateNull; @@ -257,26 +257,22 @@ C.emitReport(std::make_unique( *BT_nullfp, BT_nullfp->getDescription(), N)); } - return false; - } - - if (StateNotNull) { - State = StateNotNull; - return true; + return nullptr; } - return false; + return StateNotNull; } -void StreamChecker::checkFseekWhence(SVal SV, CheckerContext &C, - ProgramStateRef &State) const { +// Check the legality of the 'whence' argument of 'fseek'. +ProgramStateRef StreamChecker::checkFseekWhence(SVal SV, CheckerContext &C, + ProgramStateRef State) const { Optional CI = SV.getAs(); if (!CI) - return; + return State; int64_t X = CI->getValue().getSExtValue(); if (X >= 0 && X <= 2) - return; + return State; if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { if (!BT_illegalwhence) @@ -286,20 +282,24 @@ "SEEK_SET, SEEK_END, or SEEK_CUR.")); C.emitReport(std::make_unique( *BT_illegalwhence, BT_illegalwhence->getDescription(), N)); + return nullptr; } + + return State; } -bool StreamChecker::checkDoubleClose(const CallEvent &Call, CheckerContext &C, - ProgramStateRef &State) const { +ProgramStateRef StreamChecker::checkDoubleClose(const CallEvent &Call, + CheckerContext &C, + ProgramStateRef State) const { SymbolRef Sym = Call.getArgSVal(0).getAsSymbol(); if (!Sym) - return false; + return State; const StreamState *SS = State->get(Sym); // If the file stream is not tracked, return. if (!SS) - return false; + return State; // Check: Double close a File Descriptor could cause undefined behaviour. // Conforming to man-pages @@ -312,14 +312,16 @@ " closed. Cause undefined behaviour.")); C.emitReport(std::make_unique( *BT_doubleclose, BT_doubleclose->getDescription(), N)); + return nullptr; } - return false; + + return State; } // Close the File Descriptor. State = State->set(Sym, StreamState::getClosed()); - return true; + return State; } void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,