[clang] 37785fe - [clang][analyzer] Bring cplusplus.ArrayDelete out of alpha (#83985)

via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 25 03:09:00 PDT 2024


Author: Discookie
Date: 2024-03-25T10:08:56Z
New Revision: 37785fedabd8fa752129ef5bac3462311af91c35

URL: https://github.com/llvm/llvm-project/commit/37785fedabd8fa752129ef5bac3462311af91c35
DIFF: https://github.com/llvm/llvm-project/commit/37785fedabd8fa752129ef5bac3462311af91c35.diff

LOG: [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 <benicsbalazs at gmail.com>
Co-authored-by: whisperity <whisperity at gmail.com>

Added: 
    

Modified: 
    clang/docs/analyzer/checkers.rst
    clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
    clang/test/Analysis/ArrayDelete.cpp
    clang/www/analyzer/alpha_checks.html
    clang/www/analyzer/available_checks.html

Removed: 
    


################################################################################
diff  --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index fe211514914272..66da1c7b35f28b 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -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 
diff erent 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++)
@@ -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++)

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 686e5e99f4a62c..bf46766d44b391 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 
diff erent 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">


        


More information about the cfe-commits mailing list