Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

Integer Set Library using retain-count based allocation which is not
modeled in MallocChecker.


Repository:
  rC Clang

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//                          based allocation.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+    return;
+
+  free(O);
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+    return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -41,6 +42,7 @@
 // Used to check correspondence between allocators and deallocators.
 enum AllocationFamily {
   AF_None,
+  AF_NotModeled,
   AF_Malloc,
   AF_CXXNew,
   AF_CXXNewArray,
@@ -50,19 +52,22 @@
 };
 
 class RefState {
-  enum Kind { // Reference to allocated memory.
-              Allocated,
-              // Reference to zero-allocated memory.
-              AllocatedOfSizeZero,
-              // Reference to released/freed memory.
-              Released,
-              // The responsibility for freeing resources has transferred from
-              // this reference. A relinquished symbol should not be freed.
-              Relinquished,
-              // We are no longer guaranteed to have observed all manipulations
-              // of this pointer/memory. For example, it could have been
-              // passed as a parameter to an opaque function.
-              Escaped
+  enum Kind {
+    // If this checker does not model the allocation.
+    DoNothing,
+    // Reference to allocated memory.
+    Allocated,
+    // Reference to zero-allocated memory.
+    AllocatedOfSizeZero,
+    // Reference to released/freed memory.
+    Released,
+    // The responsibility for freeing resources has transferred from
+    // this reference. A relinquished symbol should not be freed.
+    Relinquished,
+    // We are no longer guaranteed to have observed all manipulations
+    // of this pointer/memory. For example, it could have been
+    // passed as a parameter to an opaque function.
+    Escaped
   };
 
   const Stmt *S;
@@ -75,6 +80,7 @@
     assert(family != AF_None);
   }
 public:
+  bool isDoNothing() const { return K == DoNothing; }
   bool isAllocated() const { return K == Allocated; }
   bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; }
   bool isReleased() const { return K == Released; }
@@ -105,6 +111,9 @@
   static RefState getEscaped(const RefState *RS) {
     return RefState(Escaped, RS->getStmt(), RS->getAllocationFamily());
   }
+  static RefState getDoNothing(const Stmt *S) {
+    return RefState(DoNothing, S, AF_NotModeled);
+  }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddInteger(K);
@@ -115,6 +124,7 @@
   void dump(raw_ostream &OS) const {
     switch (static_cast<Kind>(K)) {
 #define CASE(ID) case ID: OS << #ID; break;
+    CASE(DoNothing)
     CASE(Allocated)
     CASE(AllocatedOfSizeZero)
     CASE(Released)
@@ -359,6 +369,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext &C) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext &C) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
@@ -822,13 +835,16 @@
 }
 
 void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
-  if (C.wasInlined)
-    return;
-
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD)
     return;
 
+  if (isNotModeled(CE, C))
+    return;
+
+  if (C.wasInlined)
+    return;
+
   ProgramStateRef State = C.getState();
   bool ReleasedAllocatedMemory = false;
 
@@ -1460,6 +1476,7 @@
     case AF_CXXNewArray: os << "'new[]'"; return;
     case AF_IfNameIndex: os << "'if_nameindex()'"; return;
     case AF_InnerBuffer: os << "container-specific allocator"; return;
+    case AF_NotModeled: os << "not modeled allocator"; return;
     case AF_Alloca:
     case AF_None: llvm_unreachable("not a deallocation expression");
   }
@@ -1473,6 +1490,7 @@
     case AF_CXXNewArray: os << "'delete[]'"; return;
     case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
     case AF_InnerBuffer: os << "container-specific deallocator"; return;
+    case AF_NotModeled: os << "not modeled deallocator"; return;
     case AF_Alloca:
     case AF_None: llvm_unreachable("suspicious argument");
   }
@@ -1662,6 +1680,8 @@
       return CK_InnerPointerChecker;
     return None;
   }
+  case AF_NotModeled:
+    return None;
   case AF_None: {
     llvm_unreachable("no family");
   }
@@ -2532,6 +2552,44 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext &C) const {
+  if (CE->getNumArgs() == 0)
+    return false;
+
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  const Stmt *Body = FD->getBody();
+  if (!Body)
+    return false;
+
+  StringRef FunctionStr = Lexer::getSourceText(
+      CharSourceRange::getTokenRange({FD->getBeginLoc(), Body->getBeginLoc()}),
+      C.getSourceManager(), C.getLangOpts());
+
+  // We do not model Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+    return false;
+
+  ProgramStateRef State = C.getState();
+  const LocationContext *LCtx = C.getLocationContext();
+
+  for (const auto &Arg : CE->arguments()) {
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreImpCasts())) {
+      if (const auto *VD = dyn_cast<ParmVarDecl>(DRE->getDecl())) {
+        SVal V = State->getSVal(State->getLValue(VD, LCtx));
+        if (SymbolRef Sym = V.getAsSymbol()) {
+          const RefState *RS = State->get<RegionState>(Sym);
+          if (!RS) {
+            State = State->set<RegionState>(Sym, RefState::getDoNothing(Arg));
+            C.addTransition(State);
+          }
+        }
+      }
+    }
+  }
+
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
                                       const Stmt *S) const {
 
@@ -2876,9 +2934,15 @@
   ProgramStateRef statePrev = N->getFirstPred()->getState();
 
   const RefState *RS = state->get<RegionState>(Sym);
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+
+  if (RS && RS->isDoNothing()) {
+    BR.markInvalid(getTag(), S);
+    return nullptr;
+  }
+
   const RefState *RSPrev = statePrev->get<RegionState>(Sym);
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
   // When dealing with containers, we sometimes want to give a note
   // even if the statement is missing.
   if (!S && (!RS || RS->getAllocationFamily() != AF_InnerBuffer))
@@ -2963,6 +3027,8 @@
           Msg = OS.str();
           break;
         }
+        case AF_NotModeled:
+          llvm_unreachable("The report should be invalidated at this point.");
         case AF_None:
           llvm_unreachable("Unhandled allocation family!");
       }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to