Random related thought, should clang warn on: -- int *x = malloc(11); x[1]; -- when it can prove alloc size differs from multiple of type size?
To be false positive free, the warning should probably check that the pointer is directly indexed by a non-zero value, to avoid warning on cases when extra data is being allocated following the object. - Daniel On Mon, Jan 18, 2010 at 12:54 AM, Zhongxing Xu <[email protected]> wrote: > Author: zhongxingxu > Date: Mon Jan 18 02:54:31 2010 > New Revision: 93722 > > URL: http://llvm.org/viewvc/llvm-project?rev=93722&view=rev > Log: > Add support for computing size in elements for symbolic regions obtained from > malloc(). > > Modified: > cfe/trunk/include/clang/Analysis/PathSensitive/Store.h > cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp > cfe/trunk/lib/Analysis/MallocChecker.cpp > cfe/trunk/lib/Analysis/RegionStore.cpp > cfe/trunk/lib/Analysis/ReturnPointerRangeChecker.cpp > cfe/trunk/test/Analysis/outofbound.c > > Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Store.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Store.h?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Analysis/PathSensitive/Store.h (original) > +++ cfe/trunk/include/clang/Analysis/PathSensitive/Store.h Mon Jan 18 > 02:54:31 2010 > @@ -105,7 +105,8 @@ > > // FIXME: Make out-of-line. > virtual DefinedOrUnknownSVal getSizeInElements(const GRState *state, > - const MemRegion *region) { > + const MemRegion *region, > + QualType EleTy) { > return UnknownVal(); > } > > > Modified: cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp (original) > +++ cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp Mon Jan 18 02:54:31 2010 > @@ -57,7 +57,8 @@ > > // Get the size of the array. > DefinedOrUnknownSVal NumElements > - = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion()); > + = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(), > + > ER->getValueType(C.getASTContext())); > > const GRState *StInBound = state->AssumeInBound(Idx, NumElements, true); > const GRState *StOutBound = state->AssumeInBound(Idx, NumElements, false); > > Modified: cfe/trunk/lib/Analysis/MallocChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/MallocChecker.cpp?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/MallocChecker.cpp (original) > +++ cfe/trunk/lib/Analysis/MallocChecker.cpp Mon Jan 18 02:54:31 2010 > @@ -72,7 +72,7 @@ > private: > void MallocMem(CheckerContext &C, const CallExpr *CE); > const GRState *MallocMemAux(CheckerContext &C, const CallExpr *CE, > - const GRState *state); > + const Expr *SizeEx, const GRState *state); > void FreeMem(CheckerContext &C, const CallExpr *CE); > const GRState *FreeMemAux(CheckerContext &C, const CallExpr *CE, > const GRState *state); > @@ -136,18 +136,24 @@ > } > > void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) { > - const GRState *state = MallocMemAux(C, CE, C.getState()); > + const GRState *state = MallocMemAux(C, CE, CE->getArg(0), C.getState()); > C.addTransition(state); > } > > const GRState *MallocChecker::MallocMemAux(CheckerContext &C, > const CallExpr *CE, > + const Expr *SizeEx, > 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->BindExpr(CE, RetVal); > > SymbolRef Sym = RetVal.getAsLocSymbol(); > @@ -216,7 +222,7 @@ > if (Sym) > stateEqual = stateEqual->set<RegionState>(Sym, > RefState::getReleased(CE)); > > - const GRState *stateMalloc = MallocMemAux(C, CE, stateEqual); > + const GRState *stateMalloc = MallocMemAux(C, CE, CE->getArg(1), > stateEqual); > C.addTransition(stateMalloc); > } > > @@ -237,7 +243,8 @@ > const GRState *stateFree = FreeMemAux(C, CE, stateSizeNotZero); > if (stateFree) { > // FIXME: We should copy the content of the original buffer. > - const GRState *stateRealloc = MallocMemAux(C, CE, stateFree); > + const GRState *stateRealloc = MallocMemAux(C, CE, CE->getArg(1), > + stateFree); > C.addTransition(stateRealloc); > } > } > > Modified: cfe/trunk/lib/Analysis/RegionStore.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/RegionStore.cpp (original) > +++ cfe/trunk/lib/Analysis/RegionStore.cpp Mon Jan 18 02:54:31 2010 > @@ -21,6 +21,7 @@ > #include "clang/Analysis/Analyses/LiveVariables.h" > #include "clang/Analysis/Support/Optional.h" > #include "clang/Basic/TargetInfo.h" > +#include "clang/AST/CharUnits.h" > > #include "llvm/ADT/ImmutableMap.h" > #include "llvm/ADT/ImmutableList.h" > @@ -423,9 +424,9 @@ > // Region "extents". > //===------------------------------------------------------------------===// > > - const GRState *setExtent(const GRState *state, const MemRegion* R, SVal > Extent); > + const GRState *setExtent(const GRState *state,const MemRegion* R,SVal > Extent); > DefinedOrUnknownSVal getSizeInElements(const GRState *state, > - const MemRegion* R); > + const MemRegion* R, QualType EleTy); > > //===------------------------------------------------------------------===// > // Utility methods. > @@ -767,7 +768,8 @@ > //===----------------------------------------------------------------------===// > > DefinedOrUnknownSVal RegionStoreManager::getSizeInElements(const GRState > *state, > - const MemRegion > *R) { > + const MemRegion > *R, > + QualType EleTy) { > > switch (R->getKind()) { > case MemRegion::CXXThisRegionKind: > @@ -793,10 +795,25 @@ > case MemRegion::ElementRegionKind: > case MemRegion::FieldRegionKind: > case MemRegion::ObjCIvarRegionKind: > - case MemRegion::SymbolicRegionKind: > case MemRegion::CXXObjectRegionKind: > return UnknownVal(); > > + case MemRegion::SymbolicRegionKind: { > + const SVal *Size = state->get<RegionExtents>(R); > + if (!Size) > + return UnknownVal(); > + const nonloc::ConcreteInt *CI = dyn_cast<nonloc::ConcreteInt>(Size); > + if (!CI) > + return UnknownVal(); > + > + CharUnits RegionSize = > + CharUnits::fromQuantity(CI->getValue().getSExtValue()); > + CharUnits EleSize = getContext().getTypeSizeInChars(EleTy); > + assert(RegionSize % EleSize == 0); > + > + return ValMgr.makeIntVal(RegionSize / EleSize, false); > + } > + > case MemRegion::StringRegionKind: { > const StringLiteral* Str = cast<StringRegion>(R)->getStringLiteral(); > // We intentionally made the size value signed because it participates > in > > Modified: cfe/trunk/lib/Analysis/ReturnPointerRangeChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReturnPointerRangeChecker.cpp?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/ReturnPointerRangeChecker.cpp (original) > +++ cfe/trunk/lib/Analysis/ReturnPointerRangeChecker.cpp Mon Jan 18 02:54:31 > 2010 > @@ -65,7 +65,8 @@ > // into a common place. > > DefinedOrUnknownSVal NumElements > - = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion()); > + = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(), > + > ER->getValueType(C.getASTContext())); > > const GRState *StInBound = state->AssumeInBound(Idx, NumElements, true); > const GRState *StOutBound = state->AssumeInBound(Idx, NumElements, false); > > Modified: cfe/trunk/test/Analysis/outofbound.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/outofbound.c?rev=93722&r1=93721&r2=93722&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/outofbound.c (original) > +++ cfe/trunk/test/Analysis/outofbound.c Mon Jan 18 02:54:31 2010 > @@ -1,7 +1,15 @@ > -// RUN: %clang_cc1 -analyze -analyzer-experimental-internal-checks > -checker-cfref -analyzer-store=region -verify %s > +// RUN: %clang_cc1 -analyze -analyzer-experimental-internal-checks > -analyzer-experimental-checks -checker-cfref -analyzer-store=region -verify %s > + > +typedef __typeof(sizeof(int)) size_t; > +void *malloc(size_t); > > char f1() { > char* s = "abcd"; > char c = s[4]; // no-warning > return s[5] + c; // expected-warning{{Access out-of-bound array element > (buffer overflow)}} > } > + > +void f2() { > + int *p = malloc(12); > + p[3] = 4; // expected-warning{{Access out-of-bound array element (buffer > overflow)}} > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
