[cfe-commits] r138479 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp test/Analysis/keychainAPI-diagnostic-visitor.m

Anna Zaks ganna at apple.com
Wed Aug 24 13:52:46 PDT 2011


Author: zaks
Date: Wed Aug 24 15:52:46 2011
New Revision: 138479

URL: http://llvm.org/viewvc/llvm-project?rev=138479&view=rev
Log:
[analyzer] MacOSKeychainAPIChecker: Provide reacher diagnostic trace by pointing to the allocation site when reporting a leak.

Added:
    cfe/trunk/test/Analysis/keychainAPI-diagnostic-visitor.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp?rev=138479&r1=138478&r2=138479&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp Wed Aug 24 15:52:46 2011
@@ -62,7 +62,10 @@
   void checkEndPath(EndOfFunctionNodeBuilder &B, ExprEngine &Eng) const;
 
 private:
-  enum APIKind{
+  typedef std::pair<SymbolRef, const AllocationState&> AllocationPair;
+  typedef llvm::SmallVector<AllocationPair, 2> AllocationPairVec;
+
+  enum APIKind {
     /// Denotes functions tracked by this checker.
     ValidAPI = 0,
     /// The functions commonly/mistakenly used in place of the given API.
@@ -87,7 +90,7 @@
 
   /// Given the function name, returns the index of the allocator/deallocator
   /// function.
-  unsigned getTrackedFunctionIndex(StringRef Name, bool IsAllocator) const;
+  static unsigned getTrackedFunctionIndex(StringRef Name, bool IsAllocator);
 
   inline void initBugType() const {
     if (!BT)
@@ -99,7 +102,7 @@
                                          CheckerContext &C,
                                          SymbolRef ArgSM) const;
 
-  BugReport *generateAllocatedDataNotReleasedReport(const AllocationState &AS,
+  BugReport *generateAllocatedDataNotReleasedReport(const AllocationPair &AP,
                                                     ExplodedNode *N) const;
 
   /// Check if RetSym evaluates to an error value in the current state.
@@ -115,6 +118,29 @@
     return definitelyReturnedError(RetSym, State, Builder, true);
   }
 
+  /// The bug visitor which allows us to print extra diagnostics along the
+  /// BugReport path. For example, showing the allocation site of the leaked
+  /// region.
+  class SecKeychainBugVisitor : public BugReporterVisitor {
+  protected:
+    // The allocated region symbol tracked by the main analysis.
+    SymbolRef Sym;
+
+  public:
+    SecKeychainBugVisitor(SymbolRef S) : Sym(S) {}
+    virtual ~SecKeychainBugVisitor() {}
+
+    void Profile(llvm::FoldingSetNodeID &ID) const {
+      static int X = 0;
+      ID.AddPointer(&X);
+      ID.AddPointer(Sym);
+    }
+
+    PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
+                                   const ExplodedNode *PrevN,
+                                   BugReporterContext &BRC,
+                                   BugReport &BR);
+  };
 };
 }
 
@@ -155,7 +181,7 @@
 };
 
 unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name,
-                                                       bool IsAllocator) const {
+                                                          bool IsAllocator) {
   for (unsigned I = 0; I < FunctionsToTrackSize; ++I) {
     ADFunctionInfo FI = FunctionsToTrack[I];
     if (FI.Name != Name)
@@ -475,19 +501,20 @@
   C.addTransition(state);
 }
 
-// TODO: The report has to mention the expression which contains the
-// allocated content as well as the point at which it has been allocated.
 BugReport *MacOSKeychainAPIChecker::
-  generateAllocatedDataNotReleasedReport(const AllocationState &AS,
+  generateAllocatedDataNotReleasedReport(const AllocationPair &AP,
                                          ExplodedNode *N) const {
+  const AllocationState &AS = AP.second;
   const ADFunctionInfo &FI = FunctionsToTrack[AS.AllocatorIdx];
   initBugType();
   llvm::SmallString<70> sbuf;
   llvm::raw_svector_ostream os(sbuf);
+
   os << "Allocated data is not released: missing a call to '"
       << FunctionsToTrack[FI.DeallocatorIdx].Name << "'.";
   BugReport *Report = new BugReport(*BT, os.str(), N);
-  Report->addRange(AS.Address->getSourceRange());
+  Report->addVisitor(new SecKeychainBugVisitor(AP.first));
+  Report->addRange(SourceRange());
   return Report;
 }
 
@@ -499,7 +526,7 @@
     return;
 
   bool Changed = false;
-  llvm::SmallVector<const AllocationState*, 1> Errors;
+  AllocationPairVec Errors;
   for (AllocatedSetTy::iterator I = ASet.begin(), E = ASet.end(); I != E; ++I) {
     if (SR.isLive(I->first))
       continue;
@@ -511,7 +538,7 @@
     if (State->getSymVal(I->first) ||
         definitelyReturnedError(I->second.RetValue, State, C.getSValBuilder()))
       continue;
-    Errors.push_back(&I->second);
+    Errors.push_back(std::make_pair(I->first, I->second));
   }
   if (!Changed)
     return;
@@ -522,9 +549,9 @@
     return;
 
   // Generate the error reports.
-  for (llvm::SmallVector<const AllocationState*, 3>::iterator
-      I = Errors.begin(), E = Errors.end(); I != E; ++I) {
-    C.EmitReport(generateAllocatedDataNotReleasedReport(**I, N));
+  for (AllocationPairVec::iterator I = Errors.begin(), E = Errors.end();
+                                                       I != E; ++I) {
+    C.EmitReport(generateAllocatedDataNotReleasedReport(*I, N));
   }
 }
 
@@ -539,7 +566,7 @@
   // Anything which has been allocated but not freed (nor escaped) will be
   // found here, so report it.
   bool Changed = false;
-  llvm::SmallVector<const AllocationState*, 1> Errors;
+  AllocationPairVec Errors;
   for (AllocatedSetTy::iterator I = AS.begin(), E = AS.end(); I != E; ++I ) {
     Changed = true;
     state = state->remove<AllocatedData>(I->first);
@@ -550,7 +577,7 @@
                                 Eng.getSValBuilder())) {
       continue;
     }
-    Errors.push_back(&I->second);
+    Errors.push_back(std::make_pair(I->first, I->second));
   }
 
   // If no change, do not generate a new state.
@@ -562,12 +589,40 @@
     return;
 
   // Generate the error reports.
-  for (llvm::SmallVector<const AllocationState*, 3>::iterator
-      I = Errors.begin(), E = Errors.end(); I != E; ++I) {
+  for (AllocationPairVec::iterator I = Errors.begin(), E = Errors.end();
+                                                       I != E; ++I) {
     Eng.getBugReporter().EmitReport(
-      generateAllocatedDataNotReleasedReport(**I, N));
+      generateAllocatedDataNotReleasedReport(*I, N));
   }
+}
+
 
+PathDiagnosticPiece *MacOSKeychainAPIChecker::SecKeychainBugVisitor::VisitNode(
+                                                      const ExplodedNode *N,
+                                                      const ExplodedNode *PrevN,
+                                                      BugReporterContext &BRC,
+                                                      BugReport &BR) {
+  const AllocationState *AS = N->getState()->get<AllocatedData>(Sym);
+  if (!AS)
+    return 0;
+  const AllocationState *ASPrev = PrevN->getState()->get<AllocatedData>(Sym);
+  if (ASPrev)
+    return 0;
+
+  // (!ASPrev && AS) ~ We started tracking symbol in node N, it must be the
+  // allocation site.
+  const CallExpr *CE = cast<CallExpr>(cast<StmtPoint>(N->getLocation())
+                                                            .getStmt());
+  const FunctionDecl *funDecl = CE->getDirectCallee();
+  assert(funDecl && "We do not support indirect function calls as of now.");
+  StringRef funName = funDecl->getName();
+
+  // Get the expression of the corresponding argument.
+  unsigned Idx = getTrackedFunctionIndex(funName, true);
+  assert(Idx != InvalidIdx && "This should be a call to an allocator.");
+  const Expr *ArgExpr = CE->getArg(FunctionsToTrack[Idx].Param);
+  PathDiagnosticLocation Pos(ArgExpr, BRC.getSourceManager());
+  return new PathDiagnosticEventPiece(Pos, "Data is allocated here.");
 }
 
 void ento::registerMacOSKeychainAPIChecker(CheckerManager &mgr) {

Added: cfe/trunk/test/Analysis/keychainAPI-diagnostic-visitor.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/keychainAPI-diagnostic-visitor.m?rev=138479&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/keychainAPI-diagnostic-visitor.m (added)
+++ cfe/trunk/test/Analysis/keychainAPI-diagnostic-visitor.m Wed Aug 24 15:52:46 2011
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=osx.SecKeychainAPI -analyzer-store=region -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the default SecKeychainAPI checker.
+
+typedef unsigned int OSStatus;
+typedef unsigned int SecKeychainAttributeList;
+typedef unsigned int SecKeychainItemRef;
+typedef unsigned int SecItemClass;
+typedef unsigned int UInt32;
+enum {
+    noErr                      = 0,
+    GenericError               = 1
+};
+OSStatus SecKeychainItemCopyContent (
+                                     SecKeychainItemRef itemRef,
+                                     SecItemClass *itemClass,
+                                     SecKeychainAttributeList *attrList,
+                                     UInt32 *length,
+                                     void **outData
+                                     );
+
+void DellocWithCFStringCreate4() {
+    unsigned int *ptr = 0;
+    OSStatus st = 0;
+    UInt32 length;
+    char *bytes;
+    char *x;
+    st = SecKeychainItemCopyContent(2, ptr, ptr, &length, (void **)&bytes); // expected-note {{Data is allocated here}}
+    x = bytes;
+    if (st == noErr) // expected-note {{Assuming 'st' is equal to noErr}} // expected-note{{Taking true branch}}
+        x = bytes;;
+  
+    length++; // expected-warning {{Allocated data is not released}} // expected-note{{Allocated data is not released}}
+}
+





More information about the cfe-commits mailing list