Skip to content

Commit 37785fe

Browse files
Discookiesteakhalwhisperity
authored
[clang][analyzer] Bring cplusplus.ArrayDelete out of alpha (#83985)
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 <[email protected]> Co-authored-by: whisperity <[email protected]>
1 parent 94a550d commit 37785fe

File tree

6 files changed

+80
-52
lines changed

6 files changed

+80
-52
lines changed

clang/docs/analyzer/checkers.rst

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,51 @@ cplusplus
340340
341341
C++ Checkers.
342342
343+
.. _cplusplus-ArrayDelete:
344+
345+
cplusplus.ArrayDelete (C++)
346+
"""""""""""""""""""""""""""
347+
348+
Reports destructions of arrays of polymorphic objects that are destructed as
349+
their base class. If the dynamic type of the array is different from its static
350+
type, calling `delete[]` is undefined.
351+
352+
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>`_.
353+
354+
.. code-block:: cpp
355+
356+
class Base {
357+
public:
358+
virtual ~Base() {}
359+
};
360+
class Derived : public Base {};
361+
362+
Base *create() {
363+
Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
364+
return x;
365+
}
366+
367+
void foo() {
368+
Base *x = create();
369+
delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined
370+
}
371+
372+
**Limitations**
373+
374+
The checker does not emit note tags when casting to and from reference types,
375+
even though the pointer values are tracked across references.
376+
377+
.. code-block:: cpp
378+
379+
void foo() {
380+
Derived *d = new Derived[10];
381+
Derived &dref = *d;
382+
383+
Base &bref = static_cast<Base&>(dref); // no note
384+
Base *b = &bref;
385+
delete[] b; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined
386+
}
387+
343388
.. _cplusplus-InnerPointer:
344389
345390
cplusplus.InnerPointer (C++)
@@ -2139,30 +2184,6 @@ Either the comparison is useless or there is division by zero.
21392184
alpha.cplusplus
21402185
^^^^^^^^^^^^^^^
21412186
2142-
.. _alpha-cplusplus-ArrayDelete:
2143-
2144-
alpha.cplusplus.ArrayDelete (C++)
2145-
"""""""""""""""""""""""""""""""""
2146-
Reports destructions of arrays of polymorphic objects that are destructed as their base class.
2147-
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>`_.
2148-
2149-
.. code-block:: cpp
2150-
2151-
class Base {
2152-
virtual ~Base() {}
2153-
};
2154-
class Derived : public Base {}
2155-
2156-
Base *create() {
2157-
Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
2158-
return x;
2159-
}
2160-
2161-
void foo() {
2162-
Base *x = create();
2163-
delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' is undefined
2164-
}
2165-
21662187
.. _alpha-cplusplus-DeleteWithNonVirtualDtor:
21672188
21682189
alpha.cplusplus.DeleteWithNonVirtualDtor (C++)

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,11 @@ def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
622622

623623
let ParentPackage = Cplusplus in {
624624

625+
def ArrayDeleteChecker : Checker<"ArrayDelete">,
626+
HelpText<"Reports destructions of arrays of polymorphic objects that are "
627+
"destructed as their base class.">,
628+
Documentation<HasDocumentation>;
629+
625630
def InnerPointerChecker : Checker<"InnerPointer">,
626631
HelpText<"Check for inner pointers of C++ containers used after "
627632
"re/deallocation">,
@@ -777,11 +782,6 @@ def ContainerModeling : Checker<"ContainerModeling">,
777782
Documentation<NotDocumented>,
778783
Hidden;
779784

780-
def CXXArrayDeleteChecker : Checker<"ArrayDelete">,
781-
HelpText<"Reports destructions of arrays of polymorphic objects that are "
782-
"destructed as their base class.">,
783-
Documentation<HasDocumentation>;
784-
785785
def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
786786
HelpText<"Reports destructions of polymorphic objects with a non-virtual "
787787
"destructor in their base class">,

clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,11 @@ CXXDeleteChecker::PtrCastVisitor::VisitNode(const ExplodedNode *N,
220220
/*addPosRange=*/true);
221221
}
222222

223-
void ento::registerCXXArrayDeleteChecker(CheckerManager &mgr) {
223+
void ento::registerArrayDeleteChecker(CheckerManager &mgr) {
224224
mgr.registerChecker<CXXArrayDeleteChecker>();
225225
}
226226

227-
bool ento::shouldRegisterCXXArrayDeleteChecker(const CheckerManager &mgr) {
227+
bool ento::shouldRegisterArrayDeleteChecker(const CheckerManager &mgr) {
228228
return true;
229229
}
230230

clang/test/Analysis/ArrayDelete.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
1+
// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
22

33
struct Base {
44
virtual ~Base() = default;

clang/www/analyzer/alpha_checks.html

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -307,26 +307,6 @@ <h3 id="cplusplus_alpha_checkers">C++ Alpha Checkers</h3>
307307
<tbody>
308308

309309

310-
<tr><td><a id="alpha.cplusplus.ArrayDelete"><div class="namedescr expandable"><span class="name">
311-
alpha.cplusplus.ArrayDelete</span><span class="lang">
312-
(C++)</span><div class="descr">
313-
Reports destructions of arrays of polymorphic objects that are destructed as
314-
their base class
315-
</div></div></a></td>
316-
<td><div class="exampleContainer expandable">
317-
<div class="example"><pre>
318-
Base *create() {
319-
Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
320-
return x;
321-
}
322-
323-
void sink(Base *x) {
324-
delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined
325-
}
326-
327-
</pre></div></div></td></tr>
328-
329-
330310
<tr><td><a id="alpha.cplusplus.DeleteWithNonVirtualDtor"><div class="namedescr expandable"><span class="name">
331311
alpha.cplusplus.DeleteWithNonVirtualDtor</span><span class="lang">
332312
(C++)</span><div class="descr">

clang/www/analyzer/available_checks.html

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,33 @@ <h3 id="cplusplus_checkers">C++ Checkers</h3>
369369
<thead><tr><td>Name, Description</td><td>Example</td></tr></thead>
370370

371371
<tbody>
372+
373+
<tr><td><a id="cplusplus.ArrayDelete"><div class="namedescr expandable"><span class="name">
374+
cplusplus.ArrayDelete</span><span class="lang">
375+
(C++)</span><div class="descr">
376+
Reports destructions of arrays of polymorphic objects that are destructed as
377+
their base class. If the type of an array-pointer is different from the type of
378+
its underlying objects, calling <code>delete[]</code> is undefined.
379+
</div></div></a></td>
380+
<td><div class="exampleContainer expandable">
381+
<div class="example"><pre>
382+
class Base {
383+
public:
384+
virtual ~Base() {}
385+
};
386+
class Derived : public Base {};
387+
388+
Base *create() {
389+
Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
390+
return x;
391+
}
392+
393+
void sink(Base *x) {
394+
delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined
395+
}
396+
</pre></div></div></td></tr>
397+
398+
372399
<tr><td><a id="cplusplus.NewDelete"><div class="namedescr expandable"><span class="name">
373400
cplusplus.NewDelete</span><span class="lang">
374401
(C++)</span><div class="descr">

0 commit comments

Comments
 (0)