balazske updated this revision to Diff 354791.
balazske added a comment.
Add checker name to commit message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104925/new/
https://reviews.llvm.org/D104925
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-note.c
Index: clang/test/Analysis/stream-note.c
===================================================================
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -88,3 +88,60 @@
fclose(F); // expected-warning {{Stream pointer might be NULL}}
// expected-note@-1 {{Stream pointer might be NULL}}
}
+
+void check_eof_notes_1() {
+ 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);
+ if (feof(F)) { // expected-note {{Taking true branch}}
+ clearerr(F);
+ fread(Buf, 1, 1, F); // expected-note {{Assuming that stream reaches end-of-file here}}
+ if (feof(F)) { // expected-note {{End-of-file status is discovered here}} expected-note {{Taking true branch}}
+ fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
+ // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
+ }
+ }
+ fclose(F);
+}
+
+void check_eof_notes_2() {
+ 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);
+ if (feof(F)) { // expected-note {{Taking false branch}}
+ fclose(F);
+ return;
+ } else if (ferror(F)) { // expected-note {{Taking false branch}}
+ fclose(F);
+ return;
+ }
+ fread(Buf, 1, 1, F); // expected-note {{Assuming that stream reaches end-of-file here}}
+ if (feof(F)) { // expected-note {{End-of-file status is discovered here}} expected-note {{Taking true branch}}
+ fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
+ // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
+ }
+ fclose(F);
+}
+
+void check_eof_notes_3() {
+ 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;
+ int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming that stream reaches end-of-file here}}
+ if (ferror(F)) { // expected-note {{End-of-file status is discovered here}} expected-note {{Taking false branch}}
+ } else {
+ fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
+ // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
+ }
+ fclose(F);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -25,6 +25,10 @@
using namespace ento;
using namespace std::placeholders;
+//===----------------------------------------------------------------------===//
+// Definition of state data structures.
+//===----------------------------------------------------------------------===//
+
namespace {
struct FnDescription;
@@ -146,6 +150,14 @@
}
};
+} // namespace
+
+//===----------------------------------------------------------------------===//
+// StreamChecker class and utility functions.
+//===----------------------------------------------------------------------===//
+
+namespace {
+
class StreamChecker;
using FnCheck = std::function<void(const StreamChecker *, const FnDescription *,
const CallEvent &, CheckerContext &)>;
@@ -337,7 +349,8 @@
/// There will be always a state transition into the passed State,
/// by the new non-fatal error node or (if failed) a normal transition,
/// to ensure uniform handling.
- void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const;
+ void reportFEofWarning(SymbolRef StreamSym, CheckerContext &C,
+ ProgramStateRef State) const;
/// Emit resource leak warnings for the given symbols.
/// Createn a non-fatal error node for these, and returns it (if any warnings
@@ -389,10 +402,62 @@
CheckerContext &C);
};
+class StreamBugVisitor final : public BugReporterVisitor {
+public:
+ // FIXME: Use mode FOpen to find place of opening a file, instead of NoteTag.
+ enum NotificationMode { FOpen, FEof };
+
+ StreamBugVisitor(SymbolRef S, NotificationMode M, unsigned int BugSeq = 0)
+ : StreamSym{S}, Mode{M}, BugSeq{BugSeq} {}
+
+ static void *getTag() {
+ static int Tag = 0;
+ return &Tag;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(getTag());
+ ID.AddPointer(StreamSym);
+ // FIXME: Other?
+ }
+
+ PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ PathSensitiveBugReport &BR) override;
+
+private:
+ // The stream symbol to be tracked.
+ SymbolRef StreamSym;
+
+ // What kind of diagnostics to emit.
+ NotificationMode Mode;
+
+ // Sequence number of last bug-introducing operation.
+ unsigned int BugSeq;
+};
+
} // 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)
+// Additional info for a stream: A value that is incermented at change of
+// 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)
+
+ProgramStateRef initEofSeq(SymbolRef StreamSym, ProgramStateRef State) {
+ return State->set<EofSeqMap>(StreamSym, 0);
+}
+
+ProgramStateRef incrementEofSeq(SymbolRef StreamSym, ProgramStateRef State) {
+ const unsigned int *Seq = State->get<EofSeqMap>(StreamSym);
+ assert(Seq && "EofSeqMap should be initialized first");
+ return State->set<EofSeqMap>(StreamSym, *Seq + 1);
+}
+
inline void assertStreamStateOpened(const StreamState *SS) {
assert(SS->isOpened() &&
"Previous create of error node for non-opened stream failed?");
@@ -419,6 +484,55 @@
return nullptr;
}
+PathDiagnosticPieceRef StreamBugVisitor::VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ PathSensitiveBugReport &BR) {
+ const ExplodedNode *Pred = N->getFirstPred();
+ if (!Pred)
+ return nullptr;
+
+ const Stmt *S = N->getStmtForDiagnostics();
+ if (!S)
+ return nullptr;
+
+ ProgramStateRef CurrState = N->getState();
+ ProgramStateRef PredState = Pred->getState();
+ const StreamState *SCurr = CurrState->get<StreamMap>(StreamSym);
+ const StreamState *SPred = PredState->get<StreamMap>(StreamSym);
+
+ if (Mode == FEof) {
+ if (!SCurr || !SPred)
+ return nullptr;
+ const unsigned int *CurrEofSeq = CurrState->get<EofSeqMap>(StreamSym);
+ if (!CurrEofSeq)
+ return nullptr;
+ const unsigned int *PredEofSeq = PredState->get<EofSeqMap>(StreamSym);
+ if (!PredEofSeq)
+ return nullptr;
+ if (*CurrEofSeq != BugSeq)
+ return nullptr;
+ if (*CurrEofSeq == *PredEofSeq + 1) {
+ PathDiagnosticLocation L{S, BRC.getSourceManager(),
+ N->getLocationContext()};
+ return std::make_shared<PathDiagnosticEventPiece>(
+ L, "Assuming that stream reaches end-of-file here", true);
+ }
+ if (SCurr->ErrorState.isFEof() && !SPred->ErrorState.isFEof()) {
+ PathDiagnosticLocation L{S, BRC.getSourceManager(),
+ N->getLocationContext()};
+ return std::make_shared<PathDiagnosticEventPiece>(
+ L, "End-of-file status is discovered here", true);
+ }
+ return nullptr;
+ }
+
+ return nullptr;
+}
+
+//===----------------------------------------------------------------------===//
+// Methods of StreamChecker.
+//===----------------------------------------------------------------------===//
+
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
@@ -461,6 +575,7 @@
StateNotNull =
StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
+ StateNotNull = initEofSeq(RetSym, StateNotNull);
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
@@ -566,7 +681,7 @@
if (Sym && State->get<StreamMap>(Sym)) {
const StreamState *SS = State->get<StreamMap>(Sym);
if (SS->ErrorState & ErrorFEof)
- reportFEofWarning(C, State);
+ reportFEofWarning(Sym, C, State);
} else {
C.addTransition(State);
}
@@ -668,6 +783,8 @@
// indicator for the stream is indeterminate.
StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
+ if (IsFread && SS->ErrorState != ErrorFEof)
+ StateFailed = incrementEofSeq(StreamSym, StateFailed);
C.addTransition(StateFailed);
}
@@ -725,6 +842,7 @@
StateFailed = StateFailed->set<StreamMap>(
StreamSym,
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
+ StateFailed = incrementEofSeq(StreamSym, StateFailed);
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@@ -960,14 +1078,18 @@
return State;
}
-void StreamChecker::reportFEofWarning(CheckerContext &C,
+void StreamChecker::reportFEofWarning(SymbolRef StreamSym, CheckerContext &C,
ProgramStateRef State) const {
if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
+ auto R = std::make_unique<PathSensitiveBugReport>(
BT_StreamEof,
"Read function called when stream is in EOF state. "
"Function has no effect.",
- N));
+ N);
+ const unsigned int *EofSeq = State->get<EofSeqMap>(StreamSym);
+ assert(EofSeq && "No EOF sequence number found for stream.");
+ R->addVisitor<StreamBugVisitor>(StreamSym, StreamBugVisitor::FEof, *EofSeq);
+ C.emitReport(std::move(R));
return;
}
C.addTransition(State);
@@ -1058,6 +1180,10 @@
return State;
}
+//===----------------------------------------------------------------------===//
+// Checker registration.
+//===----------------------------------------------------------------------===//
+
void ento::registerStreamChecker(CheckerManager &Mgr) {
Mgr.registerChecker<StreamChecker>();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
