[clang-tools-extra] r248791 - Adding a checker (misc-new-delete-overloads) that detects mismatched overloads of operator new and operator delete. Corresponds to the CERT C++ secure coding rule: https://www.securecoding.cert.org/confluence/display/cplusplus/DCL54-CPP.+Overload+allocation+and+deallocation+functions+as+a+pair+in+the+same+scope
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 29 06:12:22 PDT 2015
Author: aaronballman
Date: Tue Sep 29 08:12:21 2015
New Revision: 248791
URL: http://llvm.org/viewvc/llvm-project?rev=248791&view=rev
Log:
Adding a checker (misc-new-delete-overloads) that detects mismatched overloads of operator new and operator delete. Corresponds to the CERT C++ secure coding rule: https://www.securecoding.cert.org/confluence/display/cplusplus/DCL54-CPP.+Overload+allocation+and+deallocation+functions+as+a+pair+in+the+same+scope
Added:
clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst
clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=248791&r1=248790&r2=248791&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Sep 29 08:12:21 2015
@@ -11,6 +11,7 @@ add_clang_library(clangTidyMiscModule
MacroRepeatedSideEffectsCheck.cpp
MiscTidyModule.cpp
MoveConstructorInitCheck.cpp
+ NewDeleteOverloadsCheck.cpp
NoexceptMoveConstructorCheck.cpp
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=248791&r1=248790&r2=248791&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Sep 29 08:12:21 2015
@@ -19,6 +19,7 @@
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "MoveConstructorInitCheck.h"
+#include "NewDeleteOverloadsCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
@@ -53,6 +54,8 @@ public:
"misc-macro-repeated-side-effects");
CheckFactories.registerCheck<MoveConstructorInitCheck>(
"misc-move-constructor-init");
+ CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
+ "misc-new-delete-overloads");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"misc-noexcept-move-constructor");
CheckFactories.registerCheck<SizeofContainerCheck>(
Added: clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp?rev=248791&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp Tue Sep 29 08:12:21 2015
@@ -0,0 +1,215 @@
+//===--- NewDeleteOverloadsCheck.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 "NewDeleteOverloadsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace {
+AST_MATCHER(FunctionDecl, isPlacementOverload) {
+ bool New;
+ switch (Node.getOverloadedOperator()) {
+ default:
+ return false;
+ case OO_New:
+ case OO_Array_New:
+ New = true;
+ break;
+ case OO_Delete:
+ case OO_Array_Delete:
+ New = false;
+ break;
+ }
+
+ // Variadic functions are always placement functions.
+ if (Node.isVariadic())
+ return true;
+
+ // Placement new is easy: it always has more than one parameter (the first
+ // parameter is always the size). If it's an overload of delete or delete[]
+ // that has only one parameter, it's never a placement delete.
+ if (New)
+ return Node.getNumParams() > 1;
+ if (Node.getNumParams() == 1)
+ return false;
+
+ // Placement delete is a little more challenging. They always have more than
+ // one parameter with the first parameter being a pointer. However, the
+ // second parameter can be a size_t for sized deallocation, and that is never
+ // a placement delete operator.
+ if (Node.getNumParams() <= 1 || Node.getNumParams() > 2)
+ return true;
+
+ const auto *FPT = Node.getType()->castAs<FunctionProtoType>();
+ ASTContext &Ctx = Node.getASTContext();
+ if (Ctx.getLangOpts().SizedDeallocation &&
+ Ctx.hasSameType(FPT->getParamType(1), Ctx.getSizeType()))
+ return false;
+
+ return true;
+}
+} // namespace
+
+namespace tidy {
+namespace misc {
+
+namespace {
+OverloadedOperatorKind getCorrespondingOverload(const FunctionDecl *FD) {
+ switch (FD->getOverloadedOperator()) {
+ default: break;
+ case OO_New:
+ return OO_Delete;
+ case OO_Delete:
+ return OO_New;
+ case OO_Array_New:
+ return OO_Array_Delete;
+ case OO_Array_Delete:
+ return OO_Array_New;
+ }
+ llvm_unreachable("Not an overloaded allocation operator");
+}
+
+const char *getOperatorName(OverloadedOperatorKind K) {
+ switch (K) {
+ default: break;
+ case OO_New:
+ return "operator new";
+ case OO_Delete:
+ return "operator delete";
+ case OO_Array_New:
+ return "operator new[]";
+ case OO_Array_Delete:
+ return "operator delete[]";
+ }
+ llvm_unreachable("Not an overloaded allocation operator");
+}
+
+bool areCorrespondingOverloads(const FunctionDecl *LHS,
+ const FunctionDecl *RHS) {
+ return RHS->getOverloadedOperator() == getCorrespondingOverload(LHS);
+}
+
+bool hasCorrespondingOverloadInOneClass(const CXXRecordDecl *RD,
+ const CXXMethodDecl *MD) {
+ // Check the methods in the given class and accessible to derived classes.
+ for (const auto *BMD : RD->methods())
+ if (BMD->isOverloadedOperator() && BMD->getAccess() != AS_private &&
+ areCorrespondingOverloads(MD, BMD))
+ return true;
+
+ // Check base classes.
+ for (const auto &BS : RD->bases())
+ if (hasCorrespondingOverloadInOneClass(BS.getType()->getAsCXXRecordDecl(),
+ MD))
+ return true;
+
+ return false;
+}
+bool hasCorrespondingOverloadInBaseClass(const CXXMethodDecl *MD) {
+ // Get the parent class of the method; we do not need to care about checking
+ // the methods in this class as the caller has already done that by looking
+ // at the declaration contexts.
+ const CXXRecordDecl *RD = MD->getParent();
+
+ for (const auto &BS : RD->bases())
+ if (hasCorrespondingOverloadInOneClass(BS.getType()->getAsCXXRecordDecl(),
+ MD))
+ return true;
+
+ return false;
+}
+} // anonymous namespace
+
+void NewDeleteOverloadsCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // Match all operator new and operator delete overloads (including the array
+ // forms). Do not match implicit operators, placement operators, or
+ // deleted/private operators.
+ //
+ // Technically, trivially-defined operator delete seems like a reasonable
+ // thing to also skip. e.g., void operator delete(void *) {}
+ // However, I think it's more reasonable to warn in this case as the user
+ // should really be writing that as a deleted function.
+ Finder->addMatcher(
+ functionDecl(
+ unless(anyOf(isImplicit(), isPlacementOverload(), isDeleted(),
+ cxxMethodDecl(isPrivate()))),
+ anyOf(hasOverloadedOperatorName("new"),
+ hasOverloadedOperatorName("new[]"),
+ hasOverloadedOperatorName("delete"),
+ hasOverloadedOperatorName("delete[]")))
+ .bind("func"),
+ this);
+}
+
+void NewDeleteOverloadsCheck::check(const MatchFinder::MatchResult &Result) {
+ // Add any matches we locate to the list of things to be checked at the
+ // end of the translation unit.
+ const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const CXXRecordDecl *RD = nullptr;
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(FD))
+ RD = MD->getParent();
+ Overloads[RD].push_back(FD);
+}
+
+void NewDeleteOverloadsCheck::onEndOfTranslationUnit() {
+ // Walk over the list of declarations we've found to see if there is a
+ // corresponding overload at the same declaration context or within a base
+ // class. If there is not, add the element to the list of declarations to
+ // diagnose.
+ SmallVector<const FunctionDecl *, 4> Diagnose;
+ for (const auto &RP : Overloads) {
+ // We don't care about the CXXRecordDecl key in the map; we use it as a way
+ // to shard the overloads by declaration context to reduce the algorithmic
+ // complexity when searching for corresponding free store functions.
+ for (const auto *Overload : RP.second) {
+ const auto *Match = std::find_if(
+ RP.second.begin(), RP.second.end(), [&](const FunctionDecl *FD) {
+ if (FD == Overload)
+ return false;
+ // If the declaration contexts don't match, we don't
+ // need to check
+ // any further.
+ if (FD->getDeclContext() != Overload->getDeclContext())
+ return false;
+
+ // Since the declaration contexts match, see whether
+ // the current
+ // element is the corresponding operator.
+ if (!areCorrespondingOverloads(Overload, FD))
+ return false;
+
+ return true;
+ });
+
+ if (Match == RP.second.end()) {
+ // Check to see if there is a corresponding overload in a base class
+ // context. If there isn't, or if the overload is not a class member
+ // function, then we should diagnose.
+ const auto *MD = dyn_cast<CXXMethodDecl>(Overload);
+ if (!MD || !hasCorrespondingOverloadInBaseClass(MD))
+ Diagnose.push_back(Overload);
+ }
+ }
+ }
+
+ for (const auto *FD : Diagnose)
+ diag(FD->getLocation(), "declaration of %0 has no matching declaration "
+ "of '%1' at the same scope")
+ << FD << getOperatorName(getCorrespondingOverload(FD));
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.h?rev=248791&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.h Tue Sep 29 08:12:21 2015
@@ -0,0 +1,37 @@
+//===--- NewDeleteOverloadsCheck.h - clang-tidy----------------------------===//
+//
+// 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_MISC_NEWDELETEOVERLOADS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NEWDELETEOVERLOADS_H
+
+#include "../ClangTidy.h"
+#include "llvm/ADT/SmallVector.h"
+#include <map>
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class NewDeleteOverloadsCheck : public ClangTidyCheck {
+ std::map<const clang::CXXRecordDecl *,
+ llvm::SmallVector<const clang::FunctionDecl *, 4>> Overloads;
+
+public:
+ NewDeleteOverloadsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void onEndOfTranslationUnit() override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NEWDELETEOVERLOADS_H
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=248791&r1=248790&r2=248791&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Sep 29 08:12:21 2015
@@ -30,6 +30,7 @@ List of clang-tidy Checks
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-move-constructor-init
+ misc-new-delete-overloads
misc-noexcept-move-constructor
misc-sizeof-container
misc-static-assert
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst?rev=248791&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst Tue Sep 29 08:12:21 2015
@@ -0,0 +1,14 @@
+misc-new-delete-overloads
+=========================
+
+The check flags overloaded operator new() and operator delete() functions that
+do not have a corresponding free store function defined within the same scope.
+For instance, the check will flag a class implementation of a non-placement
+operator new() when the class does not also define a non-placement operator
+delete() function as well.
+
+The check does not flag implicitly-defined operators, deleted or private
+operators, or placement operators.
+
+This check corresponds to CERT C++ Coding Standard rule `DCL54-CPP. Overload allocation and deallocation functions as a pair in the same scope
+<https://www.securecoding.cert.org/confluence/display/cplusplus/DCL54-CPP.+Overload+allocation+and+deallocation+functions+as+a+pair+in+the+same+scope>`_.
Added: clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp?rev=248791&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp Tue Sep 29 08:12:21 2015
@@ -0,0 +1,18 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-new-delete-overloads %t -- -std=c++14 -fsized-deallocation
+
+struct S {
+ // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: declaration of 'operator delete' has no matching declaration of 'operator new' at the same scope [misc-new-delete-overloads]
+ void operator delete(void *ptr, size_t) noexcept; // not a placement delete
+};
+
+struct T {
+ // Because we have enabled sized deallocations explicitly, this new/delete
+ // pair matches.
+ void *operator new(size_t size) noexcept;
+ void operator delete(void *ptr, size_t) noexcept; // ok because sized deallocation is enabled
+};
+
+// While we're here, check that global operator delete with no operator new
+// is also matched.
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: declaration of 'operator delete' has no matching declaration of 'operator new' at the same scope
+void operator delete(void *ptr) noexcept;
Added: clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp?rev=248791&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp Tue Sep 29 08:12:21 2015
@@ -0,0 +1,75 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-new-delete-overloads %t -- -std=c++14
+
+struct S {
+ // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope [misc-new-delete-overloads]
+ void *operator new(size_t size) noexcept;
+ // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new[]' has no matching declaration of 'operator delete[]' at the same scope
+ void *operator new[](size_t size) noexcept;
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
+void *operator new(size_t size) noexcept;
+
+struct T {
+ // Sized deallocations are not enabled by default, and so this new/delete pair
+ // does not match. However, we expect only one warning, for the new, because
+ // the operator delete is a placement delete and we do not warn on mismatching
+ // placement operations.
+ // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
+ void *operator new(size_t size) noexcept;
+ void operator delete(void *ptr, size_t) noexcept; // ok only if sized deallocation is enabled
+};
+
+struct U {
+ void *operator new(size_t size) noexcept;
+ void operator delete(void *ptr) noexcept;
+
+ void *operator new[](size_t) noexcept;
+ void operator delete[](void *) noexcept;
+};
+
+struct Z {
+ // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: declaration of 'operator delete' has no matching declaration of 'operator new' at the same scope
+ void operator delete(void *ptr) noexcept;
+ // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: declaration of 'operator delete[]' has no matching declaration of 'operator new[]' at the same scope
+ void operator delete[](void *ptr) noexcept;
+};
+
+struct A {
+ void *operator new(size_t size, Z) noexcept; // ok, placement new
+};
+
+struct B {
+ void operator delete(void *ptr, A) noexcept; // ok, placement delete
+};
+
+// It is okay to have a class with an inaccessible free store operator.
+struct C {
+ void *operator new(size_t, A) noexcept; // ok, placement new
+private:
+ void operator delete(void *) noexcept;
+};
+
+// It is also okay to have a class with a delete free store operator.
+struct D {
+ void *operator new(size_t, A) noexcept; // ok, placement new
+ void operator delete(void *) noexcept = delete;
+};
+
+struct E : U {
+ void *operator new(size_t) noexcept; // okay, we inherit operator delete from U
+};
+
+struct F : S {
+ // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
+ void *operator new(size_t) noexcept;
+};
+
+class G {
+ void operator delete(void *) noexcept;
+};
+
+struct H : G {
+ // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
+ void *operator new(size_t) noexcept; // base class operator is inaccessible
+};
More information about the cfe-commits
mailing list