[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-04 Thread via cfe-commits

https://github.com/Discookie closed 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-04 Thread via cfe-commits


@@ -0,0 +1,81 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreInheritedVirtualFunctions(
+  Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+   hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+  .bind("pointee"
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+   .bind("pointee"
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+   ? PointerExprWithVirtualMethod
+   : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on polymorphic object of type '%0' can result in "
+   "undefined behavior if the dynamic type differs from the pointer type")
+  << PointeeDecl->getName() << PointeeDecl->getSourceRange();

Discookie wrote:

Whoops that's a typo, well spotted! Fixed.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-03 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,81 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreInheritedVirtualFunctions(
+  Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+   hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+  .bind("pointee"
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+   .bind("pointee"
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+   ? PointerExprWithVirtualMethod
+   : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on polymorphic object of type '%0' can result in "
+   "undefined behavior if the dynamic type differs from the pointer type")
+  << PointeeDecl->getName() << PointeeDecl->getSourceRange();

PiotrZSL wrote:

I'm not fun of "PointeeDecl->getSourceRange();" here, should be 
"PointerExpr->getSourceRange();".
Problem with `PointeeDecl->getSourceRange();` is that it may highlight entire 
class that may be like 100 lines long, this will make result... ugly.

If you want to point decl, then simply emit second Note with message "class %0 
is defined here"


https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-03 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-03 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,81 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreInheritedVirtualFunctions(
+  Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+   hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+  .bind("pointee"
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+   .bind("pointee"
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+   ? PointerExprWithVirtualMethod
+   : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on polymorphic object of type '%0' can result in "
+   "undefined behavior if the dynamic type differs from the pointer type")
+  << PointeeDecl->getName() << PointeeDecl->getSourceRange();

PiotrZSL wrote:

`PointeeDecl->getName()` this could cause some issues (crash) for some 
potentially unnamed classes.
In theory simple "<< PointeeDecl; " should work, please verify.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-03 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL approved this pull request.

Some small issues with diagnostic left.
Except that, looks good enough.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-07-03 Thread via cfe-commits


@@ -0,0 +1,81 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreInheritedVirtualFunctions(
+  Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+   hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+  .bind("pointee"
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+   .bind("pointee"
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+   ? PointerExprWithVirtualMethod
+   : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on polymorphic object of type '%0', which can "
+   "result in undefined behavior if the pointee is a different object")

Discookie wrote:

Seems much clearer, fixed.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-23 Thread Julian Schmidt via cfe-commits


@@ -0,0 +1,81 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreInheritedVirtualFunctions(
+  Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+   hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+  .bind("pointee"
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+   .bind("pointee"
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+   ? PointerExprWithVirtualMethod
+   : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+   "pointer arithmetic on polymorphic object of type '%0', which can "
+   "result in undefined behavior if the pointee is a different object")

5chmidti wrote:

Nit/Idea: 
- remove `, which`
- `if the pointee is a different object` -> `if the dynamic type is different` 
or `if the dynamic type differs from the pointer type`

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits

Discookie wrote:

Seems to be a CodeChecker-specific issue, doesn't block upstreaming this review.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Options
+---
+
+.. option:: MatchInheritedVirtualFunctions
+
+  When `true`, all classes with a virtual function are considered,

Discookie wrote:

Changed the description along the name.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -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

Discookie wrote:

Added, and added the doc redirect as well.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");

Discookie wrote:

Indeed, vtable-wise it works out, but the size could still be different. In 
fact, as noted in [the previous CSA checker's 
PR](https://github.com/llvm/llvm-project/pull/82977#issuecomment-1968111034), 
the size very rarely stays the same for subclasses, so this is still a good 
heuristic for bug-prone cases.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = MatchInheritedVirtualFunctions
+   ? PolymorphicPointerExpr
+   : PointerExprWithVirtualMethod;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),

Discookie wrote:

There's no builtin mul/div/rem. for pointers, and and we have no information 
about whether user-defined operators handle polymorphic objects correctly or 
not, or even what operation they do on them.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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");

Discookie wrote:

I thought about making it default for the CERT alias only, but in the end made 
it default for all aliases. Didn't really find a compelling false-positive that 
wasn't project specific.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Options
+---
+
+.. option:: MatchInheritedVirtualFunctions

Discookie wrote:

Renamed to `Ignore`.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits

https://github.com/Discookie commented:

I found a curious report when browsing through the results for `llvm-project`: 
[MemoryBuffer.cpp](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?review-status=Unreviewed=Confirmed%20bug=New=Reopened=Unresolved=llvm-project_llvmorg-12.0.0_ctr56-allow-inherited=5498248=2e0e0ee19429584af2a682c9efb1cdf4=%2Flocal%2Fpersistent_docker%2FCSA-measurements-driver-2931%2Fmeasurements_workspace%2Fllvm-project%2Fllvm%2Flib%2FSupport%2FMemoryBuffer.cpp)
I have no idea where these spurious fixits could be coming from. My check 
doesn't emit any. Could this be because of the source range I'm piping into my 
diagnostic, or is it just some side-effect of other checks, or maybe the Tidy 
module? I'll investigate and fix this before merging.

Other than this case, fixed the reviews. I'm debating adding a flag for virtual 
destructor only as well, but I haven't seen a case in the wild where it was the 
only inherited function.

Made IgnoreInheritedVirtualFunctions `false` by default - doesn't have too many 
meaningful differences, but it does have more reports. [Results on 
projects](https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=ctr56%2ainherited)

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = MatchInheritedVirtualFunctions
+   ? PolymorphicPointerExpr
+   : PointerExprWithVirtualMethod;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(
+  expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const CXXRecordDecl *PointeeType =
+  PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl();

Discookie wrote:

Oops indeed, fixed.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits


@@ -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.

Discookie wrote:

Added.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-20 Thread via cfe-commits

https://github.com/Discookie edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-18 Thread via cfe-commits

https://github.com/Discookie updated 
https://github.com/llvm/llvm-project/pull/91951

>From 69cbd3da19eb0f8eb6758782b46daf99b5b79ea4 Mon Sep 17 00:00:00 2001
From: Viktor 
Date: Mon, 6 May 2024 06:11:58 +
Subject: [PATCH 01/15] 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(
 "bugprone-unused-return-value");
 CheckFactories.registerCheck("bugprone-use-after-move");
+CheckFactories.registerCheck(
+"bugprone-virtual-arithmetic");
 CheckFactories.registerCheck(
 "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 0..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 ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("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 0..6a5f86a391747
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h
@@ -0,0 +1,30 @@
+//===--- VirtualArithmeticCheck.h - clang-tidy---*- C++ 
-*-===//
+//
+// Part 

[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-18 Thread via cfe-commits

https://github.com/Discookie updated 
https://github.com/llvm/llvm-project/pull/91951

>From 69cbd3da19eb0f8eb6758782b46daf99b5b79ea4 Mon Sep 17 00:00:00 2001
From: Viktor 
Date: Mon, 6 May 2024 06:11:58 +
Subject: [PATCH 01/15] 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(
 "bugprone-unused-return-value");
 CheckFactories.registerCheck("bugprone-use-after-move");
+CheckFactories.registerCheck(
+"bugprone-virtual-arithmetic");
 CheckFactories.registerCheck(
 "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 0..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 ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("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 0..6a5f86a391747
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h
@@ -0,0 +1,30 @@
+//===--- VirtualArithmeticCheck.h - clang-tidy---*- C++ 
-*-===//
+//
+// Part 

[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -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

PiotrZSL wrote:

add in line 173 info about new alias (check release notes from previous release)

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.

PiotrZSL wrote:

wrap this line,. move link to new line.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Options
+---
+
+.. option:: MatchInheritedVirtualFunctions
+
+  When `true`, all classes with a virtual function are considered,

PiotrZSL wrote:

Maybe: "When `true`, all polymorphic classes are checked regardless if they add 
new virtual functions or not. Default value is `false`"

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,38 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// 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);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }
+

PiotrZSL wrote:

Exclude implicit code with TK_IgnoreUnlessSpelledInSource here (look into other 
checks).

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");

PiotrZSL wrote:

This doesn't make sense, simply IF type is derived with added new methods, then 
there will be no issues, as vtable will handle everything correctly, and even 
if operation would be done on base class everything will be fine.

Actually only issue will be if size of classes does not match, but that can 
happen whatever this is polymorphic object of not.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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");

PiotrZSL wrote:

This should be default scenario, to detect all usage on polymorphic classes, 
and options should reduce scope.
I personally do not see use case for pointer arithmetic on such classes.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = MatchInheritedVirtualFunctions
+   ? PolymorphicPointerExpr
+   : PointerExprWithVirtualMethod;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(
+  expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);

PiotrZSL wrote:

that's too generic, register each of those matchers separately.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -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
+  ` check.
+
+  Finds pointer arithmetic performed on classes that declare a virtual 
function.

PiotrZSL wrote:

Change this description to point about polymorphic-object. Or instead of that 
declare (class does not declare anything), just say "that contain".

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))
+  .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+  expr(hasType(hasCanonicalType(pointerType(
+   pointee(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+   unless(isFinal()),
+   anyOf(hasMethod(isVirtualAsWritten()), isAbstract())
+  .bind("pointer");
+
+  const auto SelectedPointerExpr = MatchInheritedVirtualFunctions
+   ? PolymorphicPointerExpr
+   : PointerExprWithVirtualMethod;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+  binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+ hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+  hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(
+  expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("pointer");
+  const CXXRecordDecl *PointeeType =
+  PointerExpr->getType()->getPointeeType()->getAsCXXRecordDecl();

PiotrZSL wrote:

This may fail with typedefs, you already got cxxRecordDecl in matcher, just 
bind it to some name, like "Record"

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+  expr(hasType(hasCanonicalType(
+   pointerType(pointee(hasCanonicalType(hasDeclaration(
+   cxxRecordDecl(unless(isFinal()),
+ cxxRecordDecl(hasMethod(isVirtual()))

PiotrZSL wrote:

1. you dont need to repeat cxxRecordDecl.
2. Create custom AST matcher that utilize 
[isPolymorphic](https://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html#a6989371936079ccd01556c99637e9de0)
 ()

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, as the two types can
+have different sizes.

PiotrZSL wrote:

"as the two types can have different sizes."
this apply also to non polymorphic classes.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Options
+---
+
+.. option:: MatchInheritedVirtualFunctions

PiotrZSL wrote:

this name and description is confusing.
Maybe name it like: IgnoreInheritedVirtualFunctions
Check should be default match all polymorphic classes, and then options could 
be just to reduce scope, like ignore classes with only virtual destructor, and 
so on.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-10 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL requested changes to this pull request.

Few nits:
- storeOptions needs fix
- Consider excluding implicit code with TK_IgnoreUnlessSpelledInSource (not 
this may exclude also template instances, so I leave it up to you)
- Documentation require small work
- Make check more strict by default, and add options to relax it (as thats what 
[CTR56-CPP](https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects)
 require). I undestand that on your project this may cause some 
false-positives, but still thats your project problem. Consider adding options 
to "Allow classes where only virtual function is destructor", "Classes where 
all virtual functions are marked final",  "Classes that do not add new virtual 
functions or members". (Just check what suit your project most).

Note that next release branch out is in a ~month. Would be nice if this check 
could fit-in.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-07 Thread Julian Schmidt via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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);

5chmidti wrote:

> with the amount of final classes in my test projects, it might even be slower 
> in practice...

Hm, good point.

I ran this check with `time run-clang-tidy -clang-tidy-binary ./bin/clang-tidy 
-checks="-*,*polymorphic*" CodeGen` and couldn't measure any real difference 
when swapping around any of the matchers inside the `cxxRecordDecl` around. So 
maybe this isn't needed. I was originally about to suggest moving `isAbstract` 
before `hasMethod`, but that didn't matter either, at least in llvm. If you 
measure a noticeable difference in another project, then you can probably 
adjust the order of the matchers.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-07 Thread Julian Schmidt via cfe-commits


@@ -0,0 +1,78 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  MatchInheritedVirtualFunctions(
+  Options.get("MatchInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "MatchInheritedVirtualFunctions", true);

5chmidti wrote:

Please store the value of `MatchInheritedVirtualFunctions`, instead of `true`.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-07 Thread Julian Schmidt via cfe-commits

https://github.com/5chmidti approved this pull request.

LGTM minus the stored option value. (please wait for others to finish their 
review)

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-07 Thread Julian Schmidt via cfe-commits

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.

EugeneZelenko wrote:

Yes.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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);

Discookie wrote:

Added `isAbstract`, and fixed `unless(isFinal())` as well (although with the 
amount of `final` classes in my test projects, it might even be slower in 
practice...).

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `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.

Discookie wrote:

Default `false`, added.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.

Discookie wrote:

Even after the `Options` subheading? `bugprone-reserved-identifier` has it 
before the `Options`, would look weirder at the end of it.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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 ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("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")
+  << PointeeType->getName();

Discookie wrote:

Tidy supports source ranges like that!? I never knew, thank you! Added.
I don't think I've seen support for it in CodeChecker, that's why I was 
surprised. Will have to follow up on that.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-06-03 Thread via cfe-commits

https://github.com/Discookie updated 
https://github.com/llvm/llvm-project/pull/91951

>From 69cbd3da19eb0f8eb6758782b46daf99b5b79ea4 Mon Sep 17 00:00:00 2001
From: Viktor 
Date: Mon, 6 May 2024 06:11:58 +
Subject: [PATCH 01/14] 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(
 "bugprone-unused-return-value");
 CheckFactories.registerCheck("bugprone-use-after-move");
+CheckFactories.registerCheck(
+"bugprone-virtual-arithmetic");
 CheckFactories.registerCheck(
 "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 0..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 ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("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 0..6a5f86a391747
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/VirtualArithmeticCheck.h
@@ -0,0 +1,30 @@
+//===--- VirtualArithmeticCheck.h - clang-tidy---*- C++ 
-*-===//
+//
+// Part 

[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-30 Thread Julian Schmidt via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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);

5chmidti wrote:

Creating this matcher
```c++
AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
```
will provide a way to transitively know if a record is abstract, i.e. if any 
pure virtual function was not overridden, which is a guaranteed way to find out 
if it should be considered polymorphic. The matcher can simply be added inside 
an `anyOf` together with the `hasMethod` in both pointer expr matchers 
(`anyOf(isAbstract(), hasMethod(...))`). (I realize I didn't even mention 
`isAbstract` in the above comment in the inheritance part, which was the whole 
reason for that section)

Also, please move the `unless(isFinal())` matcher before the `hasMethod` 
matcher for slightly better performance.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-30 Thread Julian Schmidt via cfe-commits


@@ -0,0 +1,49 @@
+//===--- 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 ) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs("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")
+  << PointeeType->getName();

5chmidti wrote:

The `diag` member function only accepts a `SourceLocation` as the first 
parameter, that is correct.
When you stream a `SourceRange` into the diagnostic, that range will be 
underlined by the diagnostic engine (and in clangd). The same goes for attached 
fix-it hints, which highlight the ranges they change.

E.g.
```
  125 |   base += 1;
  |   ^
```
vs
```
  125 |   base += 1;
  |   ^~~~
```
when using 
```c++
  diag(PointerExpr->getBeginLoc(),
   "pointer arithmetic on polymorphic class '%0', which can result in "
   "undefined behavior if the pointee is a different class")
  << PointeeType->getName() 
  << PointerExpr->getSourceRange(); // adding this
```

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-29 Thread via cfe-commits

https://github.com/EugeneZelenko edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-29 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.
+
+Options
+---
+
+.. option:: MatchInheritedVirtualFunctions
+
+  When ``true``, all classes with a virtual function are considered,

EugeneZelenko wrote:

Please use single quotes for option values.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-29 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `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.

EugeneZelenko wrote:

What is default value?

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-29 Thread via cfe-commits


@@ -0,0 +1,60 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+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, 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.
+
+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,
+//  which can result in undefined behavior if the pointee is a
+//  different class
+
+delete[] static_cast(b);
+  }
+
+This check corresponds to the SEI Cert rule `CTR56-CPP: Do not use pointer 
arithmetic on polymorphic objects 
`_.

EugeneZelenko wrote:

Links to rules are usually placed at the end.

https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add `bugprone-pointer-arithmetic-on-polymorphic-object` check (PR #91951)

2024-05-29 Thread via cfe-commits

https://github.com/Discookie edited 
https://github.com/llvm/llvm-project/pull/91951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits