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 @@ -405,7 +405,7 @@ class StreamBugVisitor final : public BugReporterVisitor { public: // FIXME: Use mode FOpen to find place of opening a file, instead of NoteTag. - enum NotificationMode { FOpen, FEof }; + enum NotificationMode { FOpen, FEof, Indeterminate }; StreamBugVisitor(SymbolRef S, NotificationMode M, unsigned int BugSeq = 0) : StreamSym{S}, Mode{M}, BugSeq{BugSeq} {} @@ -447,17 +447,27 @@ // FEOF flag from false to true. This way it is possible to find the last // place where the stream becomes into EOF state. REGISTER_MAP_WITH_PROGRAMSTATE(EofSeqMap, SymbolRef, unsigned int) +// Simillar for the "FilePositionIndeterminate" value. +REGISTER_MAP_WITH_PROGRAMSTATE(IndeterminateSeqMap, SymbolRef, unsigned int) -ProgramStateRef initEofSeq(SymbolRef StreamSym, ProgramStateRef State) { - return State->set(StreamSym, 0); +ProgramStateRef initSeq(SymbolRef StreamSym, ProgramStateRef State) { + State = State->set(StreamSym, 0); + return State->set(StreamSym, 0); } ProgramStateRef incrementEofSeq(SymbolRef StreamSym, ProgramStateRef State) { const unsigned int *Seq = State->get(StreamSym); - assert(Seq && "EofSeqMap should be initialized first"); + assert(Seq && "Map should be initialized first"); return State->set(StreamSym, *Seq + 1); } +ProgramStateRef incrementIndeterminateSeq(SymbolRef StreamSym, + ProgramStateRef State) { + const unsigned int *Seq = State->get(StreamSym); + assert(Seq && "Map should be initialized first"); + return State->set(StreamSym, *Seq + 1); +} + inline void assertStreamStateOpened(const StreamState *SS) { assert(SS->isOpened() && "Previous create of error node for non-opened stream failed?"); @@ -526,6 +536,28 @@ return nullptr; } + if (Mode == Indeterminate) { + if (!SCurr || !SPred) + return nullptr; + const unsigned int *CurrIndeterminateSeq = + CurrState->get(StreamSym); + if (!CurrIndeterminateSeq) + return nullptr; + const unsigned int *PredIndeterminateSeq = + PredState->get(StreamSym); + if (!PredIndeterminateSeq) + return nullptr; + if (*CurrIndeterminateSeq != BugSeq) + return nullptr; + if (*CurrIndeterminateSeq == *PredIndeterminateSeq + 1) { + PathDiagnosticLocation L{S, BRC.getSourceManager(), + N->getLocationContext()}; + return std::make_shared( + L, "Assuming that this stream operation fails", true); + } + return nullptr; + } + return nullptr; } @@ -575,7 +607,7 @@ StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened(Desc)); - StateNotNull = initEofSeq(RetSym, StateNotNull); + StateNotNull = initSeq(RetSym, StateNotNull); StateNull = StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); @@ -785,6 +817,8 @@ StateFailed = StateFailed->set(StreamSym, NewState); if (IsFread && SS->ErrorState != ErrorFEof) StateFailed = incrementEofSeq(StreamSym, StateFailed); + if (!NewES.isFEof()) + StateFailed = incrementIndeterminateSeq(StreamSym, StateFailed); C.addTransition(StateFailed); } @@ -843,6 +877,7 @@ StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true)); StateFailed = incrementEofSeq(StreamSym, StateFailed); + StateFailed = incrementIndeterminateSeq(StreamSym, StateFailed); C.addTransition(StateNotFailed); C.addTransition(StateFailed); @@ -1028,6 +1063,8 @@ assert(SS->isOpened() && "First ensure that stream is opened."); if (SS->FilePositionIndeterminate) { + const unsigned int *IndeterminateSeq = State->get(Sym); + assert(IndeterminateSeq && "No failure sequence number found for stream."); if (SS->ErrorState & ErrorFEof) { // The error is unknown but may be FEOF. // Continue analysis with the FEOF error state. @@ -1036,8 +1073,11 @@ if (!N) return nullptr; - C.emitReport(std::make_unique( - BT_IndeterminatePosition, BugMessage, N)); + auto R = std::make_unique( + BT_IndeterminatePosition, BugMessage, N); + R->addVisitor(Sym, StreamBugVisitor::Indeterminate, + *IndeterminateSeq); + C.emitReport(std::move(R)); return State->set( Sym, StreamState::getOpened(SS->LastOperation, ErrorFEof, false)); } @@ -1045,9 +1085,13 @@ // 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 (N) { + auto R = std::make_unique( + BT_IndeterminatePosition, BugMessage, N); + R->addVisitor(Sym, StreamBugVisitor::Indeterminate, + *IndeterminateSeq); + C.emitReport(std::move(R)); + } return nullptr; } diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -145,3 +145,62 @@ } fclose(F); } + +void check_indeterminate_notes_1() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}} + return; + fread(Buf, 1, 1, F); + if (ferror(F)) { // expected-note {{Taking true branch}} + F = freopen(0, "w", F); + if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}} + return; + fread(Buf, 1, 1, F); // expected-note {{Assuming that this stream operation fails}} + if (ferror(F)) { // expected-note {{Taking true branch}} + fread(Buf, 1, 1, F); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + // expected-note@-1 {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + } + } + fclose(F); +} + +void check_indeterminate_notes_2() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}} + return; + fseek(F, 1, SEEK_SET); // expected-note {{Assuming that this stream operation fails}} + if (!feof(F)) // expected-note {{Taking true branch}} + fread(Buf, 1, 1, F); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + // expected-note@-1 {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + fclose(F); +} + +void check_indeterminate_notes_3() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}} + return; + fwrite(Buf, 1, 1, F); // expected-note {{Assuming that this stream operation fails}} + if (ferror(F)) // expected-note {{Taking true branch}} + fread(Buf, 1, 1, F); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + // expected-note@-1 {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + fclose(F); +} + +void check_indeterminate_notes_4() { + FILE *F; + char Buf[10]; + F = fopen("foo1.c", "r"); + if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}} + return; + fseek(F, 1, SEEK_SET); // expected-note {{Assuming that this stream operation fails}} + if (!ferror(F) && !feof(F)) // expected-note {{Taking true branch}} // expected-note {{Left side of '&&' is true}} + fread(Buf, 1, 1, F); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + // expected-note@-1 {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} + fclose(F); +}