From: Zhongxing Xu Date: Tue, 1 Jun 2010 03:01:33 +0000 (+0000) Subject: Add support for calloc() in MallocChecker. Patch by Jordy Rose, with my X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a5ce966d1a23d84aa5e849cf0ed62494e736ea6a;p=clang Add support for calloc() in MallocChecker. Patch by Jordy Rose, with my modification. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@105264 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Checker/PathSensitive/GRState.h b/include/clang/Checker/PathSensitive/GRState.h index 25ba1f85fd..624058e6e4 100644 --- a/include/clang/Checker/PathSensitive/GRState.h +++ b/include/clang/Checker/PathSensitive/GRState.h @@ -211,6 +211,8 @@ public: const GRState *bindLoc(SVal location, SVal V) const; + const GRState *bindDefault(SVal loc, SVal V) const; + const GRState *unbindLoc(Loc LV) const; /// Get the lvalue for a variable reference. @@ -620,6 +622,12 @@ inline const GRState *GRState::bindLoc(SVal LV, SVal V) const { return !isa(LV) ? this : bindLoc(cast(LV), V); } +inline const GRState *GRState::bindDefault(SVal loc, SVal V) const { + const MemRegion *R = cast(loc).getRegion(); + Store new_store = getStateManager().StoreMgr->BindDefault(St, R, V); + return makeWithStore(new_store); +} + inline SVal GRState::getLValue(const VarDecl* VD, const LocationContext *LC) const { return getStateManager().StoreMgr->getLValueVar(VD, LC); diff --git a/include/clang/Checker/PathSensitive/Store.h b/include/clang/Checker/PathSensitive/Store.h index f3155b9aac..030f5401f3 100644 --- a/include/clang/Checker/PathSensitive/Store.h +++ b/include/clang/Checker/PathSensitive/Store.h @@ -64,6 +64,10 @@ public: /// to the location given for \c loc. virtual Store Bind(Store store, Loc loc, SVal val) = 0; + virtual Store BindDefault(Store store, const MemRegion *R, SVal V) { + return store; + } + virtual Store Remove(Store St, Loc L) = 0; /// BindCompoundLiteral - Return the store that has the bindings currently diff --git a/lib/Checker/MallocChecker.cpp b/lib/Checker/MallocChecker.cpp index 086dbd8fdd..22b8b29f78 100644 --- a/lib/Checker/MallocChecker.cpp +++ b/lib/Checker/MallocChecker.cpp @@ -59,12 +59,12 @@ class MallocChecker : public CheckerVisitor { BuiltinBug *BT_DoubleFree; BuiltinBug *BT_Leak; BuiltinBug *BT_UseFree; - IdentifierInfo *II_malloc, *II_free, *II_realloc; + IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc; public: MallocChecker() : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), - II_malloc(0), II_free(0), II_realloc(0) {} + II_malloc(0), II_free(0), II_realloc(0), II_calloc(0) {} static void *getTag(); bool EvalCallExpr(CheckerContext &C, const CallExpr *CE); void EvalDeadSymbols(CheckerContext &C,const Stmt *S,SymbolReaper &SymReaper); @@ -76,12 +76,20 @@ public: private: void MallocMem(CheckerContext &C, const CallExpr *CE); const GRState *MallocMemAux(CheckerContext &C, const CallExpr *CE, - const Expr *SizeEx, const GRState *state); + const Expr *SizeEx, SVal Init, + const GRState *state) { + return MallocMemAux(C, CE, state->getSVal(SizeEx), Init, state); + } + const GRState *MallocMemAux(CheckerContext &C, const CallExpr *CE, + SVal SizeEx, SVal Init, + const GRState *state); + void FreeMem(CheckerContext &C, const CallExpr *CE); const GRState *FreeMemAux(CheckerContext &C, const CallExpr *CE, const GRState *state); void ReallocMem(CheckerContext &C, const CallExpr *CE); + void CallocMem(CheckerContext &C, const CallExpr *CE); }; } // end anonymous namespace @@ -120,6 +128,8 @@ bool MallocChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { II_free = &Ctx.Idents.get("free"); if (!II_realloc) II_realloc = &Ctx.Idents.get("realloc"); + if (!II_calloc) + II_calloc = &Ctx.Idents.get("calloc"); if (FD->getIdentifier() == II_malloc) { MallocMem(C, CE); @@ -136,28 +146,34 @@ bool MallocChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { return true; } + if (FD->getIdentifier() == II_calloc) { + CallocMem(C, CE); + return true; + } + return false; } void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) { - const GRState *state = MallocMemAux(C, CE, CE->getArg(0), C.getState()); + const GRState *state = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), + C.getState()); C.addTransition(state); } const GRState *MallocChecker::MallocMemAux(CheckerContext &C, const CallExpr *CE, - const Expr *SizeEx, + SVal Size, SVal Init, const GRState *state) { unsigned Count = C.getNodeBuilder().getCurrentBlockCount(); ValueManager &ValMgr = C.getValueManager(); SVal RetVal = ValMgr.getConjuredSymbolVal(NULL, CE, CE->getType(), Count); - SVal Size = state->getSVal(SizeEx); - state = C.getEngine().getStoreManager().setExtent(state, RetVal.getAsRegion(), Size); + state = state->bindDefault(RetVal, Init); + state = state->BindExpr(CE, RetVal); SymbolRef Sym = RetVal.getAsLocSymbol(); @@ -234,7 +250,8 @@ void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) { if (Sym) stateEqual = stateEqual->set(Sym, RefState::getReleased(CE)); - const GRState *stateMalloc = MallocMemAux(C, CE, CE->getArg(1), stateEqual); + const GRState *stateMalloc = MallocMemAux(C, CE, CE->getArg(1), + UndefinedVal(), stateEqual); C.addTransition(stateMalloc); } @@ -256,13 +273,30 @@ void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) { if (stateFree) { // FIXME: We should copy the content of the original buffer. const GRState *stateRealloc = MallocMemAux(C, CE, CE->getArg(1), - stateFree); + UnknownVal(), stateFree); C.addTransition(stateRealloc); } } } } +void MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE) { + const GRState *state = C.getState(); + + ValueManager &ValMgr = C.getValueManager(); + SValuator &SVator = C.getSValuator(); + + SVal Count = state->getSVal(CE->getArg(0)); + SVal EleSize = state->getSVal(CE->getArg(1)); + SVal TotalSize = SVator.EvalBinOp(state, BinaryOperator::Mul, Count, EleSize, + ValMgr.getContext().getSizeType()); + + SVal Zero = ValMgr.makeZeroVal(ValMgr.getContext().CharTy); + + state = MallocMemAux(C, CE, TotalSize, Zero, state); + C.addTransition(state); +} + void MallocChecker::EvalDeadSymbols(CheckerContext &C, const Stmt *S, SymbolReaper &SymReaper) { for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp index ddcb7bee49..1603cc81a5 100644 --- a/lib/Checker/RegionStore.cpp +++ b/lib/Checker/RegionStore.cpp @@ -280,6 +280,10 @@ public: // Part of public interface to class. Store Bind(Store store, Loc LV, SVal V); + Store BindDefault(Store store, const MemRegion *R, SVal V) { + return Add(GetRegionBindings(store), R, BindingKey::Default, V).getRoot(); + } + Store BindCompoundLiteral(Store store, const CompoundLiteralExpr* CL, const LocationContext *LC, SVal V); @@ -1233,7 +1237,7 @@ SVal RegionStoreManager::RetrieveFieldOrElementCommon(Store store, if (D->isZeroConstant()) return ValMgr.makeZeroVal(Ty); - if (D->isUnknown()) + if (D->isUnknownOrUndef()) return *D; assert(0 && "Unknown default value"); @@ -1456,6 +1460,7 @@ Store RegionStoreManager::BindCompoundLiteral(Store store, V); } + Store RegionStoreManager::setImplicitDefaultValue(Store store, const MemRegion *R, QualType T) { diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index fe24bc19e6..3d59d34f07 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -77,3 +77,35 @@ void PR7217() { buf[1] = 'c'; // not crash } +// This tests that malloc() buffers are undefined by default +char mallocGarbage () { + char *buf = malloc(2); + char result = buf[1]; // expected-warning{{undefined}} + free(buf); + return result; +} + +// This tests that calloc() buffers need to be freed +void callocNoFree () { + char *buf = calloc(2,2); + return; // expected-warning{{never released}} +} + +// These test that calloc() buffers are zeroed by default +char callocZeroesGood () { + char *buf = calloc(2,2); + char result = buf[3]; // no-warning + if (buf[1] == 0) { + free(buf); + } + return result; // no-warning +} + +char callocZeroesBad () { + char *buf = calloc(2,2); + char result = buf[3]; // no-warning + if (buf[1] != 0) { + free(buf); + } + return result; // expected-warning{{never released}} +} diff --git a/test/Analysis/outofbound.c b/test/Analysis/outofbound.c index e1ff66ccf4..2d09d8d76c 100644 --- a/test/Analysis/outofbound.c +++ b/test/Analysis/outofbound.c @@ -2,6 +2,7 @@ typedef __typeof(sizeof(int)) size_t; void *malloc(size_t); +void *calloc(size_t, size_t); char f1() { char* s = "abcd"; @@ -36,3 +37,9 @@ void f4() { p[1] = a; // no-warning p[2] = a; // expected-warning{{Access out-of-bound array element (buffer overflow)}} } + +void f5() { + char *p = calloc(2,2); + p[3] = '.'; // no-warning + p[4] = '!'; // expected-warning{{out-of-bound}} +} diff --git a/test/Analysis/undef-buffers.c b/test/Analysis/undef-buffers.c new file mode 100644 index 0000000000..4c5beb320a --- /dev/null +++ b/test/Analysis/undef-buffers.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-experimental-checks -analyzer-store=region -verify %s +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); +void free(void *); + +char stackBased1 () { + char buf[2]; + buf[0] = 'a'; + return buf[1]; // expected-warning{{Undefined}} +} + +char stackBased2 () { + char buf[2]; + buf[1] = 'a'; + return buf[0]; // expected-warning{{Undefined}} +} + +char heapBased1 () { + char *buf = malloc(2); + buf[0] = 'a'; + char result = buf[1]; // expected-warning{{undefined}} + free(buf); + return result; +} + +char heapBased2 () { + char *buf = malloc(2); + buf[1] = 'a'; + char result = buf[0]; // expected-warning{{undefined}} + free(buf); + return result; +}