diff options
-rw-r--r-- | include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h | 7 | ||||
-rw-r--r-- | include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h | 6 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 18 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/MemRegion.cpp | 4 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/SValBuilder.cpp | 12 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp | 43 | ||||
-rw-r--r-- | test/Analysis/malloc.c | 51 |
7 files changed, 110 insertions, 31 deletions
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h index 5034470781..e76c20e60e 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -1130,8 +1130,11 @@ public: const CXXThisRegion *getCXXThisRegion(QualType thisPointerTy, const LocationContext *LC); - /// getSymbolicRegion - Retrieve or create a "symbolic" memory region. - const SymbolicRegion* getSymbolicRegion(SymbolRef sym); + /// \brief Retrieve or create a "symbolic" memory region. + const SymbolicRegion* getSymbolicRegion(SymbolRef Sym); + + /// \brief Return a unique symbolic region belonging to heap memory space. + const SymbolicRegion *getSymbolicHeapRegion(SymbolRef sym); const StringRegion *getStringRegion(const StringLiteral* Str); diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h index 8856c825d9..24aaa6f41b 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -182,6 +182,12 @@ public: const LocationContext *LCtx, QualType type, unsigned visitCount); + /// \brief Conjure a symbol representing heap allocated memory region. + /// + /// Note, the expression should represent a location. + DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E, + const LocationContext *LCtx, + unsigned Count); DefinedOrUnknownSVal getDerivedRegionValueSymbolVal( SymbolRef parentSymbol, const TypedValueRegion *region); diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 3b9712fd9b..c22c6a2687 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -477,19 +477,27 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, const CallExpr *CE, SVal Size, SVal Init, ProgramStateRef state) { - // Get the return value. - SVal retVal = state->getSVal(CE, C.getLocationContext()); + + // Bind the return value to the symbolic value from the heap region. + // TODO: We could rewrite post visit to eval call; 'malloc' does not have + // side effects other than what we model here. + unsigned Count = C.getCurrentBlockCount(); + SValBuilder &svalBuilder = C.getSValBuilder(); + const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); + DefinedSVal RetVal = + cast<DefinedSVal>(svalBuilder.getConjuredHeapSymbolVal(CE, LCtx, Count)); + state = state->BindExpr(CE, C.getLocationContext(), RetVal); // We expect the malloc functions to return a pointer. - if (!isa<Loc>(retVal)) + if (!isa<Loc>(RetVal)) return 0; // Fill the region with the initialization value. - state = state->bindDefault(retVal, Init); + state = state->bindDefault(RetVal, Init); // Set the region's extent equal to the Size parameter. const SymbolicRegion *R = - dyn_cast_or_null<SymbolicRegion>(retVal.getAsRegion()); + dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion()); if (!R) return 0; if (isa<DefinedOrUnknownSVal>(Size)) { diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp index 96905c081e..bcf7c3fe7d 100644 --- a/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -849,6 +849,10 @@ const SymbolicRegion *MemRegionManager::getSymbolicRegion(SymbolRef sym) { return getSubRegion<SymbolicRegion>(sym, getUnknownRegion()); } +const SymbolicRegion *MemRegionManager::getSymbolicHeapRegion(SymbolRef Sym) { + return getSubRegion<SymbolicRegion>(Sym, getHeapRegion()); +} + const FieldRegion* MemRegionManager::getFieldRegion(const FieldDecl *d, const MemRegion* superRegion){ diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp index 765ae48c73..c391489beb 100644 --- a/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -148,6 +148,18 @@ SValBuilder::getConjuredSymbolVal(const Stmt *stmt, return nonloc::SymbolVal(sym); } +DefinedOrUnknownSVal +SValBuilder::getConjuredHeapSymbolVal(const Expr *E, + const LocationContext *LCtx, + unsigned VisitCount) { + QualType T = E->getType(); + assert(Loc::isLocType(T)); + assert(SymbolManager::canSymbolicate(T)); + + SymbolRef sym = SymMgr.getConjuredSymbol(E, LCtx, T, VisitCount); + return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym)); +} + DefinedSVal SValBuilder::getMetadataSymbolVal(const void *symbolTag, const MemRegion *region, const Expr *expr, QualType type, diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 29c65f8f3e..9cbbece98e 100644 --- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -673,11 +673,18 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state, // regions, though. return UnknownVal(); - // If both values wrap regions, see if they're from different base regions. + const MemSpaceRegion *LeftMS = LeftMR->getMemorySpace(); + const MemSpaceRegion *RightMS = RightMR->getMemorySpace(); + const MemSpaceRegion *UnknownMS = MemMgr.getUnknownRegion(); const MemRegion *LeftBase = LeftMR->getBaseRegion(); const MemRegion *RightBase = RightMR->getBaseRegion(); - if (LeftBase != RightBase && - !isa<SymbolicRegion>(LeftBase) && !isa<SymbolicRegion>(RightBase)) { + + // If the two regions are from different known memory spaces they cannot be + // equal. Also, assume that no symbolic region (whose memory space is + // unknown) is on the stack. + if (LeftMS != RightMS && + ((LeftMS != UnknownMS && RightMS != UnknownMS) || + (isa<StackSpaceRegion>(LeftMS) || isa<StackSpaceRegion>(RightMS)))) { switch (op) { default: return UnknownVal(); @@ -688,24 +695,20 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state, } } - // The two regions are from the same base region. See if they're both a - // type of region we know how to compare. - const MemSpaceRegion *LeftMS = LeftBase->getMemorySpace(); - const MemSpaceRegion *RightMS = RightBase->getMemorySpace(); - - // Heuristic: assume that no symbolic region (whose memory space is - // unknown) is on the stack. - // FIXME: we should be able to be more precise once we can do better - // aliasing constraints for symbolic regions, but this is a reasonable, - // albeit unsound, assumption that holds most of the time. - if (isa<StackSpaceRegion>(LeftMS) ^ isa<StackSpaceRegion>(RightMS)) { + // If both values wrap regions, see if they're from different base regions. + // Note, heap base symbolic regions are assumed to not alias with + // each other; for example, we assume that malloc returns different address + // on each invocation. + if (LeftBase != RightBase && + ((!isa<SymbolicRegion>(LeftBase) && !isa<SymbolicRegion>(RightBase)) || + isa<HeapSpaceRegion>(LeftMS)) ){ switch (op) { - default: - break; - case BO_EQ: - return makeTruthVal(false, resultTy); - case BO_NE: - return makeTruthVal(true, resultTy); + default: + return UnknownVal(); + case BO_EQ: + return makeTruthVal(false, resultTy); + case BO_NE: + return makeTruthVal(true, resultTy); } } diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index d0d095b1d4..bdbd96e2be 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -839,10 +839,8 @@ int fPtr(unsigned cond, int x) { return (cond ? mySub : myAdd)(x, x); } -// ---------------------------------------------------------------------------- -// Below are the known false positives. +// Test anti-aliasing. -// TODO: There should be no warning here. This one might be difficult to get rid of. void dependsOnValueOfPtr(int *g, unsigned f) { int *p; @@ -855,10 +853,55 @@ void dependsOnValueOfPtr(int *g, unsigned f) { if (p != g) free(p); else - return; // expected-warning{{Memory is never released; potential leak}} + return; // no warning return; } +int CMPRegionHeapToStack() { + int x = 0; + int *x1 = malloc(8); + int *x2 = &x; + if (x1 == x2) + return 5/x; // expected-warning{{This statement is never executed}} + free(x1); + return x; +} + +int CMPRegionHeapToHeap2() { + int x = 0; + int *x1 = malloc(8); + int *x2 = malloc(8); + int *x4 = x1; + int *x5 = x2; + if (x4 == x5) + return 5/x; // expected-warning{{This statement is never executed}} + free(x1); + free(x2); + return x; +} + +int CMPRegionHeapToHeap() { + int x = 0; + int *x1 = malloc(8); + int *x4 = x1; + if (x1 == x4) { + free(x1); + return 5/x; // expected-warning{{Division by zero}} + } + return x;// expected-warning{{This statement is never executed}} +} + +int HeapAssignment() { + int m = 0; + int *x = malloc(4); + int *y = x; + *x = 5; + if (*x != *y) + return 5/m; // expected-warning{{This statement is never executed}} + free(x); + return 0; +} + // ---------------------------------------------------------------------------- // False negatives. |