[clang] 4f395db - adds more checks to -Wfree-nonheap-object
Christopher Di Bella via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 25 11:26:28 PST 2021
Author: Christopher Di Bella
Date: 2021-02-25T19:25:00Z
New Revision: 4f395db86b5cc11bb56853323d3cb1d4b6db5a0b
URL: https://github.com/llvm/llvm-project/commit/4f395db86b5cc11bb56853323d3cb1d4b6db5a0b
DIFF: https://github.com/llvm/llvm-project/commit/4f395db86b5cc11bb56853323d3cb1d4b6db5a0b.diff
LOG: adds more checks to -Wfree-nonheap-object
This commit adds checks for the following:
* labels
* block expressions
* random integers cast to `void*`
* function pointers cast to `void*`
Differential Revision: https://reviews.llvm.org/D94640
Added:
clang/test/Analysis/free.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Analysis/free.c
clang/test/Analysis/malloc-fnptr-plist.c
clang/test/Analysis/malloc.c
clang/test/Analysis/weak-functions.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8f71962502ae..481ed57c0b58 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7637,7 +7637,7 @@ 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.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2c19e91c906e..f69bdf97aa8d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10269,65 +10269,109 @@ void Sema::CheckStrncatArguments(const CallExpr *CE,
}
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();
- 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 << 0 /*object: */ << Label->getLabel()->getIdentifier();
+ return;
+ }
- if (const auto *Lvalue = dyn_cast<DeclRefExpr>(Arg))
- return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue);
+ 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
diff --git a/clang/test/Analysis/free.c b/clang/test/Analysis/free.c
index b50145713924..84d53472158c 100644
--- a/clang/test/Analysis/free.c
+++ b/clang/test/Analysis/free.c
@@ -41,7 +41,9 @@ void t5 () {
}
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 at -1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object '(void *)1000'}}
}
void t7 (char **x) {
@@ -55,11 +57,15 @@ void t8 (char **x) {
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 at -1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+ // expected-warning at -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 at -1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 't10'}}
}
void t11 () {
@@ -73,7 +79,9 @@ void t12 () {
}
void t13 () {
- free(^{return;}); // expected-warning {{Argument to free() is a block, which is not memory allocated by malloc()}}
+ free(^{return;});
+ // expected-warning at -1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object: block expression}}
}
void t14 (char a) {
@@ -93,3 +101,10 @@ 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 at -1{{Argument to free() is the address of the function 'iptr', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'iptr'}}
+}
diff --git a/clang/test/Analysis/free.cpp b/clang/test/Analysis/free.cpp
new file mode 100644
index 000000000000..2559770d6ddb
--- /dev/null
+++ b/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 at -1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t1b () {
+ int a[] = { 1 };
+ std::free(a);
+ // expected-warning at -1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t2a () {
+ int a = 1;
+ free(&a);
+ // expected-warning at -1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t2b () {
+ int a = 1;
+ std::free(&a);
+ // expected-warning at -1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t3a () {
+ static int a[] = { 1 };
+ free(a);
+ // expected-warning at -1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t3b () {
+ static int a[] = { 1 };
+ std::free(a);
+ // expected-warning at -1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -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 at -1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object '(void *)1000'}}
+}
+
+void t6b () {
+ std::free((void*)1000);
+ // expected-warning at -1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+ // expected-warning at -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 at -1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'label'}}
+}
+
+void t9b () {
+label:
+ std::free(&&label);
+ // expected-warning at -1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object 'label'}}
+}
+
+void t10a () {
+ free((void*)&t10a);
+ // expected-warning at -1{{Argument to free() is the address of the function 't10a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 't10a'}}
+}
+
+void t10b () {
+ std::free((void*)&t10b);
+ // expected-warning at -1{{Argument to free() is the address of the function 't10b', which is not memory allocated by malloc()}}
+ // expected-warning at -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 at -1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object: block expression}}
+}
+
+void t13b () {
+ std::free(^{return;});
+ // expected-warning at -1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object: block expression}}
+}
+
+void t14a () {
+ free((void *)+[]{ return; });
+ // expected-warning at -1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object: lambda-to-function-pointer conversion}}
+}
+
+void t14b () {
+ std::free((void *)+[]{ return; });
+ // expected-warning at -1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object: lambda-to-function-pointer conversion}}
+}
+
+void t15a (char a) {
+ free(&a);
+ // expected-warning at -1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t15b (char a) {
+ std::free(&a);
+ // expected-warning at -1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+static int someGlobal[2];
+void t16a () {
+ free(someGlobal);
+ // expected-warning at -1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 'someGlobal'}}
+}
+
+void t16b () {
+ std::free(someGlobal);
+ // expected-warning at -1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
+ // expected-warning at -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
+}
diff --git a/clang/test/Analysis/malloc-fnptr-plist.c b/clang/test/Analysis/malloc-fnptr-plist.c
index 6490eeb1cc03..432c1ddd7f9b 100644
--- a/clang/test/Analysis/malloc-fnptr-plist.c
+++ b/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 at -1{{Argument to free() is a function pointer}}
+ // expected-warning at -2{{attempt to call free on non-heap object '(void *)fnptr'}}
}
// Make sure the bug category is correct.
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index a26b51196781..9961c471bd7d 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1780,7 +1780,9 @@ void freeIndirectFunctionPtr() {
}
void freeFunctionPtr() {
- free((void *)fnptr); // expected-warning {{Argument to free() is a function pointer}}
+ free((void *)fnptr);
+ // expected-warning at -1{{Argument to free() is a function pointer}}
+ // expected-warning at -2{{attempt to call free on non-heap object '(void *)fnptr'}}
}
void allocateSomeMemory(void *offendingParameter, void **ptr) {
diff --git a/clang/test/Analysis/weak-functions.c b/clang/test/Analysis/weak-functions.c
index b3d8b043f8df..c29ac21d245d 100644
--- a/clang/test/Analysis/weak-functions.c
+++ b/clang/test/Analysis/weak-functions.c
@@ -71,7 +71,9 @@ void f3(void (*f)(void), void (*g)(void)) {
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 at -1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+ // expected-warning at -2{{attempt to call free on non-heap object 't10'}}
}
//===----------------------------------------------------------------------===
More information about the cfe-commits
mailing list