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@-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 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@-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 t12 () { } 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 @@ 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'}} +} 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@-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 +} 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@-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. 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@-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) { 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@-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'}} } //===----------------------------------------------------------------------=== _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
