diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 65bdc4cac3094..0208f94e1b5a2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -174,6 +174,9 @@ using FnCheck = std::function::max(); +const char *FeofNote = "Assuming stream reaches end-of-file here"; +const char *FerrorNote = "Assuming this stream operation fails"; + struct FnDescription { FnCheck PreFn; FnCheck EvalFn; @@ -218,87 +221,6 @@ 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(StreamSym); - if (!SS) - return false; - CE = dyn_cast_or_null(Call.getOriginExpr()); - if (!CE) - return false; - - assertStreamStateOpened(SS); - - return true; - } - - bool isStreamEof() const { return SS->ErrorState == ErrorFEof; } - - NonLoc getZeroVal(const CallEvent &Call) { - return *SVB.makeZeroVal(Call.getResultType()).getAs(); - } - - ProgramStateRef setStreamState(ProgramStateRef State, - const StreamState &NewSS) { - return State->set(StreamSym, NewSS); - } - - ProgramStateRef makeAndBindRetVal(ProgramStateRef State, CheckerContext &C) { - NonLoc RetVal = makeRetVal(C, CE).castAs(); - 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 bindReturnValue(ProgramStateRef State, CheckerContext &C, - SVal Val) { - return State->BindExpr(CE, C.getLocationContext(), Val); - } - - 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(); - if (!Cond) - return nullptr; - return State->assume(*Cond, true); - } - - ConstraintManager::ProgramStatePair - makeRetValAndAssumeDual(ProgramStateRef State, CheckerContext &C) { - DefinedSVal RetVal = makeRetVal(C, CE); - State = State->BindExpr(CE, C.getLocationContext(), RetVal); - return C.getConstraintManager().assumeDual(State, RetVal); - } -}; - class StreamChecker : public Checker { BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"}; @@ -322,11 +244,59 @@ class StreamChecker : public CheckergetBT_StreamEof()) + return ""; + + BR.markNotInteresting(StreamSym); + + return FeofNote; + }); + } + + const NoteTag *constructSetErrorNoteTag(CheckerContext &C, + SymbolRef StreamSym) const { + return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) { + if (!BR.isInteresting(StreamSym) || + &BR.getBugType() != this->getBT_IndeterminatePosition()) + return ""; + + BR.markNotInteresting(StreamSym); + + return FerrorNote; + }); + } + + const NoteTag *constructSetEofOrErrorNoteTag(CheckerContext &C, + SymbolRef StreamSym) const { + return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) { + if (!BR.isInteresting(StreamSym)) + return ""; + + if (&BR.getBugType() == this->getBT_StreamEof()) { + BR.markNotInteresting(StreamSym); + return FeofNote; + } + if (&BR.getBugType() == this->getBT_IndeterminatePosition()) { + BR.markNotInteresting(StreamSym); + return FerrorNote; + } + + return ""; + }); + } + /// If true, evaluate special testing stream functions. bool TestMode = false; - const BugType *getBT_StreamEof() const { return &BT_StreamEof; } - private: CallDescriptionMap FnDescriptions = { {{{"fopen"}, 2}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, @@ -557,8 +527,8 @@ class StreamChecker : public Checker std::string { if (BR.isInteresting(StreamSym) && &BR.getBugType() == &BT_ResourceLeak) @@ -567,19 +537,6 @@ class StreamChecker : public CheckergetBT_StreamEof()) - return ""; - - BR.markNotInteresting(StreamSym); - - return "Assuming stream reaches end-of-file here"; - }); - } - void initMacroValues(CheckerContext &C) const { if (EofVal) return; @@ -607,6 +564,102 @@ class StreamChecker : public Checkerget(StreamSym); + if (!SS) + return false; + NewES = SS->ErrorState; + CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return false; + + assertStreamStateOpened(SS); + + return true; + } + + bool isStreamEof() const { return SS->ErrorState == ErrorFEof; } + + NonLoc getZeroVal(const CallEvent &Call) { + return *SVB.makeZeroVal(Call.getResultType()).getAs(); + } + + ProgramStateRef setStreamState(ProgramStateRef State, + const StreamState &NewSS) { + NewES = NewSS.ErrorState; + return State->set(StreamSym, NewSS); + } + + ProgramStateRef makeAndBindRetVal(ProgramStateRef State, CheckerContext &C) { + NonLoc RetVal = makeRetVal(C, CE).castAs(); + 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 bindReturnValue(ProgramStateRef State, CheckerContext &C, + SVal Val) { + return State->BindExpr(CE, C.getLocationContext(), Val); + } + + 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(); + if (!Cond) + return nullptr; + return State->assume(*Cond, true); + } + + ConstraintManager::ProgramStatePair + makeRetValAndAssumeDual(ProgramStateRef State, CheckerContext &C) { + DefinedSVal RetVal = makeRetVal(C, CE); + State = State->BindExpr(CE, C.getLocationContext(), RetVal); + return C.getConstraintManager().assumeDual(State, RetVal); + } + + const NoteTag *getFailureNoteTag(const StreamChecker *Ch, CheckerContext &C) { + bool SetFeof = NewES.FEof && !SS->ErrorState.FEof; + bool SetFerror = NewES.FError && !SS->ErrorState.FError; + if (SetFeof && !SetFerror) + return Ch->constructSetEofNoteTag(C, StreamSym); + if (!SetFeof && SetFerror) + return Ch->constructSetErrorNoteTag(C, StreamSym); + if (SetFeof && SetFerror) + return Ch->constructSetEofOrErrorNoteTag(C, StreamSym); + return nullptr; + } +}; + } // end anonymous namespace const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, @@ -697,7 +750,7 @@ void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call, StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateNotNull, - constructNoteTag(C, RetSym, "Stream opened here")); + constructLeakNoteTag(C, RetSym, "Stream opened here")); C.addTransition(StateNull); } @@ -755,7 +808,7 @@ void StreamChecker::evalFreopen(const FnDescription *Desc, StateRetNull->set(StreamSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateRetNotNull, - constructNoteTag(C, StreamSym, "Stream reopened here")); + constructLeakNoteTag(C, StreamSym, "Stream reopened here")); C.addTransition(StateRetNull); } @@ -867,10 +920,7 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, // 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); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call, @@ -929,10 +979,7 @@ void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call, 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); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call, @@ -974,7 +1021,7 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call, ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal); StateFailed = E.setStreamState( StateFailed, StreamState::getOpened(Desc, ErrorFError, true)); - C.addTransition(StateFailed); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFprintf(const FnDescription *Desc, @@ -1008,7 +1055,7 @@ void StreamChecker::evalFprintf(const FnDescription *Desc, // position indicator for the stream is indeterminate. StateFailed = E.setStreamState( StateFailed, StreamState::getOpened(Desc, ErrorFError, true)); - C.addTransition(StateFailed); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call, @@ -1058,10 +1105,7 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call, E.isStreamEof() ? ErrorFEof : ErrorNone | 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); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call, @@ -1129,10 +1173,7 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc, 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); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, @@ -1184,7 +1225,7 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call, NewErrS = NewErrS | ErrorFEof; StateFailed = E.setStreamState(StateFailed, StreamState::getOpened(Desc, NewErrS, true)); - C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym)); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFgetpos(const FnDescription *Desc, @@ -1228,7 +1269,7 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc, StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true)); C.addTransition(StateNotFailed); - C.addTransition(StateFailed); + C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call, @@ -1541,18 +1582,22 @@ ProgramStateRef StreamChecker::ensureNoFilePositionIndeterminate( if (!N) return nullptr; - C.emitReport(std::make_unique( - BT_IndeterminatePosition, BugMessage, N)); + auto R = std::make_unique( + BT_IndeterminatePosition, BugMessage, N); + R->markInteresting(Sym); + C.emitReport(std::move(R)); return State->set( Sym, StreamState::getOpened(SS->LastOperation, ErrorFEof, false)); } // Known or unknown error state without FEOF possible. // Stop analysis, report error. - ExplodedNode *N = C.generateErrorNode(State); - if (N) - C.emitReport(std::make_unique( - BT_IndeterminatePosition, BugMessage, N)); + if (ExplodedNode *N = C.generateErrorNode(State)) { + auto R = std::make_unique( + BT_IndeterminatePosition, BugMessage, N); + R->markInteresting(Sym); + C.emitReport(std::move(R)); + } return nullptr; } diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c index abb4784c078aa..f77cd4aa62841 100644 --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -166,3 +166,70 @@ void check_eof_notes_feof_or_no_error(void) { } fclose(F); } + +void check_indeterminate_notes(void) { + FILE *F; + F = fopen("foo1.c", "r"); + if (F == NULL) // expected-note {{Taking false branch}} \ + // expected-note {{'F' is not equal to NULL}} + return; + int R = fgetc(F); // no note + if (R >= 0) { // expected-note {{Taking true branch}} \ + // expected-note {{'R' is >= 0}} + fgetc(F); // expected-note {{Assuming this stream operation fails}} + if (ferror(F)) // expected-note {{Taking true branch}} + fgetc(F); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} \ + // expected-note {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + } + fclose(F); +} + +void check_indeterminate_after_clearerr(void) { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (F == NULL) // expected-note {{Taking false branch}} \ + // expected-note {{'F' is not equal to NULL}} + return; + fread(Buf, 1, 1, F); // expected-note {{Assuming this stream operation fails}} + if (ferror(F)) { // expected-note {{Taking true branch}} + clearerr(F); + fread(Buf, 1, 1, F); // expected-warning {{might be 'indeterminate' after a failed operation}} \ + // expected-note {{might be 'indeterminate' after a failed operation}} + } + fclose(F); +} + +void check_indeterminate_eof(void) { + FILE *F; + char Buf[2]; + F = fopen("foo1.c", "r"); + if (F == NULL) // expected-note {{Taking false branch}} \ + // expected-note {{'F' is not equal to NULL}} \ + // expected-note {{Taking false branch}} \ + // expected-note {{'F' is not equal to NULL}} + return; + fgets(Buf, sizeof(Buf), F); // expected-note {{Assuming this stream operation fails}} \ + // expected-note {{Assuming stream reaches end-of-file here}} + + fgets(Buf, sizeof(Buf), F); // expected-warning {{might be 'indeterminate'}} \ + // expected-note {{might be 'indeterminate'}} \ + // expected-warning {{stream is in EOF state}} \ + // expected-note {{stream is in EOF state}} + fclose(F); +} + +void check_indeterminate_fseek(void) { + FILE *F = fopen("file", "r"); + if (!F) // expected-note {{Taking false branch}} \ + // expected-note {{'F' is non-null}} + return; + int Ret = fseek(F, 1, SEEK_SET); // expected-note {{Assuming this stream operation fails}} + if (Ret) { // expected-note {{Taking true branch}} \ + // expected-note {{'Ret' is not equal to 0}} + char Buf[2]; + fwrite(Buf, 1, 2, F); // expected-warning {{might be 'indeterminate'}} \ + // expected-note {{might be 'indeterminate'}} + } + fclose(F); +}