[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 210458.
Charusso marked 4 inline comments as done.
Charusso added a comment.

- More fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

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,37 @@
+// 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. If any of the parameters has an
+//                          '__isl_' prefixed macro definition we escape every
+//                          of them when we are about to 'free()' something.
+
+#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); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+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"
@@ -359,6 +360,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,
@@ -877,6 +881,9 @@
       State = ProcessZeroAllocation(C, CE, 0, State);
       State = ProcessZeroAllocation(C, CE, 1, State);
     } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+      if (isNotModeled(CE, C))
+        return;
+
       State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
     } else if (FunI == II_strdup || FunI == II_win_strdup ||
                FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,34 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext &C) const {
+  if (CE->getNumArgs() == 0)
+    return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl()))
+    if (const Stmt *Body = FD->getBody())
+      if (Body->getBeginLoc().isValid())
+        FunctionStr =
+            Lexer::getSourceText(CharSourceRange::getTokenRange(
+                                     {FD->getBeginLoc(), Body->getBeginLoc()}),
+                                 C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+    return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+    if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+      if (const RefState *RS = State->get<RegionState>(Sym))
+        State = State->set<RegionState>(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
                                       const Stmt *S) const {
 
@@ -2833,7 +2868,6 @@
     if (const RefState *RS = State->get<RegionState>(sym)) {
       if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
           CheckRefState(RS)) {
-        State = State->remove<RegionState>(sym);
         State = State->set<RegionState>(sym, RefState::getEscaped(RS));
       }
     }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits