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/ModernizeTidyModule.cpp =================================================================== --- clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tidy/modernize/ModernizeTidyModule.cpp @@ -15,6 +15,7 @@ #include "LoopConvertCheck.h" #include "MakeSharedCheck.h" #include "MakeUniqueCheck.h" +#include "NoMallocCheck.h" #include "PassByValueCheck.h" #include "RawStringLiteralCheck.h" #include "RedundantVoidArgCheck.h" @@ -44,6 +45,8 @@ CheckFactories.registerCheck("modernize-loop-convert"); CheckFactories.registerCheck("modernize-make-shared"); CheckFactories.registerCheck("modernize-make-unique"); + CheckFactories.registerCheck( + "modernize-no-malloc"); CheckFactories.registerCheck("modernize-pass-by-value"); CheckFactories.registerCheck( "modernize-raw-string-literal"); Index: clang-tidy/modernize/NoMallocCheck.h =================================================================== --- /dev/null +++ clang-tidy/modernize/NoMallocCheck.h @@ -0,0 +1,80 @@ +//===--- 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 handle_cast(const CStyleCastExpr *MatchedDecl); + + /// implements the malloc check and fixit hints for it + bool handle_malloc(const CallExpr *MatchedDecl, bool from_cast = false); + + /// implements the realloc check, suggests using std::vector for this variable + void handle_realloc(const CallExpr *MatchedDecl); + + /// implements the free call, suggest RAII + void handle_free(const CallExpr *MatchedDecl); + + /// Helper for malloc checking, if binaryOp in malloc, which one is the size + /// and which the count + bool determine_type_and_count(const BinaryOperator *arg, QualType &type, + bool &lhs_rhs_flag) const; + + /// handle single element and raw bytesize mallocs + bool single_or_raw(const CallExpr *malloc_call); + + /// handle malloc call with a multiplication as argument e.g. sizeof int * + /// count + bool multiplication(const CallExpr *call, const BinaryOperator *arg); + + /// just warn about a suspicious malloc argument + void suspicious_binary(const CallExpr *call); + + /// give a hint how to call malloc properly + void give_call_hint(const CallExpr *malloc_call); + + /// give notes about alternative ways to create dynamic arrays + void give_array_hint(const CallExpr *malloc_call, const std::string &t_hint, + const std::string &count_str); + + /// give deletion hint, when malloc is removed. delete/delete[] must be + /// adopted! + void give_deletion_hint(const CallExpr *malloc_call); +}; + +} // 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,363 @@ +//===--- 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; + +/* +when calling diag(), supply a SourceRange as the first "argument" after the +closing parens. e.g., diag(someLoc, "some message %0") << SomeSourceRange << +SomeArg; + the source location corresponds to the ^ caret, the source range corresponds to +the ~ underlines + + how to make better diagnostic + */ +namespace clang { +namespace tidy { +namespace modernize { + +/// Register for all function calls, and filter malloc out +void NoMallocCheck::registerMatchers(MatchFinder *Finder) { + // register function calls to malloc + // Finder->addMatcher( + // callExpr(callee(functionDecl(hasName("::malloc")))).bind("malloc"), this); + + // 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 stmt_to_string(Stmt const *stmt, LangOptions l) { + // configuration + PrintingPolicy Policy(l); + + // printing + std::string stmt_str; + llvm::raw_string_ostream s(stmt_str); + stmt->printPretty(s, 0, Policy); + + return stmt_str; +} + +/// helper functions, to check if the expr is a sizeof expression +bool is_size_of(UnaryExprOrTypeTraitExpr const *expr) noexcept { + return expr->getKind() == UnaryExprOrTypeTrait::UETT_SizeOf; +} + +// try to infere the type of a potential sizeof expression, if it fails return +// false, otherwise true +bool infer_sizeof_type(Expr const *side, QualType &type) { + auto const size_of = dyn_cast(side); + if (size_of == nullptr) // could not cast expression to sizeof like expr + return false; + else { + // casted expression wasnt sizeof, maybe a typetrait of alignof + if (is_size_of(size_of) == false) + return false; + else { // we did find a sizeof expression, write the type and return true + type = size_of->getArgumentType(); + return true; + } + } +} + +std::string count_to_string(const BinaryOperator *expr, bool lhs_rhs_flag, + LangOptions l) { + const auto *count_expr = lhs_rhs_flag ? expr->getLHS() : expr->getRHS(); + return stmt_to_string(count_expr, l); +} + +} // namespace + +/// Check if the catched function call was malloc, and warn, propose new +void NoMallocCheck::check(const MatchFinder::MatchResult &Result) { + const auto *cast_match = Result.Nodes.getNodeAs("c_cast"); + if (cast_match != nullptr) + handle_cast(cast_match); + + const auto *malloc_match = Result.Nodes.getNodeAs("malloc"); + if (malloc_match != nullptr) + handle_malloc(malloc_match); + + const auto *realloc_match = Result.Nodes.getNodeAs("realloc"); + if (realloc_match != nullptr) + handle_realloc(realloc_match); + + const auto *free_match = Result.Nodes.getNodeAs("free"); + if (free_match != nullptr) + handle_free(free_match); +} + +/// handle cast expression, containing a malloc +/// (int*) malloc(sizeof(int) * 10); would be one example +void NoMallocCheck::handle_cast(const CStyleCastExpr *MatchedDecl) { + // first handle the malloc, check if a fix was supposed + const auto *malloc_call = + dyn_cast(MatchedDecl->getSubExprAsWritten()); + if (malloc_call == nullptr) + return; + + // handle the malloc, give information that is called from a cast expression + if (handle_malloc(malloc_call, true)) { + diag(MatchedDecl->getLocStart(), "Cast not needed anymore") + << FixItHint::CreateRemoval( + {MatchedDecl->getLocStart(), + malloc_call->getLocStart().getLocWithOffset(-1)}); + } +} + +/// implements the malloc check and its fixit hints +/// returns true, if a fix was supposed +bool NoMallocCheck::handle_malloc(const CallExpr *MatchedDecl, bool from_cast) { + // i belive this is done by CSA, but i added it just in case! + // does not work with c++, how to check on c only in testcases? + auto l = getLangOpts(); + if (not(l.CPlusPlus or l.CPlusPlus11 or l.CPlusPlus14)) + diag(MatchedDecl->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 arg = dyn_cast(MatchedDecl->getArg(0)); + + // the arguent to malloc was not a computation -> no more information + // achievable + if (arg == nullptr) { + return single_or_raw(MatchedDecl); + } + + // we have a binary operation, try to infer type and size information + else { + // multiplication should be the common case + // since malloc(sizeof(int) * count) - form + if (arg->isMultiplicativeOp()) { + return multiplication(MatchedDecl, arg); + } + // we dont have a multiplication, this is shady! + else { + suspicious_binary(MatchedDecl); + return false; + } + } + return false; // since the method is to big, just do it here. needs to get + // fixed +} + +/// implements the realloc check and its suggestions (std::vector) +void NoMallocCheck::handle_realloc(const CallExpr *MatchedDecl) { + diag(MatchedDecl->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{MatchedDecl->getLocStart(), MatchedDecl->getLocEnd()}; + diag(MatchedDecl->getLocStart(), "std::vector<>::resize does the job!", + DiagnosticIDs::Note); + give_deletion_hint(MatchedDecl); +} + +/// implements the free check and its suggestions (RAII) +void NoMallocCheck::handle_free(const CallExpr *MatchedDecl) { + diag(MatchedDecl->getLocStart(), "Do not use error prone C-Style memory " + "management. Consider using RAII Objects. " + "[R.10]") + << SourceRange{MatchedDecl->getLocStart(), MatchedDecl->getLocEnd()}; + give_deletion_hint(MatchedDecl); +} + +/// find out the type, if a sizeof expr exists +/// flag is true, when count will be on lhs +/// flag is false, when count will be on rhs +bool NoMallocCheck::determine_type_and_count(const BinaryOperator *arg, + QualType &type, + bool &lhs_rhs_flag) const { + auto const *lhs = arg->getLHS(); + auto const *rhs = arg->getRHS(); + + // on success, sets the type + // lhs inferes type + if (infer_sizeof_type(lhs, type) == true) { + lhs_rhs_flag = false; // rhs is count + return true; + } + // rhs inferes type + else if (infer_sizeof_type(rhs, type) == true) { + lhs_rhs_flag = true; // lhs is count + return true; + } + // could not infere type, therefore results are invalid! + else { + return false; + } +} + +/// handle malloc call that has not a binary expression as argument +/// returns true if a fix is suggested +bool NoMallocCheck::single_or_raw(const CallExpr *malloc_call) { + // check if its malloc(sizeof(int)) and transform to normal new + // otherwise warn + QualType t; + bool is_size_of_t = infer_sizeof_type(malloc_call->getArg(0), t); + + // we got a sizeof + if (is_size_of_t) { + Type const *type = t.split().asPair().first; + std::string replacement = + "new " + QualType::getAsString(type, Qualifiers()); + + diag(malloc_call->getLocStart(), + "Use new instead of malloc for object-creation. [R.10]") + << FixItHint::CreateReplacement( + {malloc_call->getLocStart(), malloc_call->getLocEnd()}, + replacement); + give_deletion_hint(malloc_call); + + return true; + } + // we got a suspicious malloc with no information + else { + diag(malloc_call->getLocStart(), "Could not infer your usecase of malloc!"); + give_call_hint(malloc_call); + + return false; + } +} + +/// handles malloc calls with multiplication as argument +/// returns true if a fix is suggested +bool NoMallocCheck::multiplication(const CallExpr *call, + const BinaryOperator *arg) { + // determine_type_and_count sets this flag. + // on true -> lhs is count expression + // on false -> rhs is count expression + bool lhs_rhs_flag = false; + QualType size_of_type; + + if (determine_type_and_count(arg, size_of_type, lhs_rhs_flag) == true) { + // 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 count_expr_str = + count_to_string(arg, lhs_rhs_flag, this->getLangOpts()); + + // remove qualifiers from the infered type + Type const *type = size_of_type.split().asPair().first; + // generate string from it + std::string t_hint = QualType::getAsString(type, Qualifiers()); + std::string replacement = "new " + t_hint + "[" + count_expr_str + "]"; + + // warn the malloc and give a fix hint with new[] syntax + diag(call->getLocStart(), + "Create Dynamic Arrays with std::vector(resizeable), " + "std::make_unique, gsl::dyn_array(both " + "dynamic, but fixed size) [Resource Management]") + << SourceRange{call->getLocStart(), call->getLocEnd()} + << FixItHint::CreateReplacement( + SourceRange{call->getLocStart(), call->getLocEnd()}, replacement); + + give_array_hint(call, t_hint, count_expr_str); + give_deletion_hint(call); + + return true; + } + // failed to infer type and count + else { + diag(call->getLocStart(), + "suspicious malloc. weired multiplication as argument!") + << SourceRange{call->getLocStart(), call->getLocEnd()}; + + give_call_hint(call); + + return false; + } +} + +/// warning about a suspicious, possibly buggy malloc call +void NoMallocCheck::suspicious_binary(const CallExpr *call) { + // warn the suspicious malloc call + diag(call->getLocStart(), + "Suspicious malloc. Argument is a Binary Operation, but no " + "multiplication.") + << SourceRange{call->getLocStart(), call->getLocEnd()}; + + // note for a possible bug + diag(call->getLocStart(), + "Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug", + DiagnosticIDs::Note) + << SourceRange{call->getLocStart(), call->getLocEnd()}; + + // note how to call properly + give_call_hint(call); +} + +/// hint how to call malloc properly as Diagnostic::Note +void NoMallocCheck::give_call_hint(const CallExpr *malloc_call) { + diag(malloc_call->getLocStart(), + "Write malloc in the form (type*) malloc(sizeof(type) * count)", + DiagnosticIDs::Note); +} + +/// notes about other ways to create a dynamic array +void NoMallocCheck::give_array_hint(const CallExpr *malloc_call, + const std::string &t_hint, + const std::string &count_str) { + std::string vec_hint = + "Consider std::vector<" + t_hint + ">(" + count_str + ")"; + std::string uni_hint = + "Consider std::make_unique<" + t_hint + "[]>(" + count_str + ")"; + std::string gsl_hint = + "Consider gsl::dyn_array<" + t_hint + ">(" + count_str + ")"; + + // notes on other possible replacements + diag(malloc_call->getLocStart(), vec_hint, DiagnosticIDs::Note); + diag(malloc_call->getLocStart(), uni_hint, DiagnosticIDs::Note); + diag(malloc_call->getLocStart(), gsl_hint, DiagnosticIDs::Note); +} + +/// give hint on deletion of the memory resource +void NoMallocCheck::give_deletion_hint(const CallExpr *malloc_call) { + diag(malloc_call->getLocStart(), + "You must manually remove free and replace it with delete/delete[]!", + DiagnosticIDs::Note); + diag(malloc_call->getLocStart(), "When using a RAII mechanism, remove the " + "explicit deleting of this variable!", + DiagnosticIDs::Note); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -100,6 +101,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,6 @@ +.. title:: clang-tidy - modernize-no-malloc + +modernize-no-malloc +=================== + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: test/clang-tidy/modernize-no-malloc.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-no-malloc.cpp @@ -0,0 +1,93 @@ +// RUN: %check_clang_tidy %s modernize-no-malloc %t + + +#include + + +void malloced_array() +{ + int* array0 = (int*) malloc(sizeof(int) * 20); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Cast not needed anymore [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: 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]]:26: note: Consider std::vector(20) +// CHECK-MESSAGES: :[[@LINE-5]]:26: note: Consider std::make_unique(20) +// CHECK-MESSAGES: :[[@LINE-6]]:26: note: Consider gsl::dyn_array(20) +// CHECK-MESSAGES: :[[@LINE-7]]:26: note: You must manually remove free and replace it with delete/delete[]! +// CHECK-MESSAGES: :[[@LINE-8]]:26: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + int* array1 = (int*) malloc(0 * sizeof(int)); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Cast not needed anymore [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: 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]]:26: note: Consider std::vector(0) +// CHECK-MESSAGES: :[[@LINE-5]]:26: note: Consider std::make_unique(0) +// CHECK-MESSAGES: :[[@LINE-6]]:26: note: Consider gsl::dyn_array(0) +// CHECK-MESSAGES: :[[@LINE-7]]:26: note: You must manually remove free and replace it with delete/delete[]! +// CHECK-MESSAGES: :[[@LINE-8]]:26: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + int* array2 = (int*) malloc(40); // very dangerous +// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: Could not infer your usecase of malloc! [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: 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]]:26: warning: Suspicious malloc. Argument is a Binary Operation, but no multiplication. [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: note: Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug +// CHECK-MESSAGES: :[[@LINE-3]]:26: 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]]:26: warning: Suspicious malloc. Argument is a Binary Operation, but no multiplication. [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: note: Check for the 'malloc(sizeof(int) * 10 + 1)' - Bug +// CHECK-MESSAGES: :[[@LINE-3]]:26: note: Write malloc in the form (type*) malloc(sizeof(type) * count) + + int* object = (int*) malloc(sizeof(int)); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Cast not needed anymore [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: Use new instead of malloc for object-creation. [R.10] [modernize-no-malloc] +// CHECK-FIXES: new int +// CHECK-MESSAGES: :[[@LINE-4]]:26: note: You must manually remove free and replace it with delete/delete[]! +// CHECK-MESSAGES: :[[@LINE-5]]:26: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + // reallocation memory, std::vector shall be used + char* realloced = (char*) realloc(array4, 50 * sizeof(int)); +// CHECK-MESSAGES: :[[@LINE-1]]:31: 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]]:31: note: std::vector<>::resize does the job! +// CHECK-MESSAGES: :[[@LINE-3]]:31: note: You must manually remove free and replace it with delete/delete[]! +// CHECK-MESSAGES: :[[@LINE-4]]:31: note: When using a RAII mechanism, remove the explicit deleting of this variable! + + // freeing memory the bad way + free(realloced); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Do not use error prone C-Style memory management. Consider using RAII Objects. [R.10] [modernize-no-malloc] +// CHECK-MESSAGES: :[[@LINE-2]]:5: note: You must manually remove free and replace it with delete/delete[]! +// CHECK-MESSAGES: :[[@LINE-3]]:5: 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) +}