balazske created this revision.
Herald added subscribers: cfe-commits, 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.
balazske added a comment.
For older discussion see this:
https://reviews.llvm.org/D75356
Functions `feof` and `ferror` are added to show how the stream error flags are
used and to make tests possible.
A new field is added to the stream state to store the actual error.
Function `fseek` is implemented to set the error state.
(More are to be added later.)
Modeling of stream error state handling functions is added.
(`clearerr` is not done yet but should follow.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75682
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/Inputs/system-header-simulator.h
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
@@ -1,26 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
-typedef __typeof__(sizeof(int)) size_t;
-typedef __typeof__(sizeof(int)) fpos_t;
-typedef struct _IO_FILE FILE;
-#define SEEK_SET 0 /* Seek from beginning of file. */
-#define SEEK_CUR 1 /* Seek from current position. */
-#define SEEK_END 2 /* Seek from end of file. */
-extern FILE *fopen(const char *path, const char *mode);
-extern FILE *tmpfile(void);
-extern int fclose(FILE *fp);
-extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern int fseek (FILE *__stream, long int __off, int __whence);
-extern long int ftell (FILE *__stream);
-extern void rewind (FILE *__stream);
-extern int fgetpos(FILE *stream, fpos_t *pos);
-extern int fsetpos(FILE *stream, const fpos_t *pos);
-extern void clearerr(FILE *stream);
-extern int feof(FILE *stream);
-extern int ferror(FILE *stream);
-extern int fileno(FILE *stream);
-extern FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+#include "Inputs/system-header-simulator.h"
void check_fread() {
FILE *fp = tmpfile();
Index: clang/test/Analysis/stream-error.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-error.c
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_eval(int);
+
+void error_fopen() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
+
+void error_freopen() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ 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 error_fseek() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ int rc = fseek(F, 0, SEEK_SET);
+ if (rc) {
+ int Eof = feof(F), Error = ferror(F);
+ clang_analyzer_eval(Eof || Error); // expected-warning {{FALSE}} \
+ // expected-warning {{TRUE}}
+ clang_analyzer_eval(Eof && Error); // expected-warning {{FALSE}}
+ } else {
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ }
+ fclose(F);
+}
\ No newline at end of file
Index: clang/test/Analysis/Inputs/system-header-simulator.h
===================================================================
--- clang/test/Analysis/Inputs/system-header-simulator.h
+++ clang/test/Analysis/Inputs/system-header-simulator.h
@@ -9,7 +9,16 @@
#define restrict /*restrict*/
#endif
+typedef __typeof(sizeof(int)) size_t;
+typedef long long __int64_t;
+typedef __int64_t __darwin_off_t;
+typedef __darwin_off_t fpos_t;
+
typedef struct _FILE FILE;
+#define SEEK_SET 0 /* Seek from beginning of file. */
+#define SEEK_CUR 1 /* Seek from current position. */
+#define SEEK_END 2 /* Seek from end of file. */
+
extern FILE *stdin;
extern FILE *stdout;
extern FILE *stderr;
@@ -24,11 +33,34 @@
int fprintf(FILE *restrict, const char *restrict, ...);
int getchar(void);
+void setbuf(FILE * restrict, char * restrict);
+int setvbuf(FILE * restrict, char * restrict, int, size_t);
+
+FILE *funopen(const void *,
+ int (*)(void *, char *, int),
+ int (*)(void *, const char *, int),
+ fpos_t (*)(void *, fpos_t, int),
+ int (*)(void *));
+
+FILE *fopen(const char *path, const char *mode);
+FILE *tmpfile(void);
+FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+int fclose(FILE *fp);
+size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
+int fseek (FILE *__stream, long int __off, int __whence);
+long int ftell (FILE *__stream);
+void rewind (FILE *__stream);
+int fgetpos(FILE *stream, fpos_t *pos);
+int fsetpos(FILE *stream, const fpos_t *pos);
+void clearerr(FILE *stream);
+int feof(FILE *stream);
+int ferror(FILE *stream);
+int fileno(FILE *stream);
+
// Note, on some platforms errno macro gets replaced with a function call.
extern int errno;
-typedef __typeof(sizeof(int)) size_t;
-
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
@@ -39,21 +71,6 @@
typedef __darwin_pthread_key_t pthread_key_t;
int pthread_setspecific(pthread_key_t, const void *);
-typedef long long __int64_t;
-typedef __int64_t __darwin_off_t;
-typedef __darwin_off_t fpos_t;
-
-void setbuf(FILE * restrict, char * restrict);
-int setvbuf(FILE * restrict, char * restrict, int, size_t);
-
-FILE *fopen(const char * restrict, const char * restrict);
-int fclose(FILE *);
-FILE *funopen(const void *,
- int (*)(void *, char *, int),
- int (*)(void *, const char *, int),
- fpos_t (*)(void *, fpos_t, int),
- int (*)(void *));
-
int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));
typedef void (*freeCallback) (void*);
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -27,25 +27,52 @@
namespace {
+/// Full state information about a stream pointer.
struct StreamState {
- enum Kind { Opened, Closed, OpenFailed, Escaped } K;
-
- StreamState(Kind k) : K(k) {}
-
- bool isOpened() const { return K == Opened; }
- bool isClosed() const { return K == Closed; }
- bool isOpenFailed() const { return K == OpenFailed; }
- //bool isEscaped() const { return K == Escaped; }
-
- bool operator==(const StreamState &X) const { return K == X.K; }
+ /// State of a stream symbol.
+ enum KindTy {
+ Opened, /// Stream is opened.
+ Closed, /// Closed stream (an invalid stream pointer after it was closed).
+ OpenFailed /// The last open operation has failed.
+ } State;
+
+ /// The error state of a stream.
+ enum ErrorKindTy {
+ NoError, /// No error flag is set or stream is not open.
+ EofError, /// EOF condition (`feof` is true).
+ OtherError, /// Other (non-EOF) error (`ferror` is true).
+ AnyError /// EofError or OtherError, used if exact error is unknown.
+ } ErrorState = NoError;
+
+ bool isOpened() const { return State == Opened; }
+ bool isClosed() const { return State == Closed; }
+ bool isOpenFailed() const { return State == OpenFailed; }
+
+ bool isNoError() const { return ErrorState == NoError; }
+ bool isEofError() const { return ErrorState == EofError; }
+ bool isOtherError() const { return ErrorState == OtherError; }
+ bool isAnyError() const { return ErrorState == AnyError; }
+
+ bool operator==(const StreamState &X) const {
+ return State == X.State && ErrorState == X.ErrorState;
+ }
- static StreamState getOpened() { return StreamState(Opened); }
- static StreamState getClosed() { return StreamState(Closed); }
- static StreamState getOpenFailed() { return StreamState(OpenFailed); }
- static StreamState getEscaped() { return StreamState(Escaped); }
+ static StreamState getOpened() { return StreamState{Opened}; }
+ static StreamState getClosed() { return StreamState{Closed}; }
+ static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
+ static StreamState getOpenedWithAnyError() {
+ return StreamState{Opened, AnyError};
+ }
+ static StreamState getOpenedWithEofError() {
+ return StreamState{Opened, EofError};
+ }
+ static StreamState getOpenedWithOtherError() {
+ return StreamState{Opened, OtherError};
+ }
void Profile(llvm::FoldingSetNodeID &ID) const {
- ID.AddInteger(K);
+ ID.AddInteger(State);
+ ID.AddInteger(ErrorState);
}
};
@@ -71,6 +98,16 @@
return Call.getArgSVal(Desc->StreamArgNo);
}
+/// Create a conjured symbol return value for a call expression.
+DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
+ assert(CE && "Expecting a call expression.");
+
+ const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+ return C.getSValBuilder()
+ .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+ .castAs<DefinedSVal>();
+}
+
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
@@ -82,7 +119,6 @@
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
private:
-
CallDescriptionMap<FnDescription> FnDescriptions = {
{{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
{{"freopen", 3},
@@ -92,14 +128,15 @@
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
{{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
{{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
- {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 0}},
+ {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
{{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
{{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
{{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
{{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"feof", 1}, {&StreamChecker::preDefault, &StreamChecker::evalFeof, 0}},
+ {{"ferror", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalFerror, 0}},
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
@@ -116,10 +153,17 @@
void preFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFseek(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFeof(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFerror(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.
@@ -185,15 +229,11 @@
void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SValBuilder &SVB = C.getSValBuilder();
- const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;
- DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
- .castAs<DefinedSVal>();
+ DefinedSVal RetVal = makeRetVal(C, CE);
SymbolRef RetSym = RetVal.getAsSymbol();
assert(RetSym && "RetVal must be a symbol here.");
@@ -299,6 +339,131 @@
C.addTransition(State);
}
+void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ // Ignore the call if the stream is not tracked.
+ if (!State->get<StreamMap>(StreamSym))
+ return;
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+
+ // Make expression result.
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+
+ // Bifurcate the state into failed and non-failed.
+ // Return zero on success, nonzero on error.
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ // Reset the state to opened with no error.
+ StateNotFailed =
+ StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened());
+ // There are two ways of failure:
+ // We get some error (ferror or feof).
+ ProgramStateRef StateFailedWithFError = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpenedWithAnyError());
+ // We get none of the error flags.
+ ProgramStateRef StateFailedWithoutFError =
+ StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened());
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailedWithFError);
+ C.addTransition(StateFailedWithoutFError);
+}
+
+void StreamChecker::evalFeof(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ // Ignore the call if the stream is not tracked.
+ if (!SS)
+ return;
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+
+ // Make expression result.
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+
+ if (SS->isAnyError()) {
+ // We do not know yet what kind of error is set.
+ // Differentiate between EOF and other error.
+ ProgramStateRef StateEof, StateOther;
+ std::tie(StateEof, StateOther) = State->assume(RetVal);
+ assert(StateEof && StateOther &&
+ "Return value should not be constrained already.");
+ StateEof = StateEof->set<StreamMap>(StreamSym,
+ StreamState::getOpenedWithEofError());
+ StateOther = StateOther->set<StreamMap>(
+ StreamSym, StreamState::getOpenedWithOtherError());
+ C.addTransition(StateEof);
+ C.addTransition(StateOther);
+ } else {
+ // We know what error is set, make the return value accordingly.
+ State = State->assume(RetVal, SS->isEofError());
+ assert(State && "Return value should not be constrained already.");
+ C.addTransition(State);
+ }
+}
+
+void StreamChecker::evalFerror(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ // Ignore the call if the stream is not tracked.
+ if (!SS)
+ return;
+
+ // Make expression result.
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+
+ if (SS->isAnyError()) {
+ // We do not know yet what kind of error is set.
+ // Differentiate between EOF and other error.
+ ProgramStateRef StateEof, StateOther;
+ std::tie(StateOther, StateEof) = State->assume(RetVal);
+ assert(StateEof && StateOther &&
+ "Return value should not be constrained already.");
+ StateEof = StateEof->set<StreamMap>(StreamSym,
+ StreamState::getOpenedWithEofError());
+ StateOther = StateOther->set<StreamMap>(
+ StreamSym, StreamState::getOpenedWithOtherError());
+ C.addTransition(StateEof);
+ C.addTransition(StateOther);
+ } else {
+ // We know what error is set, make the return value accordingly.
+ State = State->assume(RetVal, SS->isOtherError());
+ assert(State && "Return value should not be constrained already.");
+ C.addTransition(State);
+ }
+}
+
void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
