balazske created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The checker warns if specific stream operation is called in a failed state
(that is called "indeterminate file position condition").
The commit adds indication of location of the previously failed operation
that causes the failed state.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105003
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
@@ -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);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ 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<EofSeqMap>(StreamSym, 0);
+ProgramStateRef initSeq(SymbolRef StreamSym, ProgramStateRef State) {
+ State = State->set<EofSeqMap>(StreamSym, 0);
+ return State->set<IndeterminateSeqMap>(StreamSym, 0);
}
ProgramStateRef incrementEofSeq(SymbolRef StreamSym, ProgramStateRef State) {
const unsigned int *Seq = State->get<EofSeqMap>(StreamSym);
- assert(Seq && "EofSeqMap should be initialized first");
+ assert(Seq && "Map should be initialized first");
return State->set<EofSeqMap>(StreamSym, *Seq + 1);
}
+ProgramStateRef incrementIndeterminateSeq(SymbolRef StreamSym,
+ ProgramStateRef State) {
+ const unsigned int *Seq = State->get<IndeterminateSeqMap>(StreamSym);
+ assert(Seq && "Map should be initialized first");
+ return State->set<IndeterminateSeqMap>(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<IndeterminateSeqMap>(StreamSym);
+ if (!CurrIndeterminateSeq)
+ return nullptr;
+ const unsigned int *PredIndeterminateSeq =
+ PredState->get<IndeterminateSeqMap>(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<PathDiagnosticEventPiece>(
+ L, "Assuming that this stream operation fails", true);
+ }
+ return nullptr;
+ }
+
return nullptr;
}
@@ -575,7 +607,7 @@
StateNotNull =
StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
- StateNotNull = initEofSeq(RetSym, StateNotNull);
+ StateNotNull = initSeq(RetSym, StateNotNull);
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
@@ -785,6 +817,8 @@
StateFailed = StateFailed->set<StreamMap>(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<IndeterminateSeqMap>(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<PathSensitiveBugReport>(
- BT_IndeterminatePosition, BugMessage, N));
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ BT_IndeterminatePosition, BugMessage, N);
+ R->addVisitor<StreamBugVisitor>(Sym, StreamBugVisitor::Indeterminate,
+ *IndeterminateSeq);
+ C.emitReport(std::move(R));
return State->set<StreamMap>(
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<PathSensitiveBugReport>(
- BT_IndeterminatePosition, BugMessage, N));
+ if (N) {
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ BT_IndeterminatePosition, BugMessage, N);
+ R->addVisitor<StreamBugVisitor>(Sym, StreamBugVisitor::Indeterminate,
+ *IndeterminateSeq);
+ C.emitReport(std::move(R));
+ }
return nullptr;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
