[analyzer] MacOSKeychainAPIChecker: Users of KeyChain API often use free() to deallocate the password. Catch this error explicitly and generate the error message at the place where free() is called.
llvm-svn: 138296
diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index ce149b0..086f365 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -68,9 +68,13 @@
const char* Name;
unsigned int Param;
unsigned int DeallocatorIdx;
+ /// The flag specifies if the call is valid or is a result of a common user
+ /// error (Ex: free instead of SecKeychainItemFreeContent), which we also
+ /// track for better diagnostics.
+ bool isValid;
};
static const unsigned InvalidIdx = 100000;
- static const unsigned FunctionsToTrackSize = 6;
+ static const unsigned FunctionsToTrackSize = 7;
static const ADFunctionInfo FunctionsToTrack[FunctionsToTrackSize];
/// The value, which represents no error return value for allocator functions.
static const unsigned NoErr = 0;
@@ -129,12 +133,13 @@
const MacOSKeychainAPIChecker::ADFunctionInfo
MacOSKeychainAPIChecker::FunctionsToTrack[FunctionsToTrackSize] = {
- {"SecKeychainItemCopyContent", 4, 3}, // 0
- {"SecKeychainFindGenericPassword", 6, 3}, // 1
- {"SecKeychainFindInternetPassword", 13, 3}, // 2
- {"SecKeychainItemFreeContent", 1, InvalidIdx}, // 3
- {"SecKeychainItemCopyAttributesAndData", 5, 5}, // 4
- {"SecKeychainItemFreeAttributesAndData", 1, InvalidIdx}, // 5
+ {"SecKeychainItemCopyContent", 4, 3, true}, // 0
+ {"SecKeychainFindGenericPassword", 6, 3, true}, // 1
+ {"SecKeychainFindInternetPassword", 13, 3, true}, // 2
+ {"SecKeychainItemFreeContent", 1, InvalidIdx, true}, // 3
+ {"SecKeychainItemCopyAttributesAndData", 5, 5, true}, // 4
+ {"SecKeychainItemFreeAttributesAndData", 1, InvalidIdx, true}, // 5
+ {"free", 0, InvalidIdx, false}, // 6
};
unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name,
@@ -257,6 +262,9 @@
if (idx == InvalidIdx)
return;
+ // We also track invalid deallocators. Ex: free() for enhanced error messages.
+ bool isValidDeallocator = FunctionsToTrack[idx].isValid;
+
// Check the argument to the deallocator.
const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param);
SVal ArgSVal = State->getSVal(ArgExpr);
@@ -280,7 +288,7 @@
// TODO: We might want a more precise diagnostic for double free
// (that would involve tracking all the freed symbols in the checker state).
const AllocationState *AS = State->get<AllocatedData>(ArgSM);
- if (!AS || RegionArgIsBad) {
+ if ((!AS || RegionArgIsBad) && isValidDeallocator) {
// It is possible that this is a false positive - the argument might
// have entered as an enclosing function parameter.
if (isEnclosingFunctionParam(ArgExpr))
@@ -305,7 +313,7 @@
// tracking the allocated symbol to avoid reporting a missing free after the
// deallocator mismatch error.
unsigned int PDeallocIdx = FunctionsToTrack[AS->AllocatorIdx].DeallocatorIdx;
- if (PDeallocIdx != idx) {
+ if (PDeallocIdx != idx || !isValidDeallocator) {
ExplodedNode *N = C.generateNode(State);
if (!N)
return;
@@ -313,7 +321,7 @@
llvm::SmallString<80> sbuf;
llvm::raw_svector_ostream os(sbuf);
- os << "Allocator doesn't match the deallocator: '"
+ os << "Deallocator doesn't match the allocator: '"
<< FunctionsToTrack[PDeallocIdx].Name << "' should be used.";
BugReport *Report = new BugReport(*BT, os.str(), N);
Report->addRange(ArgExpr->getSourceRange());