llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Discookie (Discookie) <details> <summary>Changes</summary> The checker finds a type of undefined behavior, where if the type of a pointer to an object-array is different from the objects' underlying type, calling delete[] is undefined, as the size of the two objects might be different. The checker has been in alpha for a while now, it is a simple checker that causes no crashes, and considering the severity of the issue, it has a low result-count on open-source projects. This commit cleans up the documentation and adds docs for the limitation related to tracking through references, in addition to moving it to `cplusplus`. --- Full diff: https://github.com/llvm/llvm-project/pull/83985.diff 6 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+44-24) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+5-5) - (modified) clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp (+2-2) - (modified) clang/test/Analysis/ArrayDelete.cpp (+1-1) - (modified) clang/www/analyzer/alpha_checks.html (-20) - (modified) clang/www/analyzer/available_checks.html (+27) ``````````diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..fc0c90f3f5d4e5 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -340,6 +340,50 @@ cplusplus C++ Checkers. +.. _cplusplus-ArrayDelete: + +cplusplus.ArrayDelete (C++) +""""""""""""""""""""""""""" +Reports destructions of arrays of polymorphic objects that are destructed as +their base class. If the dynamic type of the array's object is different from +its static type, calling `delete[]` is undefined. + +This checker corresponds to the CERT rule `EXP51-CPP: Do not delete an array through a pointer of the incorrect type <https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP51-CPP.+Do+not+delete+an+array+through+a+pointer+of+the+incorrect+type>`_. + +.. code-block:: cpp + + class Base { + public: + virtual ~Base() {} + }; + class Derived : public Base {}; + + Base *create() { + Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here + return x; + } + + void foo() { + Base *x = create(); + delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined + } + +**Limitations** + +The checker does not emit note tags when casting to and from reference types, +even though the pointer values are tracked across references. + +.. code-block:: cpp + + void foo() { + Derived *d = new Derived[10]; + Derived &dref = *d; + + Base &bref = static_cast<Base&>(dref); // no note + Base *b = &bref; + delete[] b; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined + } + .. _cplusplus-InnerPointer: cplusplus.InnerPointer (C++) @@ -2139,30 +2183,6 @@ Either the comparison is useless or there is division by zero. alpha.cplusplus ^^^^^^^^^^^^^^^ -.. _alpha-cplusplus-ArrayDelete: - -alpha.cplusplus.ArrayDelete (C++) -""""""""""""""""""""""""""""""""" -Reports destructions of arrays of polymorphic objects that are destructed as their base class. -This checker corresponds to the CERT rule `EXP51-CPP: Do not delete an array through a pointer of the incorrect type <https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP51-CPP.+Do+not+delete+an+array+through+a+pointer+of+the+incorrect+type>`_. - -.. code-block:: cpp - - class Base { - virtual ~Base() {} - }; - class Derived : public Base {} - - Base *create() { - Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here - return x; - } - - void foo() { - Base *x = create(); - delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined - } - .. _alpha-cplusplus-DeleteWithNonVirtualDtor: alpha.cplusplus.DeleteWithNonVirtualDtor (C++) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index a224b81c33a624..97fa8ac060eafa 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -622,6 +622,11 @@ def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">, let ParentPackage = Cplusplus in { +def ArrayDeleteChecker : Checker<"ArrayDelete">, + HelpText<"Reports destructions of arrays of polymorphic objects that are " + "destructed as their base class.">, + Documentation<HasDocumentation>; + def InnerPointerChecker : Checker<"InnerPointer">, HelpText<"Check for inner pointers of C++ containers used after " "re/deallocation">, @@ -777,11 +782,6 @@ def ContainerModeling : Checker<"ContainerModeling">, Documentation<NotDocumented>, Hidden; -def CXXArrayDeleteChecker : Checker<"ArrayDelete">, - HelpText<"Reports destructions of arrays of polymorphic objects that are " - "destructed as their base class.">, - Documentation<HasDocumentation>; - def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">, HelpText<"Reports destructions of polymorphic objects with a non-virtual " "destructor in their base class">, diff --git a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp index b4dee1e300e886..1b1226a7f1a71d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp @@ -220,11 +220,11 @@ CXXDeleteChecker::PtrCastVisitor::VisitNode(const ExplodedNode *N, /*addPosRange=*/true); } -void ento::registerCXXArrayDeleteChecker(CheckerManager &mgr) { +void ento::registerArrayDeleteChecker(CheckerManager &mgr) { mgr.registerChecker<CXXArrayDeleteChecker>(); } -bool ento::shouldRegisterCXXArrayDeleteChecker(const CheckerManager &mgr) { +bool ento::shouldRegisterArrayDeleteChecker(const CheckerManager &mgr) { return true; } diff --git a/clang/test/Analysis/ArrayDelete.cpp b/clang/test/Analysis/ArrayDelete.cpp index 3b8d49552376ed..6887e0a35fb8bd 100644 --- a/clang/test/Analysis/ArrayDelete.cpp +++ b/clang/test/Analysis/ArrayDelete.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s +// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s struct Base { virtual ~Base() = default; diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html index 7bbe4a20288f23..f040d1957b0f98 100644 --- a/clang/www/analyzer/alpha_checks.html +++ b/clang/www/analyzer/alpha_checks.html @@ -307,26 +307,6 @@ <h3 id="cplusplus_alpha_checkers">C++ Alpha Checkers</h3> <tbody> -<tr><td><a id="alpha.cplusplus.ArrayDelete"><div class="namedescr expandable"><span class="name"> -alpha.cplusplus.ArrayDelete</span><span class="lang"> -(C++)</span><div class="descr"> -Reports destructions of arrays of polymorphic objects that are destructed as -their base class -</div></div></a></td> -<td><div class="exampleContainer expandable"> -<div class="example"><pre> -Base *create() { - Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here - return x; -} - -void sink(Base *x) { - delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined -} - -</pre></div></div></td></tr> - - <tr><td><a id="alpha.cplusplus.DeleteWithNonVirtualDtor"><div class="namedescr expandable"><span class="name"> alpha.cplusplus.DeleteWithNonVirtualDtor</span><span class="lang"> (C++)</span><div class="descr"> diff --git a/clang/www/analyzer/available_checks.html b/clang/www/analyzer/available_checks.html index 501a2b306dfadd..c23865e57e87d3 100644 --- a/clang/www/analyzer/available_checks.html +++ b/clang/www/analyzer/available_checks.html @@ -369,6 +369,33 @@ <h3 id="cplusplus_checkers">C++ Checkers</h3> <thead><tr><td>Name, Description</td><td>Example</td></tr></thead> <tbody> + +<tr><td><a id="cplusplus.ArrayDelete"><div class="namedescr expandable"><span class="name"> +cplusplus.ArrayDelete</span><span class="lang"> +(C++)</span><div class="descr"> +Reports destructions of arrays of polymorphic objects that are destructed as +their base class. If the type of an array-pointer is different from the type of +its underlying objects, calling <code>delete[]</code> is undefined. +</div></div></a></td> +<td><div class="exampleContainer expandable"> +<div class="example"><pre> +class Base { +public: + virtual ~Base() {} +}; +class Derived : public Base {}; + +Base *create() { + Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here + return x; +} + +void sink(Base *x) { + delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined +} +</pre></div></div></td></tr> + + <tr><td><a id="cplusplus.NewDelete"><div class="namedescr expandable"><span class="name"> cplusplus.NewDelete</span><span class="lang"> (C++)</span><div class="descr"> `````````` </details> https://github.com/llvm/llvm-project/pull/83985 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
