balazske updated this revision to Diff 249643.
balazske added a comment.
Adding correct diff.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
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,40 @@
+// 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_clearerr() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ int ch = fputc('a', F);
+ if (ch == EOF) {
+ // FIXME: fputc not handled by checker yet, should expect TRUE
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ clearerr(F);
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+ }
+ fclose(F);
+}
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,35 @@
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 fputc(int ch, 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 +72,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*);
@@ -117,5 +135,6 @@
#define __DARWIN_NULL 0
#define NULL __DARWIN_NULL
#endif
+#define EOF (-1)
-#define offsetof(t, d) __builtin_offsetof(t, d)
\ No newline at end of file
+#define offsetof(t, d) __builtin_offsetof(t, d)
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,7 +19,6 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include <functional>
using namespace clang;
using namespace ento;
@@ -27,25 +26,60 @@
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; }
+ /// State of a stream symbol.
+ /// FIXME: use a bool isOpened instead.
+ 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.
+ /// Valid only if the stream is opened.
+ enum ErrorKindTy {
+ /// No error flag is set (or stream is not open).
+ NoError,
+ /// EOF condition (`feof` is true).
+ FEof,
+ /// Other generic (non-EOF) error (`ferror` is true).
+ FError,
+ } ErrorState = NoError;
+
+ bool isOpened() const { return State == Opened; }
+ bool isClosed() const { return State == Closed; }
+ bool isOpenFailed() const { return State == OpenFailed; }
+
+ bool isNoError() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == NoError;
+ }
+ bool isFEof() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == FEof;
+ }
+ bool isFError() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == FError;
+ }
- bool operator==(const StreamState &X) const { return K == X.K; }
+ bool operator==(const StreamState &X) const {
+ // In not opened state error should always NoError.
+ 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 getOpenedWithFEof() { return StreamState{Opened, FEof}; }
+ static StreamState getOpenedWithFError() {
+ return StreamState{Opened, FError};
+ }
void Profile(llvm::FoldingSetNodeID &ID) const {
- ID.AddInteger(K);
+ ID.AddInteger(State);
+ ID.AddInteger(ErrorState);
}
};
@@ -71,6 +105,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,
@@ -96,9 +140,14 @@
{{"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}},
+ {{"clearerr", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
+ {{"feof", 1},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
+ {{"ferror", 1},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
@@ -119,6 +168,13 @@
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ template <bool (StreamState::*IsOfError)() const>
+ void evalFeofFerror(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
@@ -182,15 +238,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.");
@@ -296,6 +348,54 @@
C.addTransition(State);
}
+void StreamChecker::evalClearerr(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ if (SS->isNoError())
+ return;
+
+ State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+ C.addTransition(State);
+}
+
+template <bool (StreamState::*IsOfError)() const>
+void StreamChecker::evalFeofFerror(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);
+
+ // Make the return value accordingly to the error.
+ State = State->assume(RetVal, (SS->*IsOfError)());
+ 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
