[clang] [analyzer] Do not destruct fields of unions (PR #122330)
Jameson Nash via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 4 14:21:50 PST 2025
https://github.com/vtjnash updated https://github.com/llvm/llvm-project/pull/122330
>From 0293a835a395c2e5a54efd16b70a38e73f116c59 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash at gmail.com>
Date: Thu, 9 Jan 2025 17:10:08 +0000
Subject: [PATCH 1/3] [Sema] do not destruct fields of unions
The C++ standard prohibits this implicit destructor call, leading to
incorrect reports from clang-analyzer. This causes projects that use
std::option (including llvm) to fail the cplusplus.NewDelete test
incorrectly when run through the analyzer.
Fixes #119415
---
clang/lib/Analysis/CFG.cpp | 2 ++
.../test/Analysis/NewDelete-checker-test.cpp | 28 +++++++++++++++++++
clang/test/Analysis/dtor-array.cpp | 24 ++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61d..3e144395cffc6ff 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2041,6 +2041,8 @@ void CFGBuilder::addImplicitDtorsForDestructor(const CXXDestructorDecl *DD) {
}
// First destroy member objects.
+ if (RD->isUnion())
+ return;
for (auto *FI : RD->fields()) {
// Check for constant size array. Set type to array element type.
QualType QT = FI->getType();
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 21b4cf817b5df6b..806edd47840fc11 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -441,3 +441,31 @@ void testLeakBecauseNTTPIsNotDeallocation() {
void* p = ::operator new(10);
deallocate_via_nttp<not_free>(p);
} // leak-warning{{Potential leak of memory pointed to by 'p'}}
+
+namespace optional_union {
+ template <typename T>
+ class unique_ptr {
+ T *q;
+ public:
+ unique_ptr() : q(new T) {}
+ ~unique_ptr() {
+ delete q;
+ }
+ };
+
+ union custom_union_t {
+ unique_ptr<int> present;
+ char notpresent;
+ custom_union_t() : present(unique_ptr<int>()) {}
+ ~custom_union_t() {};
+ };
+
+ void testUnionCorrect() {
+ custom_union_t a;
+ a.present.~unique_ptr<int>();
+ }
+
+ void testUnionLeak() {
+ custom_union_t a;
+ } // leak-warning{{Potential leak of memory pointed to by 'a.present.q'}}
+}
diff --git a/clang/test/Analysis/dtor-array.cpp b/clang/test/Analysis/dtor-array.cpp
index 84a34af92251692..1bbe55c09ee7e27 100644
--- a/clang/test/Analysis/dtor-array.cpp
+++ b/clang/test/Analysis/dtor-array.cpp
@@ -377,3 +377,27 @@ void directUnknownSymbol() {
}
}
+
+void testUnionDtor() {
+ static int unionDtorCalled;
+ InlineDtor::cnt = 0;
+ InlineDtor::dtorCalled = 0;
+ unionDtorCalled = 0;
+ {
+ union UnionDtor {
+ InlineDtor kind1;
+ char kind2;
+ ~UnionDtor() { unionDtorCalled++; }
+ };
+ UnionDtor u1{.kind1{}};
+ UnionDtor u2{.kind2{}};
+ auto u3 = new UnionDtor{.kind1{}};
+ auto u4 = new UnionDtor{.kind2{}};
+ delete u3;
+ delete u4;
+ }
+
+ clang_analyzer_eval(unionDtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(InlineDtor::dtorCalled != 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}}
+}
>From ec5ae1bd8976ea52f34500aabf73d6e47c4430b5 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash at gmail.com>
Date: Wed, 29 Jan 2025 19:24:39 +0000
Subject: [PATCH 2/3] fixup! [Sema] do not destruct fields of unions
---
clang/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 18ecf1efa13a164..8973c49c051a3dd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -121,6 +121,7 @@ Improvements to Clang's diagnostics
- The ``-Wunique-object-duplication`` warning has been added to warn about objects
which are supposed to only exist once per program, but may get duplicated when
built into a shared library.
+- Fixed a bug where Clang's Analysis did not correctly model the destructor behavior of ``union`` members (#GH119415).
Improvements to Clang's time-trace
----------------------------------
>From 5c581bb54baa154a00a347b2df66d2ffa0990e18 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash at gmail.com>
Date: Tue, 4 Feb 2025 21:00:21 +0000
Subject: [PATCH 3/3] fixup! [Sema] do not destruct fields of unions
---
.../test/Analysis/NewDelete-checker-test.cpp | 2 +-
clang/test/Analysis/dtor-array.cpp | 24 ------------
clang/test/Analysis/dtor-union.cpp | 38 +++++++++++++++++++
3 files changed, 39 insertions(+), 25 deletions(-)
create mode 100644 clang/test/Analysis/dtor-union.cpp
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 806edd47840fc11..06754f669b1e61b 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -457,7 +457,7 @@ namespace optional_union {
unique_ptr<int> present;
char notpresent;
custom_union_t() : present(unique_ptr<int>()) {}
- ~custom_union_t() {};
+ ~custom_union_t() {}
};
void testUnionCorrect() {
diff --git a/clang/test/Analysis/dtor-array.cpp b/clang/test/Analysis/dtor-array.cpp
index 1bbe55c09ee7e27..84a34af92251692 100644
--- a/clang/test/Analysis/dtor-array.cpp
+++ b/clang/test/Analysis/dtor-array.cpp
@@ -377,27 +377,3 @@ void directUnknownSymbol() {
}
}
-
-void testUnionDtor() {
- static int unionDtorCalled;
- InlineDtor::cnt = 0;
- InlineDtor::dtorCalled = 0;
- unionDtorCalled = 0;
- {
- union UnionDtor {
- InlineDtor kind1;
- char kind2;
- ~UnionDtor() { unionDtorCalled++; }
- };
- UnionDtor u1{.kind1{}};
- UnionDtor u2{.kind2{}};
- auto u3 = new UnionDtor{.kind1{}};
- auto u4 = new UnionDtor{.kind2{}};
- delete u3;
- delete u4;
- }
-
- clang_analyzer_eval(unionDtorCalled == 4); // expected-warning {{TRUE}}
- clang_analyzer_eval(InlineDtor::dtorCalled != 4); // expected-warning {{TRUE}}
- clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}}
-}
diff --git a/clang/test/Analysis/dtor-union.cpp b/clang/test/Analysis/dtor-union.cpp
new file mode 100644
index 000000000000000..dac366e6f9df899
--- /dev/null
+++ b/clang/test/Analysis/dtor-union.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++17 %s
+
+void clang_analyzer_eval(bool);
+
+struct InlineDtor {
+ static int cnt;
+ static int dtorCalled;
+ ~InlineDtor() {
+ ++dtorCalled;
+ }
+};
+
+int InlineDtor::cnt = 0;
+int InlineDtor::dtorCalled = 0;
+
+void testUnionDtor() {
+ static int unionDtorCalled;
+ InlineDtor::cnt = 0;
+ InlineDtor::dtorCalled = 0;
+ unionDtorCalled = 0;
+ {
+ union UnionDtor {
+ InlineDtor kind1;
+ char kind2;
+ ~UnionDtor() { unionDtorCalled++; }
+ };
+ UnionDtor u1{.kind1{}};
+ UnionDtor u2{.kind2{}};
+ auto u3 = new UnionDtor{.kind1{}};
+ auto u4 = new UnionDtor{.kind2{}};
+ delete u3;
+ delete u4;
+ }
+
+ clang_analyzer_eval(unionDtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}}
+}
More information about the cfe-commits
mailing list