Skip to content

Commit

Permalink
[analyzer] Do not destruct fields of unions (llvm#122330)
Browse files Browse the repository at this point in the history
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 llvm#119415
  • Loading branch information
vtjnash authored Feb 7, 2025
1 parent 083686d commit 9b8297b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 0 deletions.
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------------------------------
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Analysis/CFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
28 changes: 28 additions & 0 deletions clang/test/Analysis/NewDelete-checker-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
}
38 changes: 38 additions & 0 deletions clang/test/Analysis/dtor-union.cpp
Original file line number Diff line number Diff line change
@@ -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}}
}

0 comments on commit 9b8297b

Please sign in to comment.