https://github.com/Discookie updated https://github.com/llvm/llvm-project/pull/91951
>From 69cbd3da19eb0f8eb6758782b46daf99b5b79ea4 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Mon, 6 May 2024 06:11:58 +0000 Subject: [PATCH 01/10] Add `bugprone-virtual-arithmetic` check Finds pointer arithmetic on classes that declare a virtual function. --- .../bugprone/BugproneTidyModule.cpp | 3 ++ .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/VirtualArithmeticCheck.cpp | 46 +++++++++++++++++ .../bugprone/VirtualArithmeticCheck.h | 30 +++++++++++ .../checks/bugprone/virtual-arithmetic.rst | 50 +++++++++++++++++++ .../docs/clang-tidy/checks/list.rst | 1 + .../checkers/bugprone/virtual-arithmetic.cpp | 45 +++++++++++++++++ 7 files changed, 176 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 1b92d2e60cc17..813f6720074ae 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -91,6 +91,7 @@ #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" #include "UseAfterMoveCheck.h" +#include "VirtualArithmeticCheck.h" #include "VirtualNearMissCheck.h" namespace clang::tidy { @@ -254,6 +255,8 @@ class BugproneModule : public ClangTidyModule { CheckFactories.registerCheck<UnusedReturnValueCheck>( "bugprone-unused-return-value"); CheckFactories.registerCheck<UseAfterMoveCheck>("bugprone-use-after-move"); + CheckFactories.registerCheck<VirtualArithmeticCheck>( + "bugprone-virtual-arithmetic"); CheckFactories.registerCheck<VirtualNearMissCheck>( "bugprone-virtual-near-miss"); } diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 2d303191f8865..ec1f3231e7990 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -87,6 +87,7 @@ add_clang_library(clangTidyBugproneModule UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp UseAfterMoveCheck.cpp + VirtualArithmeticCheck.cpp VirtualNearMissCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp new file mode 100644 index 0000000000000..57347af2b5881 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp @@ -0,0 +1,46 @@ +//===--- VirtualArithmeticCheck.cpp - clang-tidy---------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "VirtualArithmeticCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) { + const auto PointerExprWithVirtualMethod = + expr(hasType(pointerType(pointee(hasDeclaration( + cxxRecordDecl(hasMethod(isVirtualAsWritten()))))))) + .bind("pointer"); + + const auto ArraySubscript = + arraySubscriptExpr(hasBase(PointerExprWithVirtualMethod)); + + const auto BinaryOperators = + binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="), + hasEitherOperand(PointerExprWithVirtualMethod)); + + const auto UnaryOperators = + unaryOperator(hasAnyOperatorName("++", "--"), + hasUnaryOperand(PointerExprWithVirtualMethod)); + + Finder->addMatcher( + expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this); +} + +void VirtualArithmeticCheck::check(const MatchFinder::MatchResult &Result) { + const auto *PointerExpr = Result.Nodes.getNodeAs<Expr>("pointer"); + + diag(PointerExpr->getBeginLoc(), + "pointer arithmetic on class that declares a virtual function, " + "undefined behavior if the pointee is a different class"); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h new file mode 100644 index 0000000000000..6a5f86a391747 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h @@ -0,0 +1,30 @@ +//===--- VirtualArithmeticCheck.h - clang-tidy-------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Finds pointer arithmetic on classes that declare a virtual function. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/virtual-arithmetic.html +class VirtualArithmeticCheck : public ClangTidyCheck { +public: + VirtualArithmeticCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst new file mode 100644 index 0000000000000..69d43e13392be --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst @@ -0,0 +1,50 @@ +.. title:: clang-tidy - bugprone-virtual-arithmetic + +bugprone-virtual-arithmetic +=========================== + +Warn if pointer arithmetic is performed on a class that declares a +virtual function. + +Pointer arithmetic on polymorphic objects where the pointer's static type is +different from its dynamic type is undefined behavior. +Finding pointers where the static type contains a virtual member function is a +good heuristic, as the pointer is likely to point to a different, derived class. + +This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer arithmetic on polymorphic objects <https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects>`_. + +Example: + +.. code-block:: c++ + + struct Base { + virtual void ~Base(); + }; + + struct Derived : public Base {}; + + void foo() { + Base *b = new Derived[10]; + + b += 1; + // warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class + + delete[] static_cast<Derived*>(b); + } + +Classes that do not declare a new virtual function are excluded, +as they make up the majority of false positives. + +.. code-block:: c++ + + void bar() { + Base *b = new Base[10]; + b += 1; // warning, as Base has a virtual destructor + + delete[] b; + + Derived *d = new Derived[10]; + d += 1; // no warning, as Derived overrides the destructor + + delete[] d; + } diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5cdaf9e35b6ac..ba1e5cafa6e47 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -157,6 +157,7 @@ Clang-Tidy Checks :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes" :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, :doc:`bugprone-use-after-move <bugprone/use-after-move>`, + :doc:`bugprone-virtual-arithmetic <bugprone/virtual-arithmetic>`, :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes" :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`, :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp new file mode 100644 index 0000000000000..d772112293cbc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp @@ -0,0 +1,45 @@ +// RUN: %check_clang_tidy %s bugprone-virtual-arithmetic %t + +class Base { +public: + virtual ~Base() {} +}; + +class Derived : public Base {}; + +void operators() { + Base *b = new Derived[10]; + + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + + b = b + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + + b++; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + + b[1]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + + delete[] static_cast<Derived*>(b); +} + +void subclassWarnings() { + Base *b = new Base[10]; + + // False positive that's impossible to distinguish without + // path-sensitive analysis, but the code is bug-prone regardless. + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + + delete[] b; + + // Common false positive is a class that overrides all parent functions. + Derived *d = new Derived[10]; + + d += 1; + // no-warning + + delete[] d; +} \ No newline at end of file >From 47068bd748cfb6f1b74b006d0f9918103f936a34 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Mon, 13 May 2024 09:43:46 +0000 Subject: [PATCH 02/10] Display the classname that has the virtual function Useful for reports within template instantiations. --- .../bugprone/VirtualArithmeticCheck.cpp | 7 ++++-- .../checkers/bugprone/virtual-arithmetic.cpp | 24 +++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp index 57347af2b5881..dec43ae9bd8ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp @@ -37,10 +37,13 @@ void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) { void VirtualArithmeticCheck::check(const MatchFinder::MatchResult &Result) { const auto *PointerExpr = Result.Nodes.getNodeAs<Expr>("pointer"); + const CXXRecordDecl *PointeeType = + PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl(); diag(PointerExpr->getBeginLoc(), - "pointer arithmetic on class that declares a virtual function, " - "undefined behavior if the pointee is a different class"); + "pointer arithmetic on class '%0' that declares a virtual function, " + "undefined behavior if the pointee is a different class") + << PointeeType->getName(); } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp index d772112293cbc..d89e75186c60f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp @@ -11,16 +11,16 @@ void operators() { Base *b = new Derived[10]; b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] b = b + 1; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] b++; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] b[1]; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] delete[] static_cast<Derived*>(b); } @@ -31,7 +31,7 @@ void subclassWarnings() { // False positive that's impossible to distinguish without // path-sensitive analysis, but the code is bug-prone regardless. b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function delete[] b; @@ -42,4 +42,18 @@ void subclassWarnings() { // no-warning delete[] d; +} + +template <typename T> +void templateWarning(T *t) { + // FIXME: Show the location of the template instantiation in diagnostic. + t += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function +} + +void functionArgument(Base *b) { + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function + + templateWarning(b); } \ No newline at end of file >From df2e3f1d40b674a8a49b678bf3403a5a4cb5e08b Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Thu, 23 May 2024 11:26:50 +0000 Subject: [PATCH 03/10] Rename check to `bugprone-pointer-arithmetic-on-polymorphic-object` --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 6 +++--- .../clang-tidy/bugprone/CMakeLists.txt | 2 +- ...ointerArithmeticOnPolymorphicObjectCheck.cpp} | 14 ++++++++------ ... PointerArithmeticOnPolymorphicObjectCheck.h} | 15 ++++++++------- ...pointer-arithmetic-on-polymorphic-object.rst} | 10 +++++----- .../docs/clang-tidy/checks/list.rst | 2 +- ...pointer-arithmetic-on-polymorphic-object.cpp} | 16 ++++++++-------- 7 files changed, 34 insertions(+), 31 deletions(-) rename clang-tools-extra/clang-tidy/bugprone/{VirtualArithmeticCheck.cpp => PointerArithmeticOnPolymorphicObjectCheck.cpp} (74%) rename clang-tools-extra/clang-tidy/bugprone/{VirtualArithmeticCheck.h => PointerArithmeticOnPolymorphicObjectCheck.h} (52%) rename clang-tools-extra/docs/clang-tidy/checks/bugprone/{virtual-arithmetic.rst => pointer-arithmetic-on-polymorphic-object.rst} (88%) rename clang-tools-extra/test/clang-tidy/checkers/bugprone/{virtual-arithmetic.cpp => pointer-arithmetic-on-polymorphic-object.cpp} (56%) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 813f6720074ae..689eb92a3d8d1 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -51,6 +51,7 @@ #include "NotNullTerminatedResultCheck.h" #include "OptionalValueConversionCheck.h" #include "ParentVirtualCallCheck.h" +#include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" @@ -91,7 +92,6 @@ #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" #include "UseAfterMoveCheck.h" -#include "VirtualArithmeticCheck.h" #include "VirtualNearMissCheck.h" namespace clang::tidy { @@ -172,6 +172,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-multiple-statement-macro"); CheckFactories.registerCheck<OptionalValueConversionCheck>( "bugprone-optional-value-conversion"); + CheckFactories.registerCheck<PointerArithmeticOnPolymorphicObjectCheck>( + "bugprone-pointer-arithmetic-on-polymorphic-object"); CheckFactories.registerCheck<RedundantBranchConditionCheck>( "bugprone-redundant-branch-condition"); CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>( @@ -255,8 +257,6 @@ class BugproneModule : public ClangTidyModule { CheckFactories.registerCheck<UnusedReturnValueCheck>( "bugprone-unused-return-value"); CheckFactories.registerCheck<UseAfterMoveCheck>("bugprone-use-after-move"); - CheckFactories.registerCheck<VirtualArithmeticCheck>( - "bugprone-virtual-arithmetic"); CheckFactories.registerCheck<VirtualNearMissCheck>( "bugprone-virtual-near-miss"); } diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index ec1f3231e7990..cb0d8ae98bac5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -48,6 +48,7 @@ add_clang_library(clangTidyBugproneModule NotNullTerminatedResultCheck.cpp OptionalValueConversionCheck.cpp ParentVirtualCallCheck.cpp + PointerArithmeticOnPolymorphicObjectCheck.cpp PosixReturnCheck.cpp RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp @@ -87,7 +88,6 @@ add_clang_library(clangTidyBugproneModule UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp UseAfterMoveCheck.cpp - VirtualArithmeticCheck.cpp VirtualNearMissCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp similarity index 74% rename from clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp rename to clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp index dec43ae9bd8ca..918c38b416f66 100644 --- a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp @@ -1,4 +1,4 @@ -//===--- VirtualArithmeticCheck.cpp - clang-tidy---------------------------===// +//===--- PointerArithmeticOnPolymorphicObjectCheck.cpp - clang-tidy--------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "VirtualArithmeticCheck.h" +#include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -14,7 +14,8 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { -void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) { +void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers( + MatchFinder *Finder) { const auto PointerExprWithVirtualMethod = expr(hasType(pointerType(pointee(hasDeclaration( cxxRecordDecl(hasMethod(isVirtualAsWritten()))))))) @@ -35,14 +36,15 @@ void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) { expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this); } -void VirtualArithmeticCheck::check(const MatchFinder::MatchResult &Result) { +void PointerArithmeticOnPolymorphicObjectCheck::check( + const MatchFinder::MatchResult &Result) { const auto *PointerExpr = Result.Nodes.getNodeAs<Expr>("pointer"); const CXXRecordDecl *PointeeType = PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl(); diag(PointerExpr->getBeginLoc(), - "pointer arithmetic on class '%0' that declares a virtual function, " - "undefined behavior if the pointee is a different class") + "pointer arithmetic on polymorphic class '%0' that declares a virtual " + "function, undefined behavior if the pointee is a different class") << PointeeType->getName(); } diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h similarity index 52% rename from clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h rename to clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h index 6a5f86a391747..0277d2b53af7b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h @@ -1,4 +1,4 @@ -//===--- VirtualArithmeticCheck.h - clang-tidy-------------------*- C++ -*-===// +//===--- PointerArithmeticOnPolymorphicObjectCheck.h ------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H #include "../ClangTidyCheck.h" @@ -16,10 +16,11 @@ namespace clang::tidy::bugprone { /// Finds pointer arithmetic on classes that declare a virtual function. /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/virtual-arithmetic.html -class VirtualArithmeticCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.html +class PointerArithmeticOnPolymorphicObjectCheck : public ClangTidyCheck { public: - VirtualArithmeticCheck(StringRef Name, ClangTidyContext *Context) + PointerArithmeticOnPolymorphicObjectCheck(StringRef Name, + ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -27,4 +28,4 @@ class VirtualArithmeticCheck : public ClangTidyCheck { } // namespace clang::tidy::bugprone -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_VIRTUAL_ARITHMETIC_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst similarity index 88% rename from clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst rename to clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst index 69d43e13392be..a3fa06918bbb3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/virtual-arithmetic.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst @@ -1,7 +1,7 @@ -.. title:: clang-tidy - bugprone-virtual-arithmetic +.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object -bugprone-virtual-arithmetic -=========================== +bugprone-pointer-arithmetic-on-polymorphic-object +================================================= Warn if pointer arithmetic is performed on a class that declares a virtual function. @@ -11,8 +11,6 @@ different from its dynamic type is undefined behavior. Finding pointers where the static type contains a virtual member function is a good heuristic, as the pointer is likely to point to a different, derived class. -This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer arithmetic on polymorphic objects <https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects>`_. - Example: .. code-block:: c++ @@ -48,3 +46,5 @@ as they make up the majority of false positives. delete[] d; } + +This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer arithmetic on polymorphic objects <https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects>`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ba1e5cafa6e47..80e4112a3b780 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -157,7 +157,7 @@ Clang-Tidy Checks :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes" :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, :doc:`bugprone-use-after-move <bugprone/use-after-move>`, - :doc:`bugprone-virtual-arithmetic <bugprone/virtual-arithmetic>`, + :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`, :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes" :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`, :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp similarity index 56% rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp index d89e75186c60f..c5c92d2ccd9ce 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/virtual-arithmetic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s bugprone-virtual-arithmetic %t +// RUN: %check_clang_tidy %s bugprone-pointer-arithmetic-on-polymorphic-object %t class Base { public: @@ -11,16 +11,16 @@ void operators() { Base *b = new Derived[10]; b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b = b + 1; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b++; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b[1]; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-virtual-arithmetic] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] delete[] static_cast<Derived*>(b); } @@ -31,7 +31,7 @@ void subclassWarnings() { // False positive that's impossible to distinguish without // path-sensitive analysis, but the code is bug-prone regardless. b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function delete[] b; @@ -48,12 +48,12 @@ template <typename T> void templateWarning(T *t) { // FIXME: Show the location of the template instantiation in diagnostic. t += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function } void functionArgument(Base *b) { b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on class 'Base' that declares a virtual function + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function templateWarning(b); } \ No newline at end of file >From fe0d5a7a5f24f75e7eb6e19c69e8d8ea37d7491f Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:10:38 +0000 Subject: [PATCH 04/10] Add `MatchInheritedVirtualFunctions` option --- ...nterArithmeticOnPolymorphicObjectCheck.cpp | 39 +++++-- ...ointerArithmeticOnPolymorphicObjectCheck.h | 10 +- clang-tools-extra/docs/ReleaseNotes.rst | 5 + ...inter-arithmetic-on-polymorphic-object.rst | 48 +++++---- ...r-arithmetic-on-polymorphic-object-all.cpp | 100 ++++++++++++++++++ ...hmetic-on-polymorphic-object-decl-only.cpp | 98 +++++++++++++++++ ...inter-arithmetic-on-polymorphic-object.cpp | 59 ----------- 7 files changed, 267 insertions(+), 92 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp index 918c38b416f66..6d4fc67a4b79e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp @@ -14,23 +14,42 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { +PointerArithmeticOnPolymorphicObjectCheck:: + PointerArithmeticOnPolymorphicObjectCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + MatchInheritedVirtualFunctions( + Options.get("MatchInheritedVirtualFunctions", false)) {} + +void PointerArithmeticOnPolymorphicObjectCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MatchInheritedVirtualFunctions", true); +} + void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers( MatchFinder *Finder) { + const auto PolymorphicPointerExpr = + expr(hasType(pointerType( + pointee(hasDeclaration(cxxRecordDecl(hasMethod(isVirtual()))))))) + .bind("pointer"); + const auto PointerExprWithVirtualMethod = - expr(hasType(pointerType(pointee(hasDeclaration( - cxxRecordDecl(hasMethod(isVirtualAsWritten()))))))) + expr(hasType(pointerType(pointee(hasDeclaration(cxxRecordDecl( + hasMethod(anyOf(isVirtualAsWritten(), isPure())))))))) .bind("pointer"); - const auto ArraySubscript = - arraySubscriptExpr(hasBase(PointerExprWithVirtualMethod)); + const auto SelectedPointerExpr = MatchInheritedVirtualFunctions + ? PolymorphicPointerExpr + : PointerExprWithVirtualMethod; + + const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr)); const auto BinaryOperators = binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="), - hasEitherOperand(PointerExprWithVirtualMethod)); + hasEitherOperand(SelectedPointerExpr)); - const auto UnaryOperators = - unaryOperator(hasAnyOperatorName("++", "--"), - hasUnaryOperand(PointerExprWithVirtualMethod)); + const auto UnaryOperators = unaryOperator( + hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr)); Finder->addMatcher( expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this); @@ -43,8 +62,8 @@ void PointerArithmeticOnPolymorphicObjectCheck::check( PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl(); diag(PointerExpr->getBeginLoc(), - "pointer arithmetic on polymorphic class '%0' that declares a virtual " - "function, undefined behavior if the pointee is a different class") + "pointer arithmetic on polymorphic class '%0', " + "undefined behavior if the pointee is a different class") << PointeeType->getName(); } diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h index 0277d2b53af7b..0144b840b3429 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h @@ -13,17 +13,21 @@ namespace clang::tidy::bugprone { -/// Finds pointer arithmetic on classes that declare a virtual function. +/// Finds pointer arithmetic performed on classes that declare a +/// virtual function. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.html class PointerArithmeticOnPolymorphicObjectCheck : public ClangTidyCheck { public: PointerArithmeticOnPolymorphicObjectCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool MatchInheritedVirtualFunctions; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d59da3a61b7b6..cc7038488b228 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -126,6 +126,11 @@ New checks reference. This may cause use-after-free errors if the caller uses xvalues as arguments. +- New :doc:`bugprone-pointer-arithmetic-on-polymorphic-object + <clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object>` check. + + Finds pointer arithmetic performed on classes that declare a virtual function. + - New :doc:`bugprone-suspicious-stringview-data-usage <clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst index a3fa06918bbb3..ac782d20a043f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst @@ -3,8 +3,7 @@ bugprone-pointer-arithmetic-on-polymorphic-object ================================================= -Warn if pointer arithmetic is performed on a class that declares a -virtual function. +Finds pointer arithmetic performed on classes that declare a virtual function. Pointer arithmetic on polymorphic objects where the pointer's static type is different from its dynamic type is undefined behavior. @@ -25,26 +24,35 @@ Example: Base *b = new Derived[10]; b += 1; - // warning: pointer arithmetic on class that declares a virtual function, undefined behavior if the pointee is a different class + // warning: pointer arithmetic on class that declares a virtual function, + // undefined behavior if the pointee is a different class delete[] static_cast<Derived*>(b); } -Classes that do not declare a new virtual function are excluded, -as they make up the majority of false positives. - -.. code-block:: c++ - - void bar() { - Base *b = new Base[10]; - b += 1; // warning, as Base has a virtual destructor - - delete[] b; - - Derived *d = new Derived[10]; - d += 1; // no warning, as Derived overrides the destructor - - delete[] d; - } - This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer arithmetic on polymorphic objects <https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects>`_. + +Options +------- + +.. option:: MatchInheritedVirtualFunctions + + When ``true``, all classes with a virtual function are considered, + even if the function is inherited. + Classes that do not declare a new virtual function are excluded + by default, as they make up the majority of false positives. + + .. code-block:: c++ + + void bar() { + Base *b = new Base[10]; + b += 1; // warning, as Base declares a virtual destructor + + delete[] b; + + Derived *d = new Derived[10]; // Derived overrides the destructor, and + // declares no other virtual functions + d += 1; // warning only if MatchVirtualDeclarationsOnly is set to true + + delete[] d; + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp new file mode 100644 index 0000000000000..2498c292b0027 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp @@ -0,0 +1,100 @@ +// RUN: %check_clang_tidy %s bugprone-pointer-arithmetic-on-polymorphic-object %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: {bugprone-pointer-arithmetic-on-polymorphic-object.MatchInheritedVirtualFunctions: true}}" + +class Base { +public: + virtual ~Base() {} +}; + +class Derived : public Base {}; + +class AbstractBase { +public: + virtual void f() = 0; + virtual ~AbstractBase() {} +}; + +class AbstractInherited : public AbstractBase {}; + +class AbstractOverride : public AbstractInherited { +public: + void f() override {} +}; + +void operators() { + Base *b = new Derived[10]; + + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b = b + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b++; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b[1]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + delete[] static_cast<Derived*>(b); +} + +void subclassWarnings() { + Base *b = new Base[10]; + + // False positive that's impossible to distinguish without + // path-sensitive analysis, but the code is bug-prone regardless. + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + delete[] b; + + // Common false positive is a class that overrides all parent functions. + // Matched due to the check configuration. + Derived *d = new Derived[10]; + + d += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' + + delete[] d; +} + +void abstractWarnings() { + // Classes with an abstract member funtion are always matched. + AbstractBase *ab = new AbstractOverride[10]; + + ab += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'AbstractBase' + + delete[] static_cast<AbstractOverride*>(ab); + + AbstractInherited *ai = new AbstractOverride[10]; + + ai += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'AbstractInherited' + + delete[] static_cast<AbstractOverride*>(ai); + + // Matched due to the check configuration. + AbstractOverride *ao = new AbstractOverride[10]; + + ao += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'AbstractOverride' + + delete[] ao; +} + +template <typename T> +void templateWarning(T *t) { + // FIXME: Show the location of the template instantiation in diagnostic. + t += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' +} + +void functionArgument(Base *b) { + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + templateWarning(b); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp new file mode 100644 index 0000000000000..4d5583c31d34e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp @@ -0,0 +1,98 @@ +// RUN: %check_clang_tidy %s bugprone-pointer-arithmetic-on-polymorphic-object %t + +class Base { +public: + virtual ~Base() {} +}; + +class Derived : public Base {}; + +class AbstractBase { +public: + virtual void f() = 0; + virtual ~AbstractBase() {} +}; + +class AbstractInherited : public AbstractBase {}; + +class AbstractOverride : public AbstractInherited { +public: + void f() override {} +}; + +void operators() { + Base *b = new Derived[10]; + + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b = b + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b++; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + b[1]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + + delete[] static_cast<Derived*>(b); +} + +void subclassWarnings() { + Base *b = new Base[10]; + + // False positive that's impossible to distinguish without + // path-sensitive analysis, but the code is bug-prone regardless. + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + delete[] b; + + // Common false positive is a class that overrides all parent functions. + Derived *d = new Derived[10]; + + d += 1; + // no-warning + + delete[] d; +} + +void abstractWarnings() { + // Classes with an abstract member funtion are always matched. + AbstractBase *ab = new AbstractOverride[10]; + + ab += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'AbstractBase' + + delete[] static_cast<AbstractOverride*>(ab); + + AbstractInherited *ai = new AbstractOverride[10]; + + // FIXME: Match abstract functions that were not overridden. + ai += 1; + // no-warning + + delete[] static_cast<AbstractOverride*>(ai); + + // If all abstract member functions are overridden, the class is not matched. + AbstractOverride *ao = new AbstractOverride[10]; + + ao += 1; + // no-warning + + delete[] ao; +} + +template <typename T> +void templateWarning(T *t) { + // FIXME: Show the location of the template instantiation in diagnostic. + t += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' +} + +void functionArgument(Base *b) { + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + templateWarning(b); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp deleted file mode 100644 index c5c92d2ccd9ce..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object.cpp +++ /dev/null @@ -1,59 +0,0 @@ -// RUN: %check_clang_tidy %s bugprone-pointer-arithmetic-on-polymorphic-object %t - -class Base { -public: - virtual ~Base() {} -}; - -class Derived : public Base {}; - -void operators() { - Base *b = new Derived[10]; - - b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] - - b = b + 1; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] - - b++; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] - - b[1]; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function, undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] - - delete[] static_cast<Derived*>(b); -} - -void subclassWarnings() { - Base *b = new Base[10]; - - // False positive that's impossible to distinguish without - // path-sensitive analysis, but the code is bug-prone regardless. - b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function - - delete[] b; - - // Common false positive is a class that overrides all parent functions. - Derived *d = new Derived[10]; - - d += 1; - // no-warning - - delete[] d; -} - -template <typename T> -void templateWarning(T *t) { - // FIXME: Show the location of the template instantiation in diagnostic. - t += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function -} - -void functionArgument(Base *b) { - b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' that declares a virtual function - - templateWarning(b); -} \ No newline at end of file >From f37392270da0540da49666cb09aeb971739fd2d8 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:11:55 +0000 Subject: [PATCH 05/10] Restrict to C++ --- .../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h index 0144b840b3429..057691a3d57c7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h @@ -25,6 +25,9 @@ class PointerArithmeticOnPolymorphicObjectCheck : public ClangTidyCheck { void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } private: const bool MatchInheritedVirtualFunctions; >From a29c9cdfc1c88aa8778c8acf9b0c7abe8ed7b786 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:42:05 +0000 Subject: [PATCH 06/10] Update check message and comments --- ...PointerArithmeticOnPolymorphicObjectCheck.cpp | 2 +- .../pointer-arithmetic-on-polymorphic-object.rst | 3 ++- ...nter-arithmetic-on-polymorphic-object-all.cpp | 16 +++++++++------- ...rithmetic-on-polymorphic-object-decl-only.cpp | 16 +++++++++------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp index 6d4fc67a4b79e..b994cab8c2a3b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp @@ -62,7 +62,7 @@ void PointerArithmeticOnPolymorphicObjectCheck::check( PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl(); diag(PointerExpr->getBeginLoc(), - "pointer arithmetic on polymorphic class '%0', " + "pointer arithmetic on polymorphic class '%0', which can result in " "undefined behavior if the pointee is a different class") << PointeeType->getName(); } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst index ac782d20a043f..598c14be04021 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst @@ -25,7 +25,8 @@ Example: b += 1; // warning: pointer arithmetic on class that declares a virtual function, - // undefined behavior if the pointee is a different class + // which can result in undefined behavior if the pointee is a + // different class delete[] static_cast<Derived*>(b); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp index 2498c292b0027..de731b5b7ead7 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp @@ -15,7 +15,9 @@ class AbstractBase { virtual ~AbstractBase() {} }; -class AbstractInherited : public AbstractBase {}; +class AbstractInherited : public AbstractBase { + void f() override = 0; +}; class AbstractOverride : public AbstractInherited { public: @@ -26,16 +28,16 @@ void operators() { Base *b = new Derived[10]; b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b = b + 1; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b++; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b[1]; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] delete[] static_cast<Derived*>(b); } @@ -51,7 +53,7 @@ void subclassWarnings() { delete[] b; // Common false positive is a class that overrides all parent functions. - // Matched due to the check configuration. + // Is a warning because of the check configuration. Derived *d = new Derived[10]; d += 1; @@ -76,7 +78,7 @@ void abstractWarnings() { delete[] static_cast<AbstractOverride*>(ai); - // Matched due to the check configuration. + // Is a warning because of the check configuration. AbstractOverride *ao = new AbstractOverride[10]; ao += 1; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp index 4d5583c31d34e..e511d7dc644d8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp @@ -13,7 +13,10 @@ class AbstractBase { virtual ~AbstractBase() {} }; -class AbstractInherited : public AbstractBase {}; +class AbstractInherited : public AbstractBase { + // FIXME: Check pure virtual functions transitively, not just explicit delete. + void f() override = 0; +}; class AbstractOverride : public AbstractInherited { public: @@ -24,16 +27,16 @@ void operators() { Base *b = new Derived[10]; b += 1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b = b + 1; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b++; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] b[1]; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] delete[] static_cast<Derived*>(b); } @@ -68,9 +71,8 @@ void abstractWarnings() { AbstractInherited *ai = new AbstractOverride[10]; - // FIXME: Match abstract functions that were not overridden. ai += 1; - // no-warning + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'AbstractInherited' delete[] static_cast<AbstractOverride*>(ai); >From 914f349c07b9aff0e5dfc4ba527b6bd8c9294881 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:43:10 +0000 Subject: [PATCH 07/10] Add support for type aliases --- ...nterArithmeticOnPolymorphicObjectCheck.cpp | 10 ++++++---- ...r-arithmetic-on-polymorphic-object-all.cpp | 20 +++++++++++++++++++ ...hmetic-on-polymorphic-object-decl-only.cpp | 20 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp index b994cab8c2a3b..16946000e2041 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp @@ -29,13 +29,15 @@ void PointerArithmeticOnPolymorphicObjectCheck::storeOptions( void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers( MatchFinder *Finder) { const auto PolymorphicPointerExpr = - expr(hasType(pointerType( - pointee(hasDeclaration(cxxRecordDecl(hasMethod(isVirtual()))))))) + expr(hasType(hasCanonicalType( + pointerType(pointee(hasCanonicalType(hasDeclaration( + cxxRecordDecl(cxxRecordDecl(hasMethod(isVirtual())))))))))) .bind("pointer"); const auto PointerExprWithVirtualMethod = - expr(hasType(pointerType(pointee(hasDeclaration(cxxRecordDecl( - hasMethod(anyOf(isVirtualAsWritten(), isPure())))))))) + expr(hasType(hasCanonicalType(pointerType( + pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl( + hasMethod(anyOf(isVirtualAsWritten(), isPure())))))))))) .bind("pointer"); const auto SelectedPointerExpr = MatchInheritedVirtualFunctions diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp index de731b5b7ead7..bdacf09576c93 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp @@ -100,3 +100,23 @@ void functionArgument(Base *b) { templateWarning(b); } + +using BaseAlias = Base; +using DerivedAlias = Derived; + +using BasePtr = Base*; +using DerivedPtr = Derived*; + +void typeAliases(BaseAlias *b, DerivedAlias *d, BasePtr bp, DerivedPtr dp) { + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + d += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' + + bp += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + dp += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp index e511d7dc644d8..a7b3c8a19f05d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp @@ -98,3 +98,23 @@ void functionArgument(Base *b) { templateWarning(b); } + +using BaseAlias = Base; +using DerivedAlias = Derived; + +using BasePtr = Base*; +using DerivedPtr = Derived*; + +void typeAliases(BaseAlias *b, DerivedAlias *d, BasePtr bp, DerivedPtr dp) { + b += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + d += 1; + // no-warning + + bp += 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' + + dp += 1; + // no-warning +} >From 801d7ec82550b41fd56ae34966ec5dcfb73112be Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:46:47 +0000 Subject: [PATCH 08/10] Add tests for prefix `++` --- .../bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp | 3 +++ .../pointer-arithmetic-on-polymorphic-object-decl-only.cpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp index bdacf09576c93..5bf96b44b56b9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp @@ -36,6 +36,9 @@ void operators() { b++; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + --b; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + b[1]; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp index a7b3c8a19f05d..3718c622596cc 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp @@ -35,6 +35,9 @@ void operators() { b++; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + --b; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] + b[1]; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base', which can result in undefined behavior if the pointee is a different class [bugprone-pointer-arithmetic-on-polymorphic-object] >From e5dc6b21cd9e810a0446c788518e690b3c5eff51 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 03:57:18 +0000 Subject: [PATCH 09/10] Add comment about the UB's cause --- .../bugprone/pointer-arithmetic-on-polymorphic-object.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst index 598c14be04021..b22676f5284d6 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst @@ -6,7 +6,8 @@ bugprone-pointer-arithmetic-on-polymorphic-object Finds pointer arithmetic performed on classes that declare a virtual function. Pointer arithmetic on polymorphic objects where the pointer's static type is -different from its dynamic type is undefined behavior. +different from its dynamic type is undefined behavior, as the two types can +have different sizes. Finding pointers where the static type contains a virtual member function is a good heuristic, as the pointer is likely to point to a different, derived class. >From dc7e63cb7e462adafde499849633e7bf92829755 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 29 May 2024 04:04:25 +0000 Subject: [PATCH 10/10] Exclude final classes --- ...nterArithmeticOnPolymorphicObjectCheck.cpp | 6 ++++-- ...r-arithmetic-on-polymorphic-object-all.cpp | 21 ++++++++++++++++++- ...hmetic-on-polymorphic-object-decl-only.cpp | 21 ++++++++++++++++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp index 16946000e2041..81cbcfec8d43d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp @@ -31,13 +31,15 @@ void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers( const auto PolymorphicPointerExpr = expr(hasType(hasCanonicalType( pointerType(pointee(hasCanonicalType(hasDeclaration( - cxxRecordDecl(cxxRecordDecl(hasMethod(isVirtual())))))))))) + cxxRecordDecl(cxxRecordDecl(hasMethod(isVirtual())), + unless(isFinal()))))))))) .bind("pointer"); const auto PointerExprWithVirtualMethod = expr(hasType(hasCanonicalType(pointerType( pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl( - hasMethod(anyOf(isVirtualAsWritten(), isPure())))))))))) + hasMethod(anyOf(isVirtualAsWritten(), isPure())), + unless(isFinal()))))))))) .bind("pointer"); const auto SelectedPointerExpr = MatchInheritedVirtualFunctions diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp index 5bf96b44b56b9..d8213416295ac 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-all.cpp @@ -9,6 +9,8 @@ class Base { class Derived : public Base {}; +class FinalDerived final : public Base {}; + class AbstractBase { public: virtual void f() = 0; @@ -63,6 +65,14 @@ void subclassWarnings() { // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' delete[] d; + + // Final classes cannot have a dynamic type. + FinalDerived *fd = new FinalDerived[10]; + + fd += 1; + // no-warning + + delete[] fd; } void abstractWarnings() { @@ -106,20 +116,29 @@ void functionArgument(Base *b) { using BaseAlias = Base; using DerivedAlias = Derived; +using FinalDerivedAlias = FinalDerived; using BasePtr = Base*; using DerivedPtr = Derived*; +using FinalDerivedPtr = FinalDerived*; -void typeAliases(BaseAlias *b, DerivedAlias *d, BasePtr bp, DerivedPtr dp) { +void typeAliases(BaseAlias *b, DerivedAlias *d, FinalDerivedAlias *fd, + BasePtr bp, DerivedPtr dp, FinalDerivedPtr fdp) { b += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' d += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' + fd += 1; + // no-warning + bp += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' dp += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Derived' + + fdp += 1; + // no-warning } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp index 3718c622596cc..c6cd9d843c7e9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/pointer-arithmetic-on-polymorphic-object-decl-only.cpp @@ -7,6 +7,8 @@ class Base { class Derived : public Base {}; +class FinalDerived final : public Base {}; + class AbstractBase { public: virtual void f() = 0; @@ -61,6 +63,14 @@ void subclassWarnings() { // no-warning delete[] d; + + // Final classes cannot have a dynamic type. + FinalDerived *fd = new FinalDerived[10]; + + fd += 1; + // no-warning + + delete[] fd; } void abstractWarnings() { @@ -104,20 +114,29 @@ void functionArgument(Base *b) { using BaseAlias = Base; using DerivedAlias = Derived; +using FinalDerivedAlias = FinalDerived; using BasePtr = Base*; using DerivedPtr = Derived*; +using FinalDerivedPtr = FinalDerived*; -void typeAliases(BaseAlias *b, DerivedAlias *d, BasePtr bp, DerivedPtr dp) { +void typeAliases(BaseAlias *b, DerivedAlias *d, FinalDerivedAlias *fd, + BasePtr bp, DerivedPtr dp, FinalDerivedPtr fdp) { b += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' d += 1; // no-warning + fd += 1; + // no-warning + bp += 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: pointer arithmetic on polymorphic class 'Base' dp += 1; // no-warning + + fdp += 1; + // no-warning } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits