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