adds more checks to -Wfree-nonheap-object

[cjdb]

Summary

This commit adds checks for the following:

• labels
• block expressions
• random integers cast to void*
• function pointers cast to void*

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

10274–10279

I think this simplifies things a bit (even though it's doing an isa<> followed by a cast<>).

10282–10288

10340

Please spell out the type (also, we don't typically use top-level const qualification on local variables or parameters).

10342

10345

The extra compound scope doesn't add too much, so I'd remove it.

10368

10372

Any reason not to handle LambdaExpr at the same time?

clang/test/Analysis/free.c

84

The formatting for this diagnostic is somewhat unfortunate in that it has the leading space before the :. I think that changing the diagnostic to use a %select would be an improvement.

[cjdb]

addresses comments

[cjdb]

clang/lib/Sema/SemaChecking.cpp

10274–10279

Wow, that's much nicer, thanks! :D

10282–10288

This one required a bit more work than the previous one because there are two overloads for CheckFreeArgumentsOnLvalue.

10340

Heh, this was only named for debugging purposes. I've consolidated it into the switch statement ;-)

10342

Done, but why? I quite like making it clear that fallthrough is intentional.

10345

I find it improves readability and groups what the comment above it (now inline with {) is talking about.

10372

None, I hadn't considered it. I'm not sure I see the relationship bet

clang/test/Analysis/free.c

84

I'm having a *lot* of difficulty getting %select to work. Here's what I've tried, but the space in %select{ %2 is being ignored :(

: Warning<"attempt to call %0 on non-heap object%select{ %2|: block expression}1">,

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

10342

I don't think an explicit marker is necessary because the two cases have no whitespace separating them (so it should be obvious from context that fallthrough is intentional rather than accidental). However, we have the LLVM_FALLTHROUGH macro for the cases where fallthrough is intentional -- we typically only use that macro when compilers would otherwise diagnose the fallthrough though.

clang/test/Analysis/free.c

84

We could cheat a little bit. :-D

Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression}1">

(The diagnostic should probably be updated to distinguish between block expressions and lambda expressions, which may add another layer of %select not shown here.)

[cjdb]

clang/test/Analysis/free.c

84

That doesn't fix the issue, which is that everything before %2 is deleted.

[aaron.ballman]

clang/test/Analysis/free.c

84

Huh? That seems very surprising given that we use this pattern in plenty of other places:

https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L483
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L685
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L1439

Can you post the output you're getting when you try my workaround? (I don't know if your original attempt will work because of the lack of whitespace before the %select in object%select.)

[cjdb]

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?

[cjdb]

clang/test/Analysis/free.c

84

Yeah, not sure what was going on here (maybe I didn't hit save before testing?), since it ICEd up when I re-applied it. Anyway, got it working :-)

[cjdb]

Ping. @aaron.ballman any further requests? I think this is good to land otherwise?

[aaron.ballman]

In D94640#2560647, @cjdb wrote:

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?

Sorry about missing your question -- this fell off my radar. I was thinking it'd be roughly the same as blocks, but I now think this'll create compile errors anyway: free([]{ return nullptr; }); (where the lambda is accidentally not being called). So not nearly as important as I was thinking (sorry for that, but thank you for adding the support and test cases just the same!).

One worry I have is the number of effectively duplicated diagnostics we're now getting in conjunction with the static analyzer. It's not quite perfect overlap and so we can't get rid of the static analyzer check, but is there a long-term plan for how to resolve this?

clang/lib/Sema/SemaChecking.cpp

10271–10272

After seeing my suggested workaround in practice, I'm hopeful we can figure out how to get the diagnostics engine to not require us to jump through these hoops. I wouldn't block the patch over this approach, but it's not very obvious what's happening from reading the code either.

10379

FWIW, this diagnostic will be awkward for lambdas because it mentions blocks explicitly. It's not critical thing given how hard it would be to hit the lambda case, but if you think of an easy way to solve it, I won't complain.

clang/test/Analysis/free.c

66–68

This is another variant of the test that's slightly different (and only valid in C):

int *ptr(void);
void func(void) {
  free(ptr); // Oops, forgot to call ptr().
}

[cjdb]

In D94640#2579447, @aaron.ballman wrote:

In D94640#2560647, @cjdb wrote:

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?

Sorry about missing your question -- this fell off my radar. I was thinking it'd be roughly the same as blocks, but I now think this'll create compile errors anyway: free([]{ return nullptr; }); (where the lambda is accidentally not being called). So not nearly as important as I was thinking (sorry for that, but thank you for adding the support and test cases just the same!).

One worry I have is the number of effectively duplicated diagnostics we're now getting in conjunction with the static analyzer. It's not quite perfect overlap and so we can't get rid of the static analyzer check, but is there a long-term plan for how to resolve this?

I filed issue 48767, but I don't think the analyser folks are motivated to address it any time soon.

clang/lib/Sema/SemaChecking.cpp

10271–10272

Sorry, I'm not entirely sure what you're getting at here? :/

10379

Nothing "easy" comes to mind.

[cjdb]

applies @aaron.ballman's feedback

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

10271–10272

I used to find the Placeholder stuff to be confusing in practice and would have preferred to see a diagnostic solution that doesn't need the extra argument. Now I think I'm changing my mind and have a different suggestion -- remove the Placeholder and NoPlaceholder variables and pass in a literal with a comment:

S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
        << CalleeName << 0 /*object*/<< cast<NamedDecl>(D);

  S.Diag(Lambda->getBeginLoc(), diag::warn_free_nonheap_object)
      << CalleeName << 2 /*lambda*/;

That removes the akwardness of trying to name the variables, which is what was was I found non-obvious before.

[cjdb]

applies @aaron.ballman's suggestion

[aaron.ballman]

LGTM, thank you for this!

[thakis]

This seems to flag https://source.chromium.org/chromium/chromium/src/+/master:third_party/libsync/src/sync.c;l=142?q=sync.c&ss=chromium :

info->sync_fence_info = (uint64_t) calloc(num_fences,
                                sizeof(struct sync_fence_info));
if ((void *)info->sync_fence_info == NULL)
    goto free;

err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
if (err < 0) {
    free((void *)info->sync_fence_info);
    goto free;
}

What's the motivation for flagging an integer that's >= sizeof(void*) and that's explicitly cast to void*? That seems like code that's pretty explicit about its intentions.

Did you do true positive / false positive evaluation of this change?

[cjdb]

In D94640#2590512, @thakis wrote:

This seems to flag https://source.chromium.org/chromium/chromium/src/+/master:third_party/libsync/src/sync.c;l=142?q=sync.c&ss=chromium :

info->sync_fence_info = (uint64_t) calloc(num_fences,
                                sizeof(struct sync_fence_info));
if ((void *)info->sync_fence_info == NULL)
    goto free;

err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
if (err < 0) {
    free((void *)info->sync_fence_info);
    goto free;
}

What's the motivation for flagging an integer that's >= sizeof(void*) and that's explicitly cast to void*? That seems like code that's pretty explicit about its intentions.

Did you do true positive / false positive evaluation of this change?

This change was a bit too aggressive and is being fixed in D97512.

[Ka-Ka]

This patch seems to introduce warnings for the case

#include <stdlib.h>
#include <stdint.h>
void foo(void* ptr)
{

free((void*)(intptr_t) ptr);

}

something that gcc don't warn for (see https://godbolt.org/z/1WT9c6 ).

As intptr_t is suppose to be used to pass pointers in I think its a bit strange that clang warns in this case.

[lebedev.ri]

<...>

Is the patch still not reverted?
That should have happened almost a week ago.

[aaron.ballman]

In D94640#2603054, @lebedev.ri wrote:

<...>

Is the patch still not reverted?
That should have happened almost a week ago.

FWIW, the fix for the issue was just approved.

[Ka-Ka]

Thanks for the information. I found the review now in https://reviews.llvm.org/D97512

[rsmith]

Sorry for the late review.

clang/lib/Sema/SemaChecking.cpp

10294–10303

This special case is rather too special, and I don't think it's worth the lines of code we're spending on it.

There's a fair amount of work being done here to figure out what's being passed to free, and some of it, like this, seems pretty hard to justify in isolation. I wonder if we can reuse some of our existing mechanisms -- for example, can we run the constant evaluator on the operand to free instead, and see if it evaluates to a known non-heap object?

10326–10328

Please don't pretty-print source expressions and include them in the diagnostic text. Instead, include a source range to highlight them in the caret diagnostic. See https://clang.llvm.org/docs/InternalsManual.html#the-format-string for our diagnostic best practices.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
 def err_incomplete_object_call : Error<
   "incomplete type in call to object of type %0">;
 
 def warn_condition_is_assignment : Warning<"using the result of an "
   "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|object: lambda-to-function-pointer conversion}1">,
     InGroup<FreeNonHeapObject>;
 
 // Completely identical except off by default.
 def warn_condition_is_idiomatic_assignment : Warning<"using the result "
   "of an assignment as a condition without parentheses">,
   InGroup<DiagGroup<"idiomatic-parentheses">>, DefaultIgnore;
 def note_condition_assign_to_comparison : Note<
   "use '==' to turn this assignment into an equality comparison">;

clang/lib/Sema/SemaChecking.cpp

void Sema::CheckStrncatArguments(const CallExpr *CE,
   OS << "strlen(";
   DstArg->printPretty(OS, nullptr, getPrintingPolicy());
   OS << ") - 1";
 
   Diag(SL, diag::note_strncat_wrong_size)
     << FixItHint::CreateReplacement(SR, OS.str());
 }
 
 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;
-}
-
-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 << 0 /*object: */ << 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,
                                       Lvalue->getMemberDecl());
 }
 
-void CheckFreeArgumentsStackArray(Sema &S, const std::string &CalleeName,
-                                  const DeclRefExpr *Lvalue) {
-  if (!Lvalue->getType()->isArrayType())
+void CheckFreeArgumentsPlus(Sema &S, const std::string &CalleeName,
+                            const UnaryOperator *UnaryExpr) {
+  const auto *Lambda = dyn_cast<LambdaExpr>(
+      UnaryExpr->getSubExpr()->IgnoreImplicitAsWritten()->IgnoreParens());
+  if (!Lambda)
     return;
 
+  S.Diag(Lambda->getBeginLoc(), diag::warn_free_nonheap_object)
+      << CalleeName << 2 /*object: lambda expression*/;
+}
+
+void CheckFreeArgumentsStackArray(Sema &S, const std::string &CalleeName,
+                                  const DeclRefExpr *Lvalue) {
   const auto *Var = dyn_cast<VarDecl>(Lvalue->getDecl());
   if (Var == nullptr)
     return;
 
   S.Diag(Lvalue->getBeginLoc(), diag::warn_free_nonheap_object)
-      << CalleeName << Var;
+      << CalleeName << 0 /*object: */ << 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;
+    [[clang::fallthrough]];
+  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 << 0 /*object: */ << 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();
 
+  { // 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))
+      switch (UnaryExpr->getOpcode()) {
+      case UnaryOperator::Opcode::UO_AddrOf:
         return CheckFreeArgumentsAddressof(*this, CalleeName, UnaryExpr);
+      case UnaryOperator::Opcode::UO_Plus:
+        return CheckFreeArgumentsPlus(*this, CalleeName, UnaryExpr);
+      default:
+        break;
+      }
 
     if (const auto *Lvalue = dyn_cast<DeclRefExpr>(Arg))
+      if (Lvalue->getType()->isArrayType())
         return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue);
+
+    if (const auto *Label = dyn_cast<AddrLabelExpr>(Arg)) {
+      Diag(Label->getBeginLoc(), diag::warn_free_nonheap_object)
+          << CalleeName << 0 /*object: */ << Label->getLabel()->getIdentifier();
+      return;
+    }
+
+    if (isa<BlockExpr>(Arg)) {
+      Diag(Arg->getBeginLoc(), diag::warn_free_nonheap_object)
+          << CalleeName << 1 /*object: block*/;
+      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
 Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
                          SourceLocation ReturnLoc,
                          bool isObjCMethod,
                          const AttrVec *Attrs,
                          const FunctionDecl *FD) {
   // Check if the return value is null but should not be.
   if (((Attrs && hasSpecificAttr<ReturnsNonNullAttr>(*Attrs)) ||
        (!isObjCMethod && isNonNullType(Context, lhsType))) &&
       CheckNonNullExpr(*this, RetValExp))
     Diag(ReturnLoc, diag::warn_null_ret)

clang/test/Analysis/free.c

 }
 
 void t5 () {
   extern char *ptr();
   free(ptr()); // no-warning
 }
 
 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) {
   free(*x); // no-warning
 }
 
 void t8 (char **x) {
   // ugh
   free((*x)+8); // no-warning
 }
 
 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 () {
   char *p = (char*)alloca(2);
   free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
 }
 
 void t12 () {
   char *p = (char*)__builtin_alloca(2);
   free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
 }
 
 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) {
   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'}}
 }
 
 static int someGlobal[2];
 void t15 () {
   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 t16 (char **x, int offset) {
   // Unknown value
   free(x[offset]); // no-warning
 }
+
+int *iptr(void);
+void t17(void) {
+  free(iptr); // Oops, forgot to call iptr().
+  // expected-warning@-1{{Argument to free() is the address of the function 'iptr', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'iptr'}}
+}

clang/test/Analysis/free.cpp

+// 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
+}

clang/test/Analysis/malloc-fnptr-plist.c

 // RUN: %clang_analyze_cc1 -analyzer-checker core,unix.Malloc -analyzer-output=plist -o %t.plist -verify %s
 // RUN: FileCheck --input-file=%t.plist %s
 
 void free(void *);
 void (*fnptr)(int);
 void foo() {
-  free((void *)fnptr); // expected-warning{{Argument to free() is a function pointer}}
+  free((void *)fnptr);
+  // expected-warning@-1{{Argument to free() is a function pointer}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}}
 }
 
 // Make sure the bug category is correct.
 // CHECK: <key>category</key><string>Memory error</string>

clang/test/Analysis/malloc.c

 
 void (*fnptr)(int);
 void freeIndirectFunctionPtr() {
   void *p = (void *)fnptr;
   free(p); // expected-warning {{Argument to free() is a function pointer}}
 }
 
 void freeFunctionPtr() {
-  free((void *)fnptr); // expected-warning {{Argument to free() is a function pointer}}
+  free((void *)fnptr);
+  // expected-warning@-1{{Argument to free() is a function pointer}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}}
 }
 
 void allocateSomeMemory(void *offendingParameter, void **ptr) {
   *ptr = malloc(1);
 }
 
 void testNoCrashOnOffendingParameter() {
   // "extern" is necessary to avoid unrelated warnings

clang/test/Analysis/weak-functions.c

 }
 
 //===----------------------------------------------------------------------===
 // free.c
 //===----------------------------------------------------------------------===
 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'}}
 }
 
 //===----------------------------------------------------------------------===
 // string.c : strnlen()
 //===----------------------------------------------------------------------===
 typedef typeof(sizeof(int)) size_t;
 size_t strlen(const char *s) __attribute__((weak_import));