Skip to content

Commit

Permalink
[clang][analyzer] Bring cplusplus.ArrayDelete out of alpha (#83985)
Browse files Browse the repository at this point in the history
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 (in my last test-run on
my usual projects, it had 0 results).

This commit cleans up the documentation and adds docs for the limitation
related to tracking through references, in addition to moving it to
`cplusplus`.

---------

Co-authored-by: Balazs Benics <benicsbalazs@gmail.com>
Co-authored-by: whisperity <whisperity@gmail.com>
  • Loading branch information
3 people committed Mar 25, 2024
1 parent 94a550d commit 37785fe
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 52 deletions.
69 changes: 45 additions & 24 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,51 @@ 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 is different from its static
type, calling `delete[]` is undefined.
This checker corresponds to the SEI 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++)
Expand Down Expand Up @@ -2139,30 +2184,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++)
Expand Down
10 changes: 5 additions & 5 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Expand Down Expand Up @@ -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">,
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/ArrayDelete.cpp
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
20 changes: 0 additions & 20 deletions clang/www/analyzer/alpha_checks.html
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
27 changes: 27 additions & 0 deletions clang/www/analyzer/available_checks.html
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down

0 comments on commit 37785fe

Please sign in to comment.