balazske updated this revision to Diff 355594.
balazske added a comment.
Using NoteTag.
Removed the EOF sequence number.
Renamed test functions.
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_feof_after_feof() {
+ 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 {{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_feof_after_no_feof() {
+ 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 {{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_feof_or_no_error() {
+ 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 {{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,8 +25,15 @@
using namespace ento;
using namespace std::placeholders;
+//===----------------------------------------------------------------------===//
+// Definition of state data structures.
+//===----------------------------------------------------------------------===//
+
namespace {
+const char *Desc_StreamEof = "Stream already in EOF";
+const char *Desc_ResourceLeak = "Resource leak";
+
struct FnDescription;
/// State of the stream error flags.
@@ -146,6 +153,14 @@
}
};
+} // namespace
+
+//===----------------------------------------------------------------------===//
+// StreamChecker class and utility functions.
+//===----------------------------------------------------------------------===//
+
+namespace {
+
class StreamChecker;
using FnCheck = std::function<void(const StreamChecker *, const FnDescription *,
const CallEvent &, CheckerContext &)>;
@@ -203,8 +218,8 @@
"Stream handling error"};
BugType BT_IllegalWhence{this, "Illegal whence argument",
"Stream handling error"};
- BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"};
- BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error",
+ BugType BT_StreamEof{this, Desc_StreamEof, "Stream handling error"};
+ BugType BT_ResourceLeak{this, Desc_ResourceLeak, "Stream handling error",
/*SuppressOnSink =*/true};
public:
@@ -337,7 +352,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
@@ -370,6 +386,7 @@
std::string operator()(PathSensitiveBugReport &BR) const {
if (BR.isInteresting(StreamSym) &&
+ BR.getBugType().getDescription() == Desc_ResourceLeak &&
CheckerName == BR.getBugType().getCheckerName())
return Message;
@@ -391,8 +408,38 @@
} // 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)
+namespace {
+
+struct SetEofNoteFn {
+ const CheckerNameRef CheckerName;
+ SymbolRef StreamSym;
+ const ExplodedNode *NotePosition;
+
+ std::string operator()(PathSensitiveBugReport &BR) const {
+ if (!BR.isInteresting(StreamSym) ||
+ BR.getBugType().getDescription() != Desc_StreamEof ||
+ CheckerName != BR.getBugType().getCheckerName())
+ return "";
+
+ const ExplodedNode *N = BR.getErrorNode();
+ while (N && N != NotePosition) {
+ const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym);
+ if (!StreamS || !StreamS->ErrorState.FEof)
+ return "";
+ N = N->getFirstPred();
+ }
+
+ return "Assuming that stream reaches end-of-file here";
+ }
+};
+
+} // namespace
+
inline void assertStreamStateOpened(const StreamState *SS) {
assert(SS->isOpened() &&
"Previous create of error node for non-opened stream failed?");
@@ -419,6 +466,10 @@
return nullptr;
}
+//===----------------------------------------------------------------------===//
+// Methods of StreamChecker.
+//===----------------------------------------------------------------------===//
+
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
@@ -566,7 +617,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,7 +719,12 @@
// indicator for the stream is indeterminate.
StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
- C.addTransition(StateFailed);
+ if (IsFread && SS->ErrorState != ErrorFEof)
+ C.addTransition(StateFailed,
+ C.getNoteTag(SetEofNoteFn{getCheckerName(), StreamSym,
+ C.getPredecessor()}));
+ else
+ C.addTransition(StateFailed);
}
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -727,7 +783,9 @@
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
C.addTransition(StateNotFailed);
- C.addTransition(StateFailed);
+ C.addTransition(StateFailed,
+ C.getNoteTag(SetEofNoteFn{getCheckerName(), StreamSym,
+ C.getPredecessor()}));
}
void StreamChecker::evalClearerr(const FnDescription *Desc,
@@ -960,14 +1018,16 @@
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);
+ R->markInteresting(StreamSym);
+ C.emitReport(std::move(R));
return;
}
C.addTransition(State);
@@ -1058,6 +1118,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
