balazske updated this revision to Diff 357947.
balazske added a comment.
Using the new symbol "uninterestingness" feature.
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,11 +218,18 @@
"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:
+ static const StreamChecker *Instance;
+
+ static const BugType *getBT_StreamEof() { return &Instance->BT_StreamEof; }
+ static const BugType *getBT_ResourceLeak() {
+ return &Instance->BT_ResourceLeak;
+ }
+
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -337,7 +359,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
@@ -364,13 +387,12 @@
/// Generate a message for BugReporterVisitor if the stored symbol is
/// marked as interesting by the actual bug report.
struct NoteFn {
- const CheckerNameRef CheckerName;
SymbolRef StreamSym;
std::string Message;
std::string operator()(PathSensitiveBugReport &BR) const {
if (BR.isInteresting(StreamSym) &&
- CheckerName == BR.getBugType().getCheckerName())
+ &BR.getBugType() == StreamChecker::getBT_ResourceLeak())
return Message;
return "";
@@ -379,7 +401,20 @@
const NoteTag *constructNoteTag(CheckerContext &C, SymbolRef StreamSym,
const std::string &Message) const {
- return C.getNoteTag(NoteFn{getCheckerName(), StreamSym, Message});
+ return C.getNoteTag(NoteFn{StreamSym, Message});
+ }
+
+ const NoteTag *constructSetEofNoteTag(CheckerContext &C,
+ SymbolRef StreamSym) const {
+ return C.getNoteTag([StreamSym](PathSensitiveBugReport &BR) {
+ if (!BR.isInteresting(StreamSym) ||
+ &BR.getBugType() != StreamChecker::getBT_StreamEof())
+ return "";
+
+ BR.markNotInteresting(StreamSym);
+
+ return "Assuming that stream reaches end-of-file here";
+ });
}
/// Searches for the ExplodedNode where the file descriptor was acquired for
@@ -389,8 +424,13 @@
CheckerContext &C);
};
+const StreamChecker *StreamChecker::Instance;
+
} // 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)
inline void assertStreamStateOpened(const StreamState *SS) {
@@ -419,6 +459,10 @@
return nullptr;
}
+//===----------------------------------------------------------------------===//
+// Methods of StreamChecker.
+//===----------------------------------------------------------------------===//
+
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
@@ -566,7 +610,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 +712,10 @@
// 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, constructSetEofNoteTag(C, StreamSym));
+ else
+ C.addTransition(StateFailed);
}
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -727,7 +774,7 @@
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
C.addTransition(StateNotFailed);
- C.addTransition(StateFailed);
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
}
void StreamChecker::evalClearerr(const FnDescription *Desc,
@@ -960,14 +1007,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,8 +1107,13 @@
return State;
}
+//===----------------------------------------------------------------------===//
+// Checker registration.
+//===----------------------------------------------------------------------===//
+
void ento::registerStreamChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<StreamChecker>();
+ auto *Checker = Mgr.registerChecker<StreamChecker>();
+ StreamChecker::Instance = Checker;
}
bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) {
@@ -1073,4 +1127,4 @@
bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) {
return true;
-}
+}
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
