From 425a83a5f069eb1a692145d2c92e6d3bfe564a62 Mon Sep 17 00:00:00 2001 From: Christopher Di Bella Date: Wed, 28 Oct 2020 16:16:17 -0700 Subject: [PATCH] [Sema] adds basic -Wfree-nonheap-object functionality Checks to make sure that stdlib's (std::)free is being appropriately used. Presently checks for the following misuses: - free(&stack_object) - free(stack_array) Differential Revision: https://reviews.llvm.org/D89988 --- clang/include/clang/Basic/Builtins.def | 1 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 5 ++ clang/include/clang/Sema/Sema.h | 2 + clang/lib/AST/Decl.cpp | 6 ++ clang/lib/Sema/SemaChecking.cpp | 69 ++++++++++++++++-- clang/test/Sema/warn-free-nonheap-object.c | 34 +++++++++ clang/test/Sema/warn-free-nonheap-object.cpp | 90 ++++++++++++++++++++++++ 7 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 clang/test/Sema/warn-free-nonheap-object.c create mode 100644 clang/test/Sema/warn-free-nonheap-object.cpp diff --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def index b2876ed..45ea7d9 100644 --- a/clang/include/clang/Basic/Builtins.def +++ b/clang/include/clang/Basic/Builtins.def @@ -913,6 +913,7 @@ LIBBUILTIN(exit, "vi", "fr", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(_Exit, "vi", "fr", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(malloc, "v*z", "f", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(realloc, "v*v*z", "f", "stdlib.h", ALL_LANGUAGES) +LIBBUILTIN(free, "vv*", "f", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(strtod, "dcC*c**", "f", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(strtof, "fcC*c**", "f", "stdlib.h", ALL_LANGUAGES) LIBBUILTIN(strtold, "LdcC*c**", "f", "stdlib.h", ALL_LANGUAGES) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e1d20fb..97cacbe 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7539,6 +7539,11 @@ def err_incomplete_object_call : Error< def warn_condition_is_assignment : Warning<"using the result of an " "assignment as a condition without parentheses">, InGroup; +def warn_free_nonheap_object + : Warning<"attempt to call %0 on non-heap object %1">, + InGroup>, + DefaultIgnore; // FIXME: add to -Wall after sufficient testing + // Completely identical except off by default. def warn_condition_is_idiomatic_assignment : Warning<"using the result " "of an assignment as a condition without parentheses">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 3c31906..61ee743 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12400,6 +12400,8 @@ private: void CheckStrncatArguments(const CallExpr *Call, IdentifierInfo *FnName); + void CheckFreeArguments(const CallExpr *E); + void CheckReturnValExpr(Expr *RetValExp, QualType lhsType, SourceLocation ReturnLoc, bool isObjCMethod = false, diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 80bc22b..b4f92d7 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3959,6 +3959,9 @@ unsigned FunctionDecl::getMemoryFunctionKind() const { case Builtin::BIbzero: return Builtin::BIbzero; + case Builtin::BIfree: + return Builtin::BIfree; + default: if (isExternC()) { if (FnInfo->isStr("memset")) @@ -3987,6 +3990,9 @@ unsigned FunctionDecl::getMemoryFunctionKind() const { return Builtin::BIstrlen; else if (FnInfo->isStr("bzero")) return Builtin::BIbzero; + } else if (isInStdNamespace()) { + if (FnInfo->isStr("free")) + return Builtin::BIfree; } break; } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index e87adf8..3d5e2d7 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -4496,16 +4496,24 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, DiagnoseCStringFormatDirectiveInCFAPI(*this, FDecl, Args, NumArgs); unsigned CMId = FDecl->getMemoryFunctionKind(); - if (CMId == 0) - return false; // Handle memory setting and copying functions. - if (CMId == Builtin::BIstrlcpy || CMId == Builtin::BIstrlcat) + switch (CMId) { + case 0: + return false; + case Builtin::BIstrlcpy: // fallthrough + case Builtin::BIstrlcat: CheckStrlcpycatArguments(TheCall, FnInfo); - else if (CMId == Builtin::BIstrncat) + break; + case Builtin::BIstrncat: CheckStrncatArguments(TheCall, FnInfo); - else + break; + case Builtin::BIfree: + CheckFreeArguments(TheCall); + break; + default: CheckMemaccessArguments(TheCall, CMId, FnInfo); + } return false; } @@ -10098,6 +10106,57 @@ void Sema::CheckStrncatArguments(const CallExpr *CE, << FixItHint::CreateReplacement(SR, OS.str()); } +namespace { +void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName, + const UnaryOperator *UnaryExpr) { + if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf) + return; + + const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr()); + if (Lvalue == nullptr) + return; + + const auto *Var = dyn_cast(Lvalue->getDecl()); + if (Var == nullptr) + return; + + 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 CheckFreeArgumentsStackArray(Sema &S, const std::string &CalleeName, + const DeclRefExpr *Lvalue) { + if (!Lvalue->getType()->isArrayType()) + return; + + const auto *Var = dyn_cast(Lvalue->getDecl()); + if (Var == nullptr) + return; + + S.Diag(Lvalue->getBeginLoc(), diag::warn_free_nonheap_object) + << CalleeName << Var; +} +} // 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(E->getCalleeDecl())->getQualifiedNameAsString(); + + if (const auto *UnaryExpr = dyn_cast(Arg)) + return CheckFreeArgumentsAddressof(*this, CalleeName, UnaryExpr); + + if (const auto *Lvalue = dyn_cast(Arg)) + return CheckFreeArgumentsStackArray(*this, CalleeName, Lvalue); +} + void Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType, SourceLocation ReturnLoc, diff --git a/clang/test/Sema/warn-free-nonheap-object.c b/clang/test/Sema/warn-free-nonheap-object.c new file mode 100644 index 0000000..e149e83 --- /dev/null +++ b/clang/test/Sema/warn-free-nonheap-object.c @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -Wfree-nonheap-object -fsyntax-only -verify %s + +typedef __SIZE_TYPE__ size_t; +void *malloc(size_t); +void free(void *); + +int GI; +void test() { + { + free(&GI); // expected-warning {{attempt to call free on non-heap object 'GI'}} + } + { + static int SI = 0; + free(&SI); // expected-warning {{attempt to call free on non-heap object 'SI'}} + } + { + int I = 0; + free(&I); // expected-warning {{attempt to call free on non-heap object 'I'}} + } + { + int I = 0; + int *P = &I; + free(P); // FIXME diagnosing this would require control flow analysis. + } + { + void *P = malloc(8); + free(P); + } + { + int A[] = {0, 1, 2, 3}; + free(A); // expected-warning {{attempt to call free on non-heap object 'A'}} + free(&A); // expected-warning {{attempt to call free on non-heap object 'A'}} + } +} diff --git a/clang/test/Sema/warn-free-nonheap-object.cpp b/clang/test/Sema/warn-free-nonheap-object.cpp new file mode 100644 index 0000000..0578d9e --- /dev/null +++ b/clang/test/Sema/warn-free-nonheap-object.cpp @@ -0,0 +1,90 @@ +// RUN: %clang_cc1 -Wfree-nonheap-object -std=c++11 -x c++ -fsyntax-only -verify %s + +extern "C" void free(void *) {} + +namespace std { +using size_t = decltype(sizeof(0)); +void *malloc(size_t); +void free(void *p); +} // namespace std + +int GI; + +struct S { + operator char *() { return ptr; } + +private: + char *ptr = (char *)std::malloc(10); +}; + +void test1() { + { + free(&GI); // expected-warning {{attempt to call free on non-heap object 'GI'}} + } + { + static int SI = 0; + free(&SI); // expected-warning {{attempt to call free on non-heap object 'SI'}} + } + { + int I = 0; + free(&I); // expected-warning {{attempt to call free on non-heap object 'I'}} + } + { + int I = 0; + int *P = &I; + free(P); + } + { + void *P = std::malloc(8); + free(P); // FIXME diagnosing this would require control flow analysis. + } + { + int A[] = {0, 1, 2, 3}; + free(A); // expected-warning {{attempt to call free on non-heap object 'A'}} + } + { + int A[] = {0, 1, 2, 3}; + free(&A); // expected-warning {{attempt to call free on non-heap object 'A'}} + } + { + S s; + free(s); + free(&s); // expected-warning {{attempt to call free on non-heap object 's'}} + } +} + +void test2() { + { + std::free(&GI); // expected-warning {{attempt to call std::free on non-heap object 'GI'}} + } + { + static int SI = 0; + std::free(&SI); // expected-warning {{attempt to call std::free on non-heap object 'SI'}} + } + { + int I = 0; + std::free(&I); // expected-warning {{attempt to call std::free on non-heap object 'I'}} + } + { + int I = 0; + int *P = &I; + std::free(P); // FIXME diagnosing this would require control flow analysis. + } + { + void *P = std::malloc(8); + std::free(P); + } + { + int A[] = {0, 1, 2, 3}; + std::free(A); // expected-warning {{attempt to call std::free on non-heap object 'A'}} + } + { + int A[] = {0, 1, 2, 3}; + std::free(&A); // expected-warning {{attempt to call std::free on non-heap object 'A'}} + } + { + S s; + std::free(s); + std::free(&s); // expected-warning {{attempt to call std::free on non-heap object 's'}} + } +} -- 2.7.4