cjdb updated this revision to Diff 323414.
cjdb added a comment.

fixes block expressions

@aaron.ballman I'm not sure I follow what you're after re lambdas. Could you 
please provide a test case that I can work with?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94640

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/free.c
  clang/test/Analysis/free.cpp
  clang/test/Analysis/weak-functions.c

Index: clang/test/Analysis/weak-functions.c
===================================================================
--- clang/test/Analysis/weak-functions.c
+++ clang/test/Analysis/weak-functions.c
@@ -71,7 +71,9 @@
 void free(void *) __attribute__((weak_import));
 
 void t10 () {
-  free((void*)&t10); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  free((void*)&t10);
+  // expected-warning@-1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 't10'}}
 }
 
 //===----------------------------------------------------------------------===
Index: clang/test/Analysis/free.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/free.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc
+//
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
+namespace std {
+  using size_t = decltype(sizeof(int));
+  void free(void *);
+}
+
+extern "C" void free(void *);
+extern "C" void *alloca(std::size_t);
+
+void t1a () {
+  int a[] = { 1 };
+  free(a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t1b () {
+  int a[] = { 1 };
+  std::free(a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t2a () {
+  int a = 1;
+  free(&a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t2b () {
+  int a = 1;
+  std::free(&a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t3a () {
+  static int a[] = { 1 };
+  free(a);
+  // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t3b () {
+  static int a[] = { 1 };
+  std::free(a);
+  // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t4a (char *x) {
+  free(x); // no-warning
+}
+
+void t4b (char *x) {
+  std::free(x); // no-warning
+}
+
+void t5a () {
+  extern char *ptr();
+  free(ptr()); // no-warning
+}
+
+void t5b () {
+  extern char *ptr();
+  std::free(ptr()); // no-warning
+}
+
+void t6a () {
+  free((void*)1000);
+  // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)1000'}}
+}
+
+void t6b () {
+  std::free((void*)1000);
+  // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object '(void *)1000'}}
+}
+
+void t7a (char **x) {
+  free(*x); // no-warning
+}
+
+void t7b (char **x) {
+  std::free(*x); // no-warning
+}
+
+void t8a (char **x) {
+  // ugh
+  free((*x)+8); // no-warning
+}
+
+void t8b (char **x) {
+  // ugh
+  std::free((*x)+8); // no-warning
+}
+
+void t9a () {
+label:
+  free(&&label);
+  // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'label'}}
+}
+
+void t9b () {
+label:
+  std::free(&&label);
+  // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'label'}}
+}
+
+void t10a () {
+  free((void*)&t10a);
+  // expected-warning@-1{{Argument to free() is the address of the function 't10a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 't10a'}}
+}
+
+void t10b () {
+  std::free((void*)&t10b);
+  // expected-warning@-1{{Argument to free() is the address of the function 't10b', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 't10b'}}
+}
+
+void t11a () {
+  char *p = (char*)alloca(2);
+  free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void t11b () {
+  char *p = (char*)alloca(2);
+  std::free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void t12a () {
+  char *p = (char*)__builtin_alloca(2);
+  free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void t12b () {
+  char *p = (char*)__builtin_alloca(2);
+  std::free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void t13a () {
+  free(^{return;});
+  // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object: block expression}}
+}
+
+void t13b () {
+  std::free(^{return;});
+  // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object: block expression}}
+}
+
+void t14a () {
+  free((void *)+[]{ return; });
+  // expected-warning@-1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object (lambda-to-function-pointer conversion)}}
+}
+
+void t14b () {
+  std::free((void *)+[]{ return; });
+  // expected-warning@-1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object (lambda-to-function-pointer conversion)}}
+}
+
+void t15a (char a) {
+  free(&a);
+  // expected-warning@-1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t15b (char a) {
+  std::free(&a);
+  // expected-warning@-1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+static int someGlobal[2];
+void t16a () {
+  free(someGlobal);
+  // expected-warning@-1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'someGlobal'}}
+}
+
+void t16b () {
+  std::free(someGlobal);
+  // expected-warning@-1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'someGlobal'}}
+}
+
+void t17a (char **x, int offset) {
+  // Unknown value
+  free(x[offset]); // no-warning
+}
+
+void t17b (char **x, int offset) {
+  // Unknown value
+  std::free(x[offset]); // no-warning
+}
Index: clang/test/Analysis/free.c
===================================================================
--- clang/test/Analysis/free.c
+++ clang/test/Analysis/free.c
@@ -41,7 +41,9 @@
 }
 
 void t6 () {
-  free((void*)1000); // expected-warning {{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+  free((void*)1000);
+  // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)1000'}}
 }
 
 void t7 (char **x) {
@@ -55,11 +57,15 @@
 
 void t9 () {
 label:
-  free(&&label); // expected-warning {{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+  free(&&label);
+  // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'label'}}
 }
 
 void t10 () {
-  free((void*)&t10); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  free((void*)&t10);
+  // expected-warning@-1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 't10'}}
 }
 
 void t11 () {
@@ -73,7 +79,9 @@
 }
 
 void t13 () {
-  free(^{return;}); // expected-warning {{Argument to free() is a block, which is not memory allocated by malloc()}}
+  free(^{return;});
+  // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object: block expression}}
 }
 
 void t14 (char a) {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10249,34 +10249,24 @@
 }
 
 namespace {
-void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
-                                const UnaryOperator *UnaryExpr,
-                                const VarDecl *Var) {
-  StorageClass Class = Var->getStorageClass();
-  if (Class == StorageClass::SC_Extern ||
-      Class == StorageClass::SC_PrivateExtern ||
-      Var->getType()->isReferenceType())
-    return;
-
-  S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
-      << CalleeName << Var;
-}
+constexpr int Placeholder = 0;
 
 void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
                                 const UnaryOperator *UnaryExpr, const Decl *D) {
-  if (const auto *Field = dyn_cast<FieldDecl>(D))
+  if (isa<FieldDecl, FunctionDecl, VarDecl>(D)) {
     S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
-        << CalleeName << Field;
+        << CalleeName << Placeholder << cast<NamedDecl>(D);
+    return;
+  }
 }
 
 void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName,
                                  const UnaryOperator *UnaryExpr) {
-  if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf)
-    return;
-
-  if (const auto *Lvalue = dyn_cast<DeclRefExpr>(UnaryExpr->getSubExpr()))
-    if (const auto *Var = dyn_cast<VarDecl>(Lvalue->getDecl()))
-      return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr, Var);
+  if (const auto *Lvalue = dyn_cast<DeclRefExpr>(UnaryExpr->getSubExpr())) {
+    const Decl *D = Lvalue->getDecl();
+    if (isa<VarDecl, FunctionDecl>(D))
+      return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr, D);
+  }
 
   if (const auto *Lvalue = dyn_cast<MemberExpr>(UnaryExpr->getSubExpr()))
     return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr,
@@ -10293,21 +10283,70 @@
     return;
 
   S.Diag(Lvalue->getBeginLoc(), diag::warn_free_nonheap_object)
-      << CalleeName << Var;
+      << CalleeName << Placeholder << Var;
+}
+
+void CheckFreeArgumentsCast(Sema &S, const std::string &CalleeName,
+                            const CastExpr *Cast) {
+  SmallString<128> SizeString;
+  llvm::raw_svector_ostream OS(SizeString);
+  switch (Cast->getCastKind()) {
+  case clang::CK_BitCast:
+    if (!Cast->getSubExpr()->getType()->isFunctionPointerType())
+      return;
+
+    OS << "(lambda-to-function-pointer conversion)";
+    break;
+  case clang::CK_IntegralToPointer:
+  case clang::CK_FunctionToPointerDecay:
+    OS << '\'';
+    Cast->printPretty(OS, nullptr, S.getPrintingPolicy());
+    OS << '\'';
+    break;
+  default:
+    return;
+  }
+
+  S.Diag(Cast->getBeginLoc(), diag::warn_free_nonheap_object)
+      << CalleeName << Placeholder << OS.str();
 }
 } // namespace
 
 /// Alerts the user that they are attempting to free a non-malloc'd object.
 void Sema::CheckFreeArguments(const CallExpr *E) {
-  const Expr *Arg = E->getArg(0)->IgnoreParenCasts();
   const std::string CalleeName =
       dyn_cast<FunctionDecl>(E->getCalleeDecl())->getQualifiedNameAsString();
 
-  if (const auto *UnaryExpr = dyn_cast<UnaryOperator>(Arg))
-    return CheckFreeArgumentsAddressof(*this, CalleeName, UnaryExpr);
+  { // Prefer something that doesn't involve a cast to make things simpler.
+    const Expr *Arg = E->getArg(0)->IgnoreParenCasts();
+    if (const auto *UnaryExpr = dyn_cast<UnaryOperator>(Arg))
+      if (UnaryExpr->getOpcode() == UnaryOperator::Opcode::UO_AddrOf)
+        return CheckFreeArgumentsAddressof(*this, CalleeName, UnaryExpr);
+
+    if (const auto *Lvalue = dyn_cast<DeclRefExpr>(Arg))
+      return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue);
 
-  if (const auto *Lvalue = dyn_cast<DeclRefExpr>(Arg))
-    return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue);
+    if (const auto *Label = dyn_cast<AddrLabelExpr>(Arg)) {
+      Diag(Label->getBeginLoc(), diag::warn_free_nonheap_object)
+          << CalleeName << Placeholder << Label->getLabel()->getIdentifier();
+      return;
+    }
+
+    if (isa<BlockExpr, LambdaExpr>(Arg)) {
+      SmallString<128> SizeString;
+      llvm::raw_svector_ostream OS(SizeString);
+      OS << '\'';
+      Arg->printPretty(OS, nullptr, getPrintingPolicy());
+      OS << '\'';
+      static constexpr int NoPlaceholder = 1;
+      Diag(Arg->getBeginLoc(), diag::warn_free_nonheap_object)
+          << CalleeName << NoPlaceholder;
+      return;
+    }
+  }
+  // Maybe the cast was important, check after the other cases.
+  if (const auto *Cast = dyn_cast<CastExpr>(E->getArg(0)))
+    return CheckFreeArgumentsCast(*this, CalleeName, Cast);
 }
 
 void
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7638,7 +7638,7 @@
   "assignment as a condition without parentheses">,
   InGroup<Parentheses>;
 def warn_free_nonheap_object
-  : Warning<"attempt to call %0 on non-heap object %1">,
+  : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression}1">,
     InGroup<FreeNonHeapObject>;
 
 // Completely identical except off by default.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to