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 <vtjn...@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 <vtjn...@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 <vtjn...@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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits