balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
If a stream operation is encountered with unknown stream (to the checker)
the stream is recognized and added to the data structure.
(Instead of ignoring the call.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78280
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -8,30 +8,50 @@
fclose(fp);
}
+void check_feread_noopen(FILE *fp) {
+ fread(0, 0, 0, fp);
+}
+
void check_fwrite() {
FILE *fp = tmpfile();
fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_fwrite_noopen(FILE *fp) {
+ fwrite(0, 0, 0, fp);
+}
+
void check_fseek() {
FILE *fp = tmpfile();
fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_fseek_noopen(FILE *fp) {
+ fseek(fp, 0, 0);
+}
+
void check_ftell() {
FILE *fp = tmpfile();
ftell(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_ftell_noopen(FILE *fp) {
+ ftell(fp);
+}
+
void check_rewind() {
FILE *fp = tmpfile();
rewind(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_rewind_noopen(FILE *fp) {
+ rewind(fp);
+}
+
void check_fgetpos() {
FILE *fp = tmpfile();
fpos_t pos;
@@ -39,6 +59,11 @@
fclose(fp);
}
+void check_fgetpos_noopen(FILE *fp) {
+ fpos_t pos;
+ fgetpos(fp, &pos);
+}
+
void check_fsetpos() {
FILE *fp = tmpfile();
fpos_t pos;
@@ -46,30 +71,51 @@
fclose(fp);
}
+void check_fsetpos_noopen(FILE *fp) {
+ fpos_t pos;
+ fsetpos(fp, &pos);
+}
+
void check_clearerr() {
FILE *fp = tmpfile();
clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_clearerr_noopen(FILE *fp) {
+ clearerr(fp);
+}
+
void check_feof() {
FILE *fp = tmpfile();
feof(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_feof_noopen(FILE *fp) {
+ feof(fp);
+}
+
void check_ferror() {
FILE *fp = tmpfile();
ferror(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_ferror_noopen(FILE *fp) {
+ ferror(fp);
+}
+
void check_fileno() {
FILE *fp = tmpfile();
fileno(fp); // expected-warning {{Stream pointer might be NULL}}
fclose(fp);
}
+void check_fileno_noopen(FILE *fp) {
+ fileno(fp);
+}
+
void f_open(void) {
FILE *p = fopen("foo", "r");
char buf[1024];
Index: clang/test/Analysis/stream-error.c
===================================================================
--- clang/test/Analysis/stream-error.c
+++ clang/test/Analysis/stream-error.c
@@ -3,6 +3,7 @@
#include "Inputs/system-header-simulator.h"
void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
void StreamTesterChecker_make_feof_stream(FILE *);
void StreamTesterChecker_make_ferror_stream(FILE *);
@@ -15,16 +16,12 @@
fclose(F);
}
-void error_freopen() {
- FILE *F = fopen("file", "r");
- if (!F)
- return;
+void error_freopen(FILE *F) {
F = freopen(0, "w", F);
if (!F)
return;
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
- fclose(F);
}
void stream_error_feof() {
@@ -40,6 +37,15 @@
fclose(F);
}
+void stream_error_feof_noopen(FILE *F) {
+ if (feof(F))
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ else if (ferror(F))
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
void stream_error_ferror() {
FILE *F = fopen("file", "r");
if (!F)
@@ -53,10 +59,16 @@
fclose(F);
}
-void error_fseek() {
- FILE *F = fopen("file", "r");
- if (!F)
- return;
+void stream_error_ferror_noopen(FILE *F) {
+ if (ferror(F))
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ else if (feof(F))
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void error_fseek(FILE *F) {
int rc = fseek(F, 0, SEEK_SET);
if (rc) {
int IsFEof = feof(F), IsFError = ferror(F);
@@ -81,5 +93,4 @@
clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
}
- fclose(F);
}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -31,6 +31,9 @@
struct StreamState {
/// The last file operation called in the stream.
const FnDescription *LastOperation;
+ /// Was a opening function encountered for this stream?
+ /// (False if the stream was first encountered in a non-opening function.)
+ bool OpenEncountered;
/// State of a stream symbol.
/// FIXME: We need maybe an "escaped" state later.
@@ -82,17 +85,23 @@
ErrorState == X.ErrorState;
}
- static StreamState getOpened(const FnDescription *L) {
- return StreamState{L, Opened};
+ static StreamState getOpened(const FnDescription *L,
+ const StreamState *PrevState,
+ ErrorKindTy NewError) {
+ if (PrevState)
+ return StreamState{L, PrevState->OpenEncountered, Opened, NewError};
+ else
+ return StreamState{L, false, Opened, NewError};
}
- static StreamState getOpened(const FnDescription *L, ErrorKindTy E) {
- return StreamState{L, Opened, E};
+ static StreamState getOpened(const FnDescription *L,
+ bool OpenEncountered = true) {
+ return StreamState{L, OpenEncountered, Opened, NoError};
}
static StreamState getClosed(const FnDescription *L) {
- return StreamState{L, Closed};
+ return StreamState{L, false, Closed};
}
static StreamState getOpenFailed(const FnDescription *L) {
- return StreamState{L, OpenFailed};
+ return StreamState{L, false, OpenFailed};
}
/// Return if the specified error kind is possible on the stream in the
@@ -106,6 +115,7 @@
void Profile(llvm::FoldingSetNodeID &ID) const {
ID.AddPointer(LastOperation);
+ ID.AddBoolean(OpenEncountered);
ID.AddInteger(State);
ID.AddInteger(ErrorState);
}
@@ -268,6 +278,9 @@
void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalDebugDumpState(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
/// Check that the stream (in StreamVal) is not NULL.
/// If it can only be NULL a fatal error is emitted and nullptr returned.
/// Otherwise the return value is a new state where the stream is constrained
@@ -310,7 +323,7 @@
REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
inline void assertStreamStateOpened(const StreamState *SS) {
- assert(SS->isOpened() &&
+ assert((!SS || SS->isOpened()) &&
"Previous create of error node for non-opened stream failed?");
}
@@ -394,6 +407,10 @@
if (!StreamSym)
return;
+ bool OpenEncountered = false;
+ if (const StreamState *SS = State->get<StreamMap>(StreamSym))
+ OpenEncountered = SS->OpenEncountered;
+
// Generate state for non-failed case.
// Return value is the passed stream pointer.
// According to the documentations, the stream is closed first
@@ -405,8 +422,8 @@
ProgramStateRef StateRetNull = State->BindExpr(CE, C.getLocationContext(),
C.getSValBuilder().makeNull());
- StateRetNotNull =
- StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ StateRetNotNull = StateRetNotNull->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, OpenEncountered));
StateRetNull =
StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
@@ -422,8 +439,6 @@
return;
const StreamState *SS = State->get<StreamMap>(Sym);
- if (!SS)
- return;
assertStreamStateOpened(SS);
@@ -463,13 +478,12 @@
if (!CE)
return;
- // Ignore the call if the stream is not tracked.
- if (!State->get<StreamMap>(StreamSym))
- return;
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
- DefinedSVal RetVal = makeRetVal(C, CE);
+ assertStreamStateOpened(SS);
// Make expression result.
+ DefinedSVal RetVal = makeRetVal(C, CE);
State = State->BindExpr(CE, C.getLocationContext(), RetVal);
// Bifurcate the state into failed and non-failed.
@@ -479,11 +493,11 @@
C.getConstraintManager().assumeDual(State, RetVal);
// Reset the state to opened with no error.
- StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ StateNotFailed = StateNotFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, SS, StreamState::NoError));
// We get error.
StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, StreamState::Unknown));
+ StreamSym, StreamState::getOpened(Desc, SS, StreamState::Unknown));
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@@ -498,12 +512,12 @@
return;
const StreamState *SS = State->get<StreamMap>(StreamSym);
- if (!SS)
- return;
assertStreamStateOpened(SS);
- State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ State = State->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, SS, StreamState::NoError));
+
C.addTransition(State);
}
@@ -520,18 +534,23 @@
if (!CE)
return;
- auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State,
- StreamState::ErrorKindTy SSError) {
- StreamState SSNew = StreamState::getOpened(Desc, SSError);
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+
+ assertStreamStateOpened(SS);
+
+ auto AddTransition = [&C, Desc, StreamSym,
+ SS](ProgramStateRef State,
+ StreamState::ErrorKindTy NewError) {
+ StreamState SSNew = StreamState::getOpened(Desc, SS, NewError);
State = State->set<StreamMap>(StreamSym, SSNew);
C.addTransition(State);
};
- const StreamState *SS = State->get<StreamMap>(StreamSym);
- if (!SS)
+ if (!SS) {
+ AddTransition(bindInt(0, State, C, CE), StreamState::Unknown);
+ AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind);
return;
-
- assertStreamStateOpened(SS);
+ }
if (!SS->isUnknown()) {
// The stream is in exactly known state (error or not).
@@ -582,8 +601,8 @@
assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
const StreamState *SS = State->get<StreamMap>(StreamSym);
assert(SS && "Stream should be tracked by the checker.");
- State = State->set<StreamMap>(StreamSym,
- StreamState::getOpened(SS->LastOperation, EK));
+ State = State->set<StreamMap>(
+ StreamSym, StreamState::getOpened(SS->LastOperation, SS, EK));
C.addTransition(State);
}
@@ -698,7 +717,7 @@
for (const auto &I : Map) {
SymbolRef Sym = I.first;
const StreamState &SS = I.second;
- if (!SymReaper.isDead(Sym) || !SS.isOpened())
+ if (!SymReaper.isDead(Sym) || !SS.isOpened() || !SS.OpenEncountered)
continue;
ExplodedNode *N = C.generateErrorNode();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
