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
@@ -19,6 +19,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/DenseMap.h"
#include <functional>
using namespace clang;
@@ -231,7 +232,20 @@
/// If true, evaluate special testing stream functions.
bool TestMode = false;
- const BugType *getBT_StreamEof() const { return &BT_StreamEof; }
+ using BugMessageMap = llvm::DenseMap<const BugType *, const char *>;
+ /// At any stream operation that can cause (multiple type of) bugs, we can
+ /// determine the failure reason text by only knowing the bug type. The same
+ /// text is applicable at every operation that may cause that bug. This map
+ /// is used to lookup the message text in a note tag that is added at the
+ /// failing operation.
+ const BugMessageMap BugMessages = {
+ {&BT_FileNull, "Assuming opening the stream fails here"},
+ {&BT_UseAfterClose, "Stream closed here"},
+ {&BT_UseAfterOpenFailed, "Assuming opening the stream fails here"},
+ {&BT_IndeterminatePosition, "Assuming this stream operation fails and "
+ "leaves the file position indeterminate"},
+ {&BT_StreamEof, "Assuming stream reaches end-of-file here"},
+ {&BT_ResourceLeak, "Stream opened here"}};
private:
CallDescriptionMap<FnDescription> FnDescriptions = {
@@ -256,11 +270,13 @@
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
{{"feof", 1},
{&StreamChecker::preDefault,
- std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
+ std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof,
+ FEofNoteMessages),
0}},
{{"ferror", 1},
{&StreamChecker::preDefault,
- std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
+ std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError,
+ FErrorNoteMessages),
0}},
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
@@ -277,6 +293,15 @@
0}},
};
+ const char *FEofNoteMessages[2] = {
+ "The end-of-file flag is set on the stream",
+ "The end-of-file flag is not set on the stream",
+ };
+ const char *FErrorNoteMessages[2] = {
+ "The error flag is set on the stream",
+ "The error flag is not set on the stream",
+ };
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -309,8 +334,8 @@
CheckerContext &C) const;
void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
- CheckerContext &C,
- const StreamErrorState &ErrorKind) const;
+ CheckerContext &C, const StreamErrorState &ErrorKind,
+ const char *NoteMessages[2]) const;
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C,
@@ -376,37 +401,41 @@
return FnDescriptions.lookup(Call);
}
- /// Generate a message for BugReporterVisitor if the stored symbol is
- /// marked as interesting by the actual bug report.
- // FIXME: Use lambda instead.
- struct NoteFn {
- const BugType *BT_ResourceLeak;
- SymbolRef StreamSym;
- std::string Message;
+ /// Create a NoteTag to display a note if a later bug report is generated.
+ /// A NoteTag is added at every stream operation that fails in some way or
+ /// causes a later failure (bug). Successful opening a stream is a "failure"
+ /// in this sense if a resource leak is detected later.
+ /// At a bug report the last operation in the path that has added this kind of
+ /// NoteTag is the one that caused the bug. It is enough to know the bug type
+ /// to determine the note tag text.
+ const NoteTag *constructFailureNoteTag(CheckerContext &C,
+ SymbolRef StreamSym) const {
- std::string operator()(PathSensitiveBugReport &BR) const {
- if (BR.isInteresting(StreamSym) && &BR.getBugType() == BT_ResourceLeak)
- return Message;
+ return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) {
+ if (!BR.isInteresting(StreamSym))
+ return "";
- return "";
- }
- };
+ // This is done to make the report only at the last location with the same
+ // note tag.
+ BR.markNotInteresting(StreamSym);
- const NoteTag *constructNoteTag(CheckerContext &C, SymbolRef StreamSym,
- const std::string &Message) const {
- return C.getNoteTag(NoteFn{&BT_ResourceLeak, StreamSym, Message});
+ return this->BugMessages.lookup(&BR.getBugType());
+ });
}
- const NoteTag *constructSetEofNoteTag(CheckerContext &C,
- SymbolRef StreamSym) const {
- return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) {
- if (!BR.isInteresting(StreamSym) ||
- &BR.getBugType() != this->getBT_StreamEof())
- return "";
+ /// Construct a NoteTag to display a message if any bug is detected later on
+ /// the path (if no other failing operation follows).
+ /// This note is inserted into places where something important about
+ /// the last failing operation (that can be reason of a bug) is discovered.
+ const NoteTag *constructNonFailureNoteTag(CheckerContext &C,
+ SymbolRef StreamSym,
+ const char *Message) const {
- BR.markNotInteresting(StreamSym);
+ return C.getNoteTag([StreamSym, Message](PathSensitiveBugReport &BR) {
+ if (!BR.isInteresting(StreamSym))
+ return "";
- return "Assuming stream reaches end-of-file here";
+ return Message;
});
}
@@ -499,9 +528,8 @@
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
- C.addTransition(StateNotNull,
- constructNoteTag(C, RetSym, "Stream opened here"));
- C.addTransition(StateNull);
+ C.addTransition(StateNotNull, constructFailureNoteTag(C, RetSym));
+ C.addTransition(StateNull, constructFailureNoteTag(C, RetSym));
}
void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent &Call,
@@ -556,9 +584,8 @@
StateRetNull =
StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
- C.addTransition(StateRetNotNull,
- constructNoteTag(C, StreamSym, "Stream reopened here"));
- C.addTransition(StateRetNull);
+ C.addTransition(StateRetNotNull, constructFailureNoteTag(C, StreamSym));
+ C.addTransition(StateRetNull, constructFailureNoteTag(C, StreamSym));
}
void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
@@ -579,7 +606,7 @@
// and can not be used any more.
State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
- C.addTransition(State);
+ C.addTransition(State, constructFailureNoteTag(C, Sym));
}
void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
@@ -704,10 +731,7 @@
// indicator for the stream is indeterminate.
StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (IsFread && OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
- else
- C.addTransition(StateFailed);
+ C.addTransition(StateFailed, constructFailureNoteTag(C, StreamSym));
}
void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -766,7 +790,7 @@
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
C.addTransition(StateNotFailed);
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ C.addTransition(StateFailed, constructFailureNoteTag(C, StreamSym));
}
void StreamChecker::evalClearerr(const FnDescription *Desc,
@@ -792,7 +816,8 @@
void StreamChecker::evalFeofFerror(const FnDescription *Desc,
const CallEvent &Call, CheckerContext &C,
- const StreamErrorState &ErrorKind) const {
+ const StreamErrorState &ErrorKind,
+ const char *NoteMessages[2]) const {
ProgramStateRef State = C.getState();
SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
if (!StreamSym)
@@ -813,20 +838,24 @@
// Function returns true.
// From now on it is the only one error state.
ProgramStateRef TrueState = bindAndAssumeTrue(State, C, CE);
- C.addTransition(TrueState->set<StreamMap>(
+ TrueState = TrueState->set<StreamMap>(
StreamSym, StreamState::getOpened(Desc, ErrorKind,
SS->FilePositionIndeterminate &&
- !ErrorKind.isFEof())));
+ !ErrorKind.isFEof()));
+ C.addTransition(TrueState,
+ constructNonFailureNoteTag(C, StreamSym, NoteMessages[0]));
}
if (StreamErrorState NewES = SS->ErrorState & (~ErrorKind)) {
// Execution path(s) with ErrorKind not set.
// Function returns false.
// New error state is everything before minus ErrorKind.
ProgramStateRef FalseState = bindInt(0, State, C, CE);
- C.addTransition(FalseState->set<StreamMap>(
+ FalseState = FalseState->set<StreamMap>(
StreamSym,
StreamState::getOpened(
- Desc, NewES, SS->FilePositionIndeterminate && !NewES.isFEof())));
+ Desc, NewES, SS->FilePositionIndeterminate && !NewES.isFEof()));
+ C.addTransition(FalseState,
+ constructNonFailureNoteTag(C, StreamSym, NoteMessages[1]));
}
}
@@ -901,9 +930,11 @@
// according to cppreference.com .
ExplodedNode *N = C.generateErrorNode();
if (N) {
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
+ auto R = std::make_unique<PathSensitiveBugReport>(
BT_UseAfterClose,
- "Stream might be already closed. Causes undefined behaviour.", N));
+ "Stream might be already closed. Causes undefined behaviour.", N);
+ R->markInteresting(Sym);
+ C.emitReport(std::move(R));
return nullptr;
}
@@ -917,12 +948,14 @@
// failed to open.
ExplodedNode *N = C.generateErrorNode();
if (N) {
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
+ auto R = std::make_unique<PathSensitiveBugReport>(
BT_UseAfterOpenFailed,
"Stream might be invalid after "
"(re-)opening it has failed. "
"Can cause undefined behaviour.",
- N));
+ N);
+ R->markInteresting(Sym);
+ C.emitReport(std::move(R));
return nullptr;
}
return State;
@@ -957,8 +990,10 @@
if (!N)
return nullptr;
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
- BT_IndeterminatePosition, BugMessage, N));
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ BT_IndeterminatePosition, BugMessage, N);
+ R->markInteresting(Sym);
+ C.emitReport(std::move(R));
return State->set<StreamMap>(
Sym, StreamState::getOpened(SS->LastOperation, ErrorFEof, false));
}
@@ -966,9 +1001,12 @@
// Known or unknown error state without FEOF possible.
// Stop analysis, report error.
ExplodedNode *N = C.generateErrorNode(State);
- if (N)
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
- BT_IndeterminatePosition, BugMessage, N));
+ if (N) {
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ 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
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -33,12 +33,12 @@
// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
void check_note_freopen() {
- FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+ FILE *F = fopen("file", "r");
if (!F)
// expected-note@-1 {{'F' is non-null}}
// expected-note@-2 {{Taking false branch}}
return;
- F = freopen(0, "w", F); // expected-note {{Stream reopened here}}
+ F = freopen(0, "w", F); // expected-note {{Stream opened here}}
if (!F)
// expected-note@-1 {{'F' is non-null}}
// expected-note@-2 {{Taking false branch}}
@@ -47,6 +47,32 @@
// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+void check_note_fopen_fail() {
+ FILE *F = fopen("file", "r"); // expected-note {{Assuming opening the stream fails here}} expected-note {{Assuming pointer value is null}} expected-note {{'F' initialized here}}
+ fclose(F); // expected-warning {{Stream pointer might be NULL}}
+ // expected-note@-1 {{Stream pointer might be NULL}}
+}
+
+void check_note_freopen_fail() {
+ FILE *F = fopen("file", "r");
+ if (!F) // expected-note {{'F' is non-null}} expected-note {{Taking false branch}}
+ return;
+ freopen(0, "w", F); // expected-note {{Assuming opening the stream fails here}}
+ fclose(F); // expected-warning {{Stream might be invalid after (re-)opening it has failed. Can cause undefined behaviour}}
+ // expected-note@-1 {{Stream might be invalid after (re-)opening it has failed. Can cause undefined behaviour}}
+}
+
+void check_note_freopen_fail_null() {
+ // FIXME: The following note should not be here.
+ FILE *F = fopen("file", "r"); // expected-note {{Assuming opening the stream fails here}}
+ if (!F) // expected-note {{'F' is non-null}} expected-note {{Taking false branch}}
+ return;
+ // FIXME: Note about failing open belongs here.
+ F = freopen(0, "w", F); // expected-note {{Null pointer value stored to 'F'}}
+ fclose(F); // expected-warning {{Stream pointer might be NULL}}
+ // expected-note@-1 {{Stream pointer might be NULL}}
+}
+
void check_note_leak_2(int c) {
FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
if (!F1)
@@ -80,7 +106,7 @@
void check_track_null() {
FILE *F;
- F = fopen("foo1.c", "r"); // expected-note {{Value assigned to 'F'}} expected-note {{Assuming pointer value is null}}
+ F = fopen("foo1.c", "r"); // expected-note {{Value assigned to 'F'}} expected-note {{Assuming pointer value is null}} expected-note {{Assuming opening the stream fails here}}
if (F != NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is equal to NULL}}
fclose(F);
return;
@@ -100,7 +126,7 @@
if (feof(F)) { // expected-note {{Taking true branch}}
clearerr(F);
fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
- if (feof(F)) { // expected-note {{Taking true branch}}
+ if (feof(F)) { // expected-note {{The end-of-file flag is set on the stream}} 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}}
}
@@ -124,7 +150,7 @@
return;
}
fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
- if (feof(F)) { // expected-note {{Taking true branch}}
+ if (feof(F)) { // expected-note {{The end-of-file flag is set on the stream}} 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}}
}
@@ -138,10 +164,99 @@
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 stream reaches end-of-file here}}
- if (ferror(F)) { // expected-note {{Taking false branch}}
+ if (ferror(F)) { // expected-note {{The error flag is not set on the stream}} 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);
}
+
+void check_indeterminate_notes_only_at_last_failure() {
+ 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 this stream operation fails and leaves the file position indeterminate}}
+ if (ferror(F)) { // expected-note {{The error flag is set on the stream}} 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_fseek() {
+ 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 this stream operation fails and leaves the file position indeterminate}}
+ if (!feof(F)) // expected-note{{The end-of-file flag is not set on the stream}} 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_fwrite() {
+ 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 this stream operation fails and leaves the file position indeterminate}}
+ if (ferror(F)) // expected-note {{The error flag is set on the stream}} 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_fseek_no_feof_no_ferror() {
+ 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 this stream operation fails and leaves the file position indeterminate}}
+ if (!ferror(F) && !feof(F)) // expected-note {{The error flag is not set on the stream}} expected-note {{The end-of-file flag is not set on the stream}}
+ // expected-note@-1 {{Taking true branch}} expected-note@-1 {{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);
+}
+
+void check_feof_notes_fseek() {
+ 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 stream reaches end-of-file here}}
+ if (feof(F)) // expected-note{{The end-of-file flag is set on the stream}} 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_notes_fseek() {
+ FILE *F;
+ char Buf[10];
+ F = fopen("foo1.c", "r");
+ if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is non-null}}
+ // expected-note@-1 {{Taking false branch}} expected-note@-1 {{'F' is non-null}}
+ return;
+ fseek(F, 1, SEEK_SET); // expected-note {{Assuming this stream operation fails and leaves the file position indeterminate}}
+ // expected-note@-1 {{Assuming stream reaches end-of-file here}}
+ 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}}
+ // expected-warning@-2 {{Read function called when stream is in EOF state. Function has no effect}}
+ // expected-note@-3 {{Read function called when stream is in EOF state. Function has no effect}}
+ fclose(F);
+}