[clang-tidy] Function names configurable for cppcoreguidelines-nomalloc - checker

Summary:
Hello everybody,

this is an incremental patch for the NoMalloc-Checker I wrote. It allows to configure the memory-management functions, that are checked,
This might be helpful for a code base with custom functions in use, or non-standard functionality, like posix_memalign.

Reviewers: aaron.ballman, hokein, alexfh

Reviewed By: aaron.ballman, alexfh

Subscribers: sbenza, nemanjai, JDevlieghere

Tags: #clang-tools-extra

Patch by Jonas Toth!

Differential Revision: https://reviews.llvm.org/D28239

llvm-svn: 296734

Author: alexfh

clang-tools-extra/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp

@@ -8,43 +8,63 @@
 //===----------------------------------------------------------------------===//
 
 #include "NoMallocCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include <iostream>
+#include <algorithm>
 #include <string>
+#include <vector>
 
 using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
 
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
+namespace {
+Matcher<FunctionDecl> hasAnyListedName(const std::string &FunctionNames) {
+  const std::vector<std::string> NameList =
+      utils::options::parseStringList(FunctionNames);
+  return hasAnyName(std::vector<StringRef>(NameList.begin(), NameList.end()));
+}
+}
+
+void NoMallocCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Allocations", AllocList);
+  Options.store(Opts, "Reallocations", ReallocList);
+  Options.store(Opts, "Deallocations", DeallocList);
+}
+
 void NoMallocCheck::registerMatchers(MatchFinder *Finder) {
   // C-style memory management is only problematic in C++.
   if (!getLangOpts().CPlusPlus)
     return;
 
   // Registering malloc, will suggest RAII.
-  Finder->addMatcher(
-      callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"))))
-          .bind("aquisition"),
-      this);
+  Finder->addMatcher(callExpr(callee(functionDecl(hasAnyListedName(AllocList))))
+                         .bind("allocation"),
+                     this);
 
   // Registering realloc calls, suggest std::vector or std::string.
   Finder->addMatcher(
-      callExpr(callee(functionDecl(hasName("::realloc")))).bind("realloc"),
+      callExpr(callee(functionDecl(hasAnyListedName(ReallocList))))
+          .bind("realloc"),
       this);
 
   // Registering free calls, will suggest RAII instead.
   Finder->addMatcher(
-      callExpr(callee(functionDecl(hasName("::free")))).bind("free"), this);
+      callExpr(callee(functionDecl(hasAnyListedName(DeallocList))))
+          .bind("free"),
+      this);
 }
 
 void NoMallocCheck::check(const MatchFinder::MatchResult &Result) {
   const CallExpr *Call = nullptr;
   StringRef Recommendation;
 
-  if ((Call = Result.Nodes.getNodeAs<CallExpr>("aquisition")))
+  if ((Call = Result.Nodes.getNodeAs<CallExpr>("allocation")))
     Recommendation = "consider a container or a smart pointer";
   else if ((Call = Result.Nodes.getNodeAs<CallExpr>("realloc")))
     Recommendation = "consider std::vector or std::string";

clang-tools-extra/clang-tidy/cppcoreguidelines/NoMallocCheck.h

@@ -27,14 +27,32 @@ namespace cppcoreguidelines {
 /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html
 class NoMallocCheck : public ClangTidyCheck {
 public:
+  /// Construct Checker and read in configuration for function names.
   NoMallocCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        AllocList(Options.get("Allocations", "::malloc;::calloc")),
+        ReallocList(Options.get("Reallocations", "::realloc")),
+        DeallocList(Options.get("Deallocations", "::free")) {}
+
+  /// Make configuration of checker discoverable.
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
   /// Registering for malloc, calloc, realloc and free calls.
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
 
   /// Checks matched function calls and gives suggestion to modernize the code.
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  /// Semicolon-seperated list of fully qualified names of memory allocation
+  /// functions the check warns about. Defaults to `::malloc;::calloc`.
+  const std::string AllocList;
+  /// Semicolon-seperated list of fully qualified names of memory reallocation 
+  /// functions the check warns about. Defaults to `::realloc`.
+  const std::string ReallocList;
+  /// Semicolon-seperated list of fully qualified names of memory deallocation 
+  /// functions the check warns about. Defaults to `::free`.
+  const std::string DeallocList;
 };
 
 } // namespace cppcoreguidelines

clang-tools-extra/docs/ReleaseNotes.rst

@@ -62,6 +62,11 @@ Improvements to clang-tidy
 
   Finds modification of the ``std`` or ``posix`` namespace.
 
+- Improved `cppcoreguidelines-no-malloc 
+  <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html>`_ check
+
+  Allow custom memory management functions to be considered as well.
+
 - New `readability-misleading-indentation
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst

@@ -6,8 +6,9 @@ cppcoreguidelines-no-malloc
 This check handles C-Style memory management using ``malloc()``, ``realloc()``,
 ``calloc()`` and ``free()``. It warns about its use and tries to suggest the use
 of an appropriate RAII object.
-See `C++ Core Guidelines
-<https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-mallocfree>`.
+Furthermore, it can be configured to check against a user-specified list of functions 
+that are used for memory management (e.g. ``posix_memalign()``).
+See `C++ Core Guidelines <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-mallocfree>`_.
 
 There is no attempt made to provide fix-it hints, since manual resource
 management isn't easily transformed automatically into RAII.
@@ -25,3 +26,21 @@ management isn't easily transformed automatically into RAII.
   // Rather use a smartpointer or stack variable.
   struct some_struct* s = (struct some_struct*) malloc(sizeof(struct some_struct));
 
+Options
+-------
+
+.. option:: Allocations
+
+   Semicolon-separated list of fully qualified names of memory allocation functions. 
+   Defaults to ``::malloc;::calloc``.
+
+.. option:: Deallocations
+
+   Semicolon-separated list of fully qualified names of memory allocation functions. 
+   Defaults to ``::free``.
+
+.. option:: Reallocations
+
+   Semicolon-separated list of fully qualified names of memory allocation functions. 
+   Defaults to ``::realloc``.
+

clang-tools-extra/test/clang-tidy/cppcoreguidelines-no-malloc-custom.cpp

@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-no-malloc.Allocations, value: "::malloc;::align_malloc;::calloc"},\
+// RUN:   {key: cppcoreguidelines-no-malloc.Reallocations, value: "::realloc;::align_realloc"},\
+// RUN:   {key: cppcoreguidelines-no-malloc.Deallocations, value: "::free;::align_free"}]}' \
+// RUN: --
+
+using size_t = __SIZE_TYPE__;
+
+void *malloc(size_t size);
+void *align_malloc(size_t size, unsigned short aligmnent);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void *align_realloc(void *ptr, size_t size, unsigned short alignment);
+void free(void *ptr);
+void *align_free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *aligned = (int *)align_malloc(20 * sizeof(int), 16);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  char *align_realloced = (char *)align_realloc(aligned, 50 * sizeof(int), 16);
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  align_free(align_realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// 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
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}

clang-tools-extra/test/clang-tidy/cppcoreguidelines-no-malloc-no-functions.cpp

@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-no-malloc.Allocations, value: "::malloc"},\
+// RUN:   {key: cppcoreguidelines-no-malloc.Reallocations, value: ""},\
+// RUN:   {key: cppcoreguidelines-no-malloc.Deallocations, value: ""}]}' \
+// RUN: --
+
+// Just ensure, the check will not crash, when no functions shall be checked.
+
+using size_t = __SIZE_TYPE__;
+
+void *malloc(size_t size);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}