[clang] 9b8297b - [analyzer] Do not destruct fields of unions (#122330)

[via cfe-commits]

Author: Jameson Nash
Date: 2025-02-07T08:01:39+01:00
New Revision: 9b8297bc7ea8f217a3f701afedd2c953a4ad7867

URL: https://github.com/llvm/llvm-project/commit/9b8297bc7ea8f217a3f701afedd2c953a4ad7867
DIFF: https://github.com/llvm/llvm-project/commit/9b8297bc7ea8f217a3f701afedd2c953a4ad7867.diff

LOG: [analyzer] Do not destruct fields of unions (#122330)

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

Added: 
    clang/test/Analysis/dtor-union.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Analysis/CFG.cpp
    clang/test/Analysis/NewDelete-checker-test.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6d7eb84927a978..92f63c15030898 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -127,6 +127,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
 ----------------------------------

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61..3e144395cffc6f 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 21b4cf817b5df6..06754f669b1e61 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-union.cpp b/clang/test/Analysis/dtor-union.cpp
new file mode 100644
index 00000000000000..dac366e6f9df89
--- /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}}
+}