[clang] [clang][analyzer] Bring cplusplus.ArrayDelete out of alpha (PR #83985)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 5 01:47:26 PST 2024
https://github.com/Discookie created https://github.com/llvm/llvm-project/pull/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.
This commit cleans up the documentation and adds docs for the limitation related to tracking through references, in addition to moving it to `cplusplus`.
>From 28ed5ca4295360c8bbe671d4aaaaaed6cd2826c6 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Tue, 5 Mar 2024 09:46:26 +0000
Subject: [PATCH] [clang][analyzer] Bring cplusplus.ArrayDelete out of alpha
---
clang/docs/analyzer/checkers.rst | 68 ++++++++++++-------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 10 +--
.../Checkers/CXXDeleteChecker.cpp | 4 +-
clang/test/Analysis/ArrayDelete.cpp | 2 +-
clang/www/analyzer/alpha_checks.html | 20 ------
clang/www/analyzer/available_checks.html | 27 ++++++++
6 files changed, 79 insertions(+), 52 deletions(-)
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">
More information about the cfe-commits
mailing list