From: Anna Zaks Date: Fri, 5 Aug 2011 00:37:00 +0000 (+0000) Subject: KeychainAPI checker: Generate an error on double allocation. Pull out getAsPointeeMem... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ca0b57e07cfa029d4a6a061260727625bd833fd4;p=clang KeychainAPI checker: Generate an error on double allocation. Pull out getAsPointeeMemoryRegion so that it could be reused. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@136952 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 7e269dc020..d4fbce7198 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -127,11 +127,26 @@ unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name, return InvalidIdx; } +/// Given the address expression, retrieve the value it's pointing to. Assume +/// that value is itself an address, and return the corresponding MemRegion. +static const MemRegion *getAsPointeeMemoryRegion(const Expr *Expr, + CheckerContext &C) { + const GRState *State = C.getState(); + SVal ArgV = State->getSVal(Expr); + if (const loc::MemRegionVal *X = dyn_cast(&ArgV)) { + StoreManager& SM = C.getStoreManager(); + const MemRegion *V = SM.Retrieve(State->getStore(), *X).getAsRegion(); + return V; + } + return 0; +} + void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { const GRState *State = C.getState(); const Expr *Callee = CE->getCallee(); SVal L = State->getSVal(Callee); + unsigned idx = InvalidIdx; const FunctionDecl *funDecl = L.getAsFunctionDecl(); if (!funDecl) @@ -141,8 +156,32 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, return; StringRef funName = funI->getName(); - // If a value has been freed, remove from the list. - unsigned idx = getTrackedFunctionIndex(funName, false); + // If it is a call to an allocator function, it could be a double allocation. + idx = getTrackedFunctionIndex(funName, true); + if (idx != InvalidIdx) { + const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param); + if (const MemRegion *V = getAsPointeeMemoryRegion(ArgExpr, C)) + if (const AllocationState *AS = State->get(V)) { + ExplodedNode *N = C.generateSink(State); + if (!N) + return; + initBugType(); + std::string sbuf; + llvm::raw_string_ostream os(sbuf); + unsigned int DIdx = FunctionsToTrack[AS->AllocatorIdx].DeallocatorIdx; + os << "Allocated data should be released before another call to " + << "the allocator: missing a call to '" + << FunctionsToTrack[DIdx].Name + << "'."; + RangedBugReport *Report = new RangedBugReport(*BT, os.str(), N); + Report->addRange(ArgExpr->getSourceRange()); + C.EmitReport(Report); + } + return; + } + + // Is it a call to one of deallocator functions? + idx = getTrackedFunctionIndex(funName, false); if (idx == InvalidIdx) return; @@ -188,7 +227,8 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, return; } - // Continue exploring from the new state. + // If a value has been freed, remove it from the list and continue exploring + // from the new state. State = State->remove(Arg); C.addTransition(State); } @@ -198,7 +238,6 @@ void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE, const GRState *State = C.getState(); const Expr *Callee = CE->getCallee(); SVal L = State->getSVal(Callee); - StoreManager& SM = C.getStoreManager(); const FunctionDecl *funDecl = L.getAsFunctionDecl(); if (!funDecl) @@ -214,19 +253,13 @@ void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE, return; const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param); - SVal Arg = State->getSVal(ArgExpr); - if (const loc::MemRegionVal *X = dyn_cast(&Arg)) { - // Add the symbolic value, which represents the location of the allocated - // data, to the set. - const MemRegion *V = SM.Retrieve(State->getStore(), *X).getAsRegion(); - // If this is not a region, it can be: + if (const MemRegion *V = getAsPointeeMemoryRegion(ArgExpr, C)) { + // If the argument points to something that's not a region, it can be: // - unknown (cannot reason about it) // - undefined (already reported by other checker) // - constant (null - should not be tracked, // other constant will generate a compiler warning) // - goto (should be reported by other checker) - if (!V) - return; // We only need to track the value if the function returned noErr(0), so // bind the return value of the function to 0 and proceed from the no error @@ -234,6 +267,8 @@ void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE, SValBuilder &Builder = C.getSValBuilder(); SVal ZeroVal = Builder.makeIntVal(0, CE->getCallReturnType()); const GRState *NoErr = State->BindExpr(CE, ZeroVal); + // Add the symbolic value V, which represents the location of the allocated + // data, to the set. NoErr = NoErr->set(V, AllocationState(ArgExpr, idx)); assert(NoErr); C.addTransition(NoErr); diff --git a/test/Analysis/keychainAPI.m b/test/Analysis/keychainAPI.m index af0258a59b..6a9a75330d 100644 --- a/test/Analysis/keychainAPI.m +++ b/test/Analysis/keychainAPI.m @@ -93,11 +93,13 @@ void fooDoNotReportNull() { void doubleAlloc() { unsigned int *ptr = 0; OSStatus st = 0; - UInt32 *length = 0; - void **outData = 0; - SecKeychainItemCopyContent(2, ptr, ptr, length, outData); - SecKeychainItemCopyContent(2, ptr, ptr, length, outData); -}// no-warning + UInt32 length; + void *outData; + st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &outData); + st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &outData); // expected-warning {{Allocated data should be released before another call to the allocator:}} + if (st == noErr) + SecKeychainItemFreeContent(ptr, outData); +} void fooOnlyFree() { unsigned int *ptr = 0;