cjdb updated this revision to Diff 325889.
cjdb added a comment.
applies @aaron.ballman's feedback
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/malloc-fnptr-plist.c
clang/test/Analysis/malloc.c
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/malloc.c
===================================================================
--- clang/test/Analysis/malloc.c
+++ clang/test/Analysis/malloc.c
@@ -1780,7 +1780,9 @@
}
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) {
Index: clang/test/Analysis/malloc-fnptr-plist.c
===================================================================
--- clang/test/Analysis/malloc-fnptr-plist.c
+++ clang/test/Analysis/malloc-fnptr-plist.c
@@ -4,7 +4,9 @@
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.
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) {
@@ -93,3 +101,10 @@
// 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'}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10249,65 +10249,113 @@
}
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,
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;
+ static constexpr int NoPlaceholder = 2;
+ S.Diag(Lambda->getBeginLoc(), diag::warn_free_nonheap_object)
+ << CalleeName << NoPlaceholder;
+}
+
+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 << 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;
+ [[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 << 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))
+ 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 << Placeholder << Label->getLabel()->getIdentifier();
+ return;
+ }
- if (const auto *Lvalue = dyn_cast<DeclRefExpr>(Arg))
- return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue);
+ if (isa<BlockExpr>(Arg)) {
+ 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
@@ -7634,7 +7634,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|object: lambda-to-function-pointer conversion}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
