Index: clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tidy/modernize/CMakeLists.txt +++ clang-tidy/modernize/CMakeLists.txt @@ -9,6 +9,7 @@ MakeSharedCheck.cpp MakeUniqueCheck.cpp ModernizeTidyModule.cpp + NoMallocCheck.cpp PassByValueCheck.cpp RawStringLiteralCheck.cpp RedundantVoidArgCheck.cpp Index: clang-tidy/modernize/NoMallocCheck.h =================================================================== --- /dev/null +++ clang-tidy/modernize/NoMallocCheck.h @@ -0,0 +1,76 @@ +//===--- NoMallocCheck.h - clang-tidy----------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NO_MALLOC_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NO_MALLOC_H + +#include "../ClangTidy.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// This checker is concerned with c-style memory management and tries to +/// sanitize its use. +/// For CPP, it follows cppcoreguidelines (Resource Management) and for C +/// it checks for common bugs and tries to clarify the code. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-no-malloc.html /// +/// github - CPPCoreGuidelines +class NoMallocCheck : public ClangTidyCheck { +public: + NoMallocCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + /// implements the cast check, calling malloc check with in + void handleCast(const CStyleCastExpr *CastExpr); + + /// implements the malloc check and fixit hints for it + bool handleMalloc(const CallExpr *MallocCall, bool FromCast = false); + + /// implements the realloc check, suggests using std::vector for this variable + void handleRealloc(const CallExpr *ReallocCall); + + /// implements the free call, suggest RAII + void handleFree(const CallExpr *FreeCall); + + /// handle single element and raw bytesize mallocs + bool singleOrRaw(const CallExpr *MallocCall); + + /// handle malloc call with a multiplication as argument e.g. sizeof int * + /// count + bool multiplication(const CallExpr *MallocCall, + const BinaryOperator *CallArg); + + /// just warn about a suspicious malloc argument + void suspiciousBinary(const CallExpr *MallocCall); + + /// give a hint how to call malloc properly + void giveCallHint(const CallExpr *MallocCall); + + /// give notes about alternative ways to create dynamic arrays + void giveArrayHint(const CallExpr *MallocCall, const std::string &THint, + const std::string &CountStr); + + /// give deletion hint, when malloc is removed. delete/delete[] must be + /// adopted! + void giveDeletionHint(const CallExpr *MallocCall); +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NO_MALLOC_H Index: clang-tidy/modernize/NoMallocCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/modernize/NoMallocCheck.cpp @@ -0,0 +1,338 @@ +//===--- NoMallocCheck.cpp - clang-tidy------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "NoMallocCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +/// Register for all function calls, and filter malloc out +void NoMallocCheck::registerMatchers(MatchFinder *Finder) { + // this matcher finds statements in the form + // char* str = (char*) malloc(...); + // ~~~~~~~~~~~~~~~~~~~~ + Finder->addMatcher( + cStyleCastExpr( + hasDescendant(callExpr(callee(functionDecl(hasName("::malloc")))) + .bind("malloc"))) + .bind("c_cast"), + this); + + // registers realloc calls, suggest std::vector + Finder->addMatcher( + callExpr(callee(functionDecl(hasName("::realloc")))).bind("realloc"), + this); + + // registers free calls, suggest proper RAII and delete + Finder->addMatcher( + callExpr(callee(functionDecl(hasName("::free")))).bind("free"), this); +} + +// -------------------- +// Helper Functionality +// -------------------- +namespace { + +/// kinda inspired from here +/// http://stackoverflow.com/questions/9607852/print-arguments-of-a-function-using-clang-ast +std::string stmtToString(const Stmt *Statement, LangOptions L) { + // configuration + PrintingPolicy Policy(L); + + // printing + std::string StmtStr; + llvm::raw_string_ostream Stream(StmtStr); + Statement->printPretty(Stream, 0, Policy); + + return StmtStr; +} + +/// helper functions, to check if the expr is a sizeof expression +bool isSizeOfExpr(const UnaryExprOrTypeTraitExpr *Expression) noexcept { + return Expression->getKind() == UnaryExprOrTypeTrait::UETT_SizeOf; +} + +/// try to infere the type of a potential sizeof expression, +/// \return false if type not infereable, otherwise true +bool inferSizeofType(const Expr *Side, QualType &QType) { + const auto sizeOf = dyn_cast(Side); + + // could not cast expression to sizeof like expr + if (sizeOf == nullptr) + return false; + + // casted expression wasnt sizeof, maybe a typetrait of alignof + if (isSizeOfExpr(sizeOf) == false) + return false; + + // we did find a sizeof expression, write the type and return true + QType = sizeOf->getArgumentType(); + return true; +} + +/// convert an array count to a string for fixing +std::string countToString(const BinaryOperator *BinExpr, bool lhsRhsFlag, + LangOptions L) { + const auto *countExpr = lhsRhsFlag ? BinExpr->getLHS() : BinExpr->getRHS(); + return stmtToString(countExpr, L); +} + +/// find out the type, if a sizeof expr exists, applied to (sizeof(type) * c) +/// flag is true, when count will be on lhs +/// flag is false, when count will be on rhs +bool determineTypeAndCount(const BinaryOperator *BinArg, QualType &QType, + bool &LhsRhsFlag) { + const auto *Lhs = BinArg->getLHS(); + const auto *Rhs = BinArg->getRHS(); + + // on success, sets the type + // lhs inferes type + if (inferSizeofType(Lhs, QType)) { + LhsRhsFlag = false; // rhs is count + return true; + } + // rhs inferes type + if (inferSizeofType(Rhs, QType)) { + LhsRhsFlag = true; // lhs is count + return true; + } + // could not infere type, therefore results are invalid! + return false; +} + +} // namespace helpers + +/// Check if the catched function call was malloc, and warn, propose new +void NoMallocCheck::check(const MatchFinder::MatchResult &Result) { + + if (const auto *CastMatch = Result.Nodes.getNodeAs("c_cast")) + handleCast(CastMatch); + + if (const auto *MallocMatch = Result.Nodes.getNodeAs("malloc")) + handleMalloc(MallocMatch); + + if (const auto *ReallocMatch = Result.Nodes.getNodeAs("realloc")) + handleRealloc(ReallocMatch); + + if (const auto *FreeMatch = Result.Nodes.getNodeAs("free")) + handleFree(FreeMatch); +} + +/// handle cast expression, containing a malloc +/// (int*) malloc(sizeof(int) * 10); would be one example +void NoMallocCheck::handleCast(const CStyleCastExpr *CastExpr) { + // first handle the malloc, check if a fix was supposed + const auto *MallocCall = dyn_cast(CastExpr->getSubExprAsWritten()); + + if (MallocCall == nullptr) + return; + + // handle the malloc, give information that is called from a cast expression + if (handleMalloc(MallocCall, true)) { + diag(CastExpr->getLocStart(), "Cast not needed anymore") + << FixItHint::CreateRemoval( + {CastExpr->getLocStart(), + MallocCall->getLocStart().getLocWithOffset(-1)}); + } +} + +/// implements the malloc check and its fixit hints +/// returns true, if a fix was supposed +bool NoMallocCheck::handleMalloc(const CallExpr *MallocCall, bool FromCast) { + // i belive this is done by CSA, but i added it just in case! + LangOptions LOpts = getLangOpts(); + if (!FromCast && !LOpts.CPlusPlus) + diag(MallocCall->getLocStart(), + "Use of malloc with out explicit cast to return value!"); + + // getting the bytesize argument of the malloc call + // if its a binaryoperator (mulitplication), infer more information! + auto CallArg = dyn_cast(MallocCall->getArg(0)); + + // the arguent to malloc was not a computation -> no more information + // achievable + if (CallArg == nullptr) { + return singleOrRaw(MallocCall); + } + + // CallArg is a BinaryOperation + // multiplication should be the common case + // since malloc(sizeof(int) * count) - form + if (CallArg->isMultiplicativeOp()) { + return multiplication(MallocCall, CallArg); + } + + // we dont have a multiplication, this is shady! + suspiciousBinary(MallocCall); + return false; +} + +/// implements the realloc check and its suggestions (std::vector) +void NoMallocCheck::handleRealloc(const CallExpr *ReallocCall) { + diag(ReallocCall->getLocStart(), "Reallocation memory suggests, that either " + "std::vector or std::string is a better " + "match for your usecase than C-Style memory " + "management. [R.10?]") + << SourceRange{ReallocCall->getLocStart(), ReallocCall->getLocEnd()}; + diag(ReallocCall->getLocStart(), "std::vector<>::resize does the job!", + DiagnosticIDs::Note); + giveDeletionHint(ReallocCall); +} + +/// implements the free check and its suggestions (RAII) +void NoMallocCheck::handleFree(const CallExpr *FreeCall) { + diag(FreeCall->getLocStart(), "Do not use error prone C-Style memory " + "management. Consider using RAII Objects. " + "[R.10]") + << SourceRange{FreeCall->getLocStart(), FreeCall->getLocEnd()}; + giveDeletionHint(FreeCall); +} + +/// handle malloc call that has not a binary expression as argument +/// \return true if a fix is suggested +bool NoMallocCheck::singleOrRaw(const CallExpr *MallocCall) { + // check if its malloc(sizeof(int)) and transform to normal new + // otherwise warn + QualType QualT; + bool SizeOfTypeFlag = inferSizeofType(MallocCall->getArg(0), QualT); + + // we got a sizeof + if (SizeOfTypeFlag) { + const Type *TypeNoQual = QualT.split().asPair().first; + std::string Replacement = + "new " + QualType::getAsString(TypeNoQual, Qualifiers()); + + diag(MallocCall->getLocStart(), + "Use new instead of malloc for object-creation. [R.10]") + << FixItHint::CreateReplacement( + {MallocCall->getLocStart(), MallocCall->getLocEnd()}, Replacement); + giveDeletionHint(MallocCall); + + return true; + } + + // we got a suspicious malloc with no information + diag(MallocCall->getLocStart(), "Could not infer your usecase of malloc!"); + giveCallHint(MallocCall); + + return false; +} + +/// handles malloc calls with multiplication as argument +/// \returns true if a fix is suggested +bool NoMallocCheck::multiplication(const CallExpr *MallocCall, + const BinaryOperator *CallArg) { + // determine_type_and_count sets this flag. + // on true -> lhs is count expression + // on false -> rhs is count expression + bool LhsRhsFlag = false; + QualType SizeOfType; + + if (determineTypeAndCount(CallArg, SizeOfType, LhsRhsFlag)) { + // either lhs or rhs are type/count. find it out and write to variables + // suggest an fix in the form 'new type[count]' + + // get the expression for the count amount + std::string CountExprStr = + countToString(CallArg, LhsRhsFlag, this->getLangOpts()); + + // remove qualifiers from the infered type + const Type *TypeNoQual = SizeOfType.split().asPair().first; + // generate string from it + std::string THint = QualType::getAsString(TypeNoQual, Qualifiers()); + std::string Replacement = "new " + THint + "[" + CountExprStr + "]"; + + // warn the malloc and give a fix hint with new[] syntax + diag(MallocCall->getLocStart(), + "Create Dynamic Arrays with std::vector(resizeable), " + "std::make_unique, gsl::dyn_array(both " + "dynamic, but fixed size) [Resource Management]") + << SourceRange{MallocCall->getLocStart(), MallocCall->getLocEnd()} + << FixItHint::CreateReplacement( + SourceRange{MallocCall->getLocStart(), MallocCall->getLocEnd()}, + Replacement); + + giveArrayHint(MallocCall, THint, CountExprStr); + giveDeletionHint(MallocCall); + + return true; + } + // failed to infer type and count + else { + diag(MallocCall->getLocStart(), + "suspicious malloc. weired multiplication as argument!") + << SourceRange{MallocCall->getLocStart(), MallocCall->getLocEnd()}; + + giveCallHint(MallocCall); + + return false; + } +} + +/// warning about a suspicious, possibly buggy malloc call +void NoMallocCheck::suspiciousBinary(const CallExpr *MallocCall) { + // warn the suspicious malloc call + diag(MallocCall->getLocStart(), + "Suspicious malloc. Argument is a Binary Operation, but no " + "multiplication.") + << SourceRange{MallocCall->getLocStart(), MallocCall->getLocEnd()}; + + // note for a possible bug + diag(MallocCall->getLocStart(), + "Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug", + DiagnosticIDs::Note) + << SourceRange{MallocCall->getLocStart(), MallocCall->getLocEnd()}; + + // note how to call properly + giveCallHint(MallocCall); +} + +/// hint how to call malloc properly as Diagnostic::Note +void NoMallocCheck::giveCallHint(const CallExpr *MallocCall) { + diag(MallocCall->getLocStart(), + "Write malloc in the form (type*) malloc(sizeof(type) * count)", + DiagnosticIDs::Note); +} + +/// notes about other ways to create a dynamic array +void NoMallocCheck::giveArrayHint(const CallExpr *MallocCall, + const std::string &THint, + const std::string &CountStr) { + std::string VecHint = "Consider std::vector<" + THint + ">(" + CountStr + ")"; + std::string UniqueHint = + "Consider std::make_unique<" + THint + "[]>(" + CountStr + ")"; + std::string GSLHint = + "Consider gsl::dyn_array<" + THint + ">(" + CountStr + ")"; + + // notes on other possible replacements + diag(MallocCall->getLocStart(), VecHint, DiagnosticIDs::Note); + diag(MallocCall->getLocStart(), UniqueHint, DiagnosticIDs::Note); + diag(MallocCall->getLocStart(), GSLHint, DiagnosticIDs::Note); +} + +/// give hint on deletion of the memory resource +void NoMallocCheck::giveDeletionHint(const CallExpr *MallocCall) { + diag(MallocCall->getLocStart(), + "You must manually remove free and replace it with delete/delete[]!", + DiagnosticIDs::Note); + diag(MallocCall->getLocStart(), "When using a RAII mechanism, remove the " + "explicit deleting of this variable!", + DiagnosticIDs::Note); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -81,6 +81,12 @@ Warns if an object is used after it has been moved, without an intervening reinitialization. +- New `modernize-no-malloc + `_ check + + Warns if C-style memory management is used and tries to replace it with + operator ``new``. + - New `mpi-buffer-deref `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -100,6 +100,7 @@ modernize-loop-convert modernize-make-shared modernize-make-unique + modernize-no-malloc modernize-pass-by-value modernize-raw-string-literal modernize-redundant-void-arg Index: docs/clang-tidy/checks/modernize-no-malloc.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/modernize-no-malloc.rst @@ -0,0 +1,53 @@ +.. title:: clang-tidy - modernize-no-malloc + +modernize-no-malloc +=================== + +This check handles C-Style memory management using ``malloc()``, ``realloc()`` +and ``free()``. It warns about its use and tries to replace ``malloc()`` with +the corresponding operator ``new`` call. If it fails to do so, a note will be emitted +how to write ``malloc()`` calls with more information, making it easier to reason +about it. +See `C++ Core Guidelines +` + +Both array... + +.. code-block:: c++ + + char* some_string = (char*) malloc(sizeof(char) * 20); + + // becomes + + char* some_string = new char[20]; + +... and single element creation are supported. + +.. code-block:: c++ + + int* integer = (int*) malloc(sizeof(int)); + + // becomes + + int* integer = new int; + + +Furthermore it warns about suspicious calls to ``malloc()``. + +.. code-block:: c++ + + // suspicious because not an multiplication of sizeof() and count + int* int_array = (int*) malloc(25); + int* buggy_array = (int*) malloc(sizeof(int) * 10 + 1); + + +Since general replacement of ``malloc()`` with ``new`` requires the replacement of the +``free()`` calls as well, automatic fixing should be used with caution. The check will +warn about C style memory management, gives hints how to do it better and +reference the C++ Core Guidelines. + +Know Bug +======== + +The fixit-hint for struct, class and enum objects are incorrect. + Index: test/clang-tidy/modernize-no-malloc.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-no-malloc.cpp @@ -0,0 +1,109 @@ +// RUN: %check_clang_tidy %s modernize-no-malloc %t + +using size_t = unsigned long; + +//void *calloc(size_t num, size_t size); +void *malloc(size_t size); +void *realloc(void *ptr, size_t size); +void free(void *ptr); + +/* These will be used when struct and class mallocing is handled correctly +struct some_struct { + int number1; + char character1; +}; + +class some_class { + int number1; + char character1; +}; +*/ + +void malloced_array() { + int *array0 = (int *)malloc(sizeof(int) * 20); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Cast not needed anymore [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: Create Dynamic Arrays with std::vector(resizeable), std::make_unique, gsl::dyn_array(both dynamic, but fixed size) [Resource Management] [modernize-no-malloc] + // CHECK-FIXES: new int[20] + // CHECK-MESSAGES: :[[@LINE-4]]:24: note: Consider std::vector(20) + // CHECK-MESSAGES: :[[@LINE-5]]:24: note: Consider std::make_unique(20) + // CHECK-MESSAGES: :[[@LINE-6]]:24: note: Consider gsl::dyn_array(20) + // CHECK-MESSAGES: :[[@LINE-7]]:24: note: You must manually remove free and replace it with delete/delete[]! + // CHECK-MESSAGES: :[[@LINE-8]]:24: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + int *array1 = (int *)malloc(0 * sizeof(int)); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Cast not needed anymore [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: Create Dynamic Arrays with std::vector(resizeable), std::make_unique, gsl::dyn_array(both dynamic, but fixed size) [Resource Management] [modernize-no-malloc] + // CHECK-FIXES: new int[0] + // CHECK-MESSAGES: :[[@LINE-4]]:24: note: Consider std::vector(0) + // CHECK-MESSAGES: :[[@LINE-5]]:24: note: Consider std::make_unique(0) + // CHECK-MESSAGES: :[[@LINE-6]]:24: note: Consider gsl::dyn_array(0) + // CHECK-MESSAGES: :[[@LINE-7]]:24: note: You must manually remove free and replace it with delete/delete[]! + // CHECK-MESSAGES: :[[@LINE-8]]:24: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + // very dangerous + int *array2 = (int *)malloc(40); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: Could not infer your usecase of malloc! [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: note: Write malloc in the form (type*) malloc(sizeof(type) * count) + + // bug, + 1 must be with ten and in paranthesis + int *array3 = (int *)malloc(10 * sizeof(int) + 1); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: Suspicious malloc. Argument is a Binary Operation, but no multiplication. [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: note: Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug + // CHECK-MESSAGES: :[[@LINE-3]]:24: note: Write malloc in the form (type*) malloc(sizeof(type) * count) + + // bug, 10 + 1 must be in paranthesis + int *array4 = (int *)malloc(10 + 1 * sizeof(int)); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: Suspicious malloc. Argument is a Binary Operation, but no multiplication. [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: note: Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug + // CHECK-MESSAGES: :[[@LINE-3]]:24: note: Write malloc in the form (type*) malloc(sizeof(type) * count) + + int *object = (int *)malloc(sizeof(int)); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Cast not needed anymore [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: Use new instead of malloc for object-creation. [R.10] [modernize-no-malloc] + // CHECK-FIXES: new int + // CHECK-MESSAGES: :[[@LINE-4]]:24: note: You must manually remove free and replace it with delete/delete[]! + // CHECK-MESSAGES: :[[@LINE-5]]:24: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + /* Checks that will be enabled, when struct and class mallocing is checked correctly + some_struct *struct_obj = (some_struct *)malloc(sizeof(some_struct)); + + some_class *class_obj = (some_class *)malloc(sizeof(some_class)); + */ + + // reallocation memory, std::vector shall be used + char *realloced = (char *)realloc(array4, 50 * sizeof(int)); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Reallocation memory suggests, that either std::vector or std::string is a better match for your usecase than C-Style memory management. [R.10?] [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:29: note: std::vector<>::resize does the job! + // CHECK-MESSAGES: :[[@LINE-3]]:29: note: You must manually remove free and replace it with delete/delete[]! + // CHECK-MESSAGES: :[[@LINE-4]]:29: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + // freeing memory the bad way + free(realloced); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Do not use error prone C-Style memory management. Consider using RAII Objects. [R.10] [modernize-no-malloc] + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: You must manually remove free and replace it with delete/delete[]! + // CHECK-MESSAGES: :[[@LINE-3]]:3: note: When using a RAII mechanism, remove the explicit deleting of this variable! +} +/*extern "C" { +void test_c() { + // using a different type for malloc arg, then assignment + // allowed in c, not in c++. how can i test with c only? + int* array7 = (char*) malloc(10); + + // poor malloc with no type information + char* buffer = malloc(100); +} +}*/ + +/// newing an array is still not good, but not relevant to this checker +void newed_array() { + int *new_array = new int[10]; // OK(1) +} + +void arbitrary_call() { + // we dont want every function to raise the warning + // even if malloc is in the name? this could be unwanted + malloced_array(); // OK(2) + + // completly unrelated function call to malloc + newed_array(); // OK(3) +}