From ec5623570e5862efa6156fed44e74e923af925ab Mon Sep 17 00:00:00 2001 From: Zhongxing Xu Date: Mon, 19 Jul 2010 01:52:29 +0000 Subject: [PATCH] Add double close check to StreamChecker. Patch by Lei Zhang. llvm-svn: 108669 --- clang/lib/Checker/StreamChecker.cpp | 91 +++++++++++++++++++++++++++-- clang/test/Analysis/stream.c | 7 +++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/clang/lib/Checker/StreamChecker.cpp b/clang/lib/Checker/StreamChecker.cpp index c527ca24496f..3945ed095df4 100644 --- a/clang/lib/Checker/StreamChecker.cpp +++ b/clang/lib/Checker/StreamChecker.cpp @@ -23,18 +23,42 @@ using namespace clang; namespace { +struct StreamState { + enum Kind { Opened, Closed, OpenFailed } K; + const Stmt *S; + + StreamState(Kind k, const Stmt *s) : K(k), S(s) {} + + bool isOpened() const { return K == Opened; } + bool isClosed() const { return K == Closed; } + bool isOpenFailed() const { return K == OpenFailed; } + + bool operator==(const StreamState &X) const { + return K == X.K && S == X.S; + } + + static StreamState getOpened(const Stmt *s) { return StreamState(Opened, s); } + static StreamState getClosed(const Stmt *s) { return StreamState(Closed, s); } + static StreamState getOpenFailed(const Stmt *s) { return StreamState(OpenFailed, s); } + + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddInteger(K); + ID.AddPointer(S); + } +}; + class StreamChecker : public CheckerVisitor { - IdentifierInfo *II_fopen, *II_fread, *II_fwrite, + IdentifierInfo *II_fopen, *II_fclose,*II_fread, *II_fwrite, *II_fseek, *II_ftell, *II_rewind, *II_fgetpos, *II_fsetpos, *II_clearerr, *II_feof, *II_ferror, *II_fileno; - BuiltinBug *BT_nullfp, *BT_illegalwhence; + BuiltinBug *BT_nullfp, *BT_illegalwhence, *BT_doubleclose; public: StreamChecker() - : II_fopen(0), II_fread(0), II_fwrite(0), + : II_fopen(0), II_fclose(0), II_fread(0), II_fwrite(0), II_fseek(0), II_ftell(0), II_rewind(0), II_fgetpos(0), II_fsetpos(0), II_clearerr(0), II_feof(0), II_ferror(0), II_fileno(0), - BT_nullfp(0), BT_illegalwhence(0) {} + BT_nullfp(0), BT_illegalwhence(0), BT_doubleclose(0) {} static void *getTag() { static int x; @@ -45,6 +69,7 @@ public: private: void Fopen(CheckerContext &C, const CallExpr *CE); + void Fclose(CheckerContext &C, const CallExpr *CE); void Fread(CheckerContext &C, const CallExpr *CE); void Fwrite(CheckerContext &C, const CallExpr *CE); void Fseek(CheckerContext &C, const CallExpr *CE); @@ -60,10 +85,20 @@ private: // Return true indicates the stream pointer is NULL. const GRState *CheckNullStream(SVal SV, const GRState *state, CheckerContext &C); + const GRState *CheckDoubleClose(const CallExpr *CE, const GRState *state, + CheckerContext &C); }; } // end anonymous namespace +namespace clang { + template <> + struct GRStateTrait + : public GRStatePartialTrait > { + static void *GDMIndex() { return StreamChecker::getTag(); } + }; +} + void clang::RegisterStreamChecker(GRExprEngine &Eng) { Eng.registerCheck(new StreamChecker()); } @@ -79,6 +114,8 @@ bool StreamChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { ASTContext &Ctx = C.getASTContext(); if (!II_fopen) II_fopen = &Ctx.Idents.get("fopen"); + if (!II_fclose) + II_fclose = &Ctx.Idents.get("fclose"); if (!II_fread) II_fread = &Ctx.Idents.get("fread"); if (!II_fwrite) @@ -106,6 +143,10 @@ bool StreamChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { Fopen(C, CE); return true; } + if (FD->getIdentifier() == II_fclose) { + Fclose(C, CE); + return true; + } if (FD->getIdentifier() == II_fread) { Fread(C, CE); return true; @@ -168,10 +209,23 @@ void StreamChecker::Fopen(CheckerContext &C, const CallExpr *CE) { const GRState *stateNotNull, *stateNull; llvm::tie(stateNotNull, stateNull) = CM.AssumeDual(state, RetVal); + SymbolRef Sym = RetVal.getAsSymbol(); + assert(Sym); + + // if RetVal is not NULL, set the symbol's state to Opened. + stateNotNull = stateNotNull->set(Sym, StreamState::getOpened(CE)); + stateNull = stateNull->set(Sym, StreamState::getOpenFailed(CE)); + C.addTransition(stateNotNull); C.addTransition(stateNull); } +void StreamChecker::Fclose(CheckerContext &C, const CallExpr *CE) { + const GRState *state = CheckDoubleClose(CE, C.getState(), C); + if (state) + C.addTransition(state); +} + void StreamChecker::Fread(CheckerContext &C, const CallExpr *CE) { const GRState *state = C.getState(); if (!CheckNullStream(state->getSVal(CE->getArg(3)), state, C)) @@ -285,3 +339,32 @@ const GRState *StreamChecker::CheckNullStream(SVal SV, const GRState *state, } return stateNotNull; } + +const GRState *StreamChecker::CheckDoubleClose(const CallExpr *CE, + const GRState *state, + CheckerContext &C) { + SymbolRef Sym = state->getSVal(CE->getArg(0)).getAsSymbol(); + assert(Sym); + + const StreamState *SS = state->get(Sym); + assert(SS); + + // Check: Double close a File Descriptor could cause undefined behaviour. + // Conforming to man-pages + if (SS->isClosed()) { + ExplodedNode *N = C.GenerateSink(); + if (N) { + if (!BT_doubleclose) + BT_doubleclose = new BuiltinBug("Double fclose", + "Try to close a file Descriptor already" + " closed. Cause undefined behaviour."); + BugReport *R = new BugReport(*BT_doubleclose, + BT_doubleclose->getDescription(), N); + C.EmitReport(R); + } + return NULL; + } + + // Close the File Descriptor. + return state->set(Sym, StreamState::getClosed(CE)); +} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index 2b6a90353b9b..953fc4dadc61 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -6,6 +6,7 @@ typedef struct _IO_FILE 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 int fclose(FILE *fp); extern size_t fread(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); @@ -39,3 +40,9 @@ void f5(void) { fseek(p, 1, SEEK_SET); // no-warning fseek(p, 1, 3); // expected-warning {{The whence argument to fseek() should be SEEK_SET, SEEK_END, or SEEK_CUR.}} } + +void f6(void) { + FILE *p = fopen("foo", "r"); + fclose(p); + fclose(p); // expected-warning {{Try to close a file Descriptor already closed. Cause Undefined Behaviour.}} +}