https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/138594
>From 4e6f2ce82744322a35614532732281eacb2fe79a Mon Sep 17 00:00:00 2001 From: Ziqing Luo <ziq...@udel.edu> Date: Mon, 5 May 2025 14:27:48 -0700 Subject: [PATCH 1/2] [StaticAnalyzer] Make it a noop when initializing a field of empty record. Previously, Static Analyzer initializes empty type fields with zeroes. This can cause problems when those fields have no unique addresses. For example, https://github.com/llvm/llvm-project/issues/137252. rdar://146753089 --- .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 8 +++- clang/test/Analysis/issue-137252.cpp | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 clang/test/Analysis/issue-137252.cpp diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 92ce3fa2225c8..219d7b4d2278c 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/ASTContext.h" #include "clang/AST/AttrIterator.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ParentMap.h" @@ -700,6 +701,7 @@ void ExprEngine::handleConstructor(const Expr *E, if (CE) { // FIXME: Is it possible and/or useful to do this before PreStmt? StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx); + ASTContext &Ctx = LCtx->getAnalysisDeclContext()->getASTContext(); for (ExplodedNode *N : DstPreVisit) { ProgramStateRef State = N->getState(); if (CE->requiresZeroInitialization()) { @@ -715,7 +717,11 @@ void ExprEngine::handleConstructor(const Expr *E, // actually make things worse. Placement new makes this tricky as well, // since it's then possible to be initializing one part of a multi- // dimensional array. - State = State->bindDefaultZero(Target, LCtx); + const CXXRecordDecl *TargetHeldRecord = + Target.getType(Ctx)->getPointeeCXXRecordDecl(); + + if (!TargetHeldRecord || !TargetHeldRecord->isEmpty()) + State = State->bindDefaultZero(Target, LCtx); } Bldr.generateNode(CE, N, State, /*tag=*/nullptr, diff --git a/clang/test/Analysis/issue-137252.cpp b/clang/test/Analysis/issue-137252.cpp new file mode 100644 index 0000000000000..8064e3f54d9fd --- /dev/null +++ b/clang/test/Analysis/issue-137252.cpp @@ -0,0 +1,45 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s -DEMPTY_CLASS + +// expected-no-diagnostics + +// This test reproduces the issue that previously the static analyzer +// initialized an [[__no_unique_address__]] empty field to zero, +// over-writing a non-empty field with the same offset. + +namespace std { +#ifdef EMPTY_CLASS + + template <typename T> + class default_delete { + T dump(); + static T x; + }; + template <class _Tp, class _Dp = default_delete<_Tp> > +#else + + struct default_delete {}; + template <class _Tp, class _Dp = default_delete > +#endif + class unique_ptr { + [[__no_unique_address__]] _Tp * __ptr_; + [[__no_unique_address__]] _Dp __deleter_; + + public: + explicit unique_ptr(_Tp* __p) noexcept + : __ptr_(__p), + __deleter_() {} + + ~unique_ptr() { + delete __ptr_; + } + }; +} + +struct X {}; + +int main() +{ + std::unique_ptr<X> a(new X()); // previously leak falsely reported + return 0; +} >From 148008d8e6058485821c784686ca0476faef6254 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <ziq...@udel.edu> Date: Tue, 6 May 2025 16:06:49 -0700 Subject: [PATCH 2/2] address comments --- .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 3 +-- clang/test/Analysis/issue-137252.cpp | 21 ++++++++++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 219d7b4d2278c..184569336b9e8 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -701,7 +701,6 @@ void ExprEngine::handleConstructor(const Expr *E, if (CE) { // FIXME: Is it possible and/or useful to do this before PreStmt? StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx); - ASTContext &Ctx = LCtx->getAnalysisDeclContext()->getASTContext(); for (ExplodedNode *N : DstPreVisit) { ProgramStateRef State = N->getState(); if (CE->requiresZeroInitialization()) { @@ -718,7 +717,7 @@ void ExprEngine::handleConstructor(const Expr *E, // since it's then possible to be initializing one part of a multi- // dimensional array. const CXXRecordDecl *TargetHeldRecord = - Target.getType(Ctx)->getPointeeCXXRecordDecl(); + dyn_cast_or_null<CXXRecordDecl>(CE->getType()->getAsRecordDecl()); if (!TargetHeldRecord || !TargetHeldRecord->isEmpty()) State = State->bindDefaultZero(Target, LCtx); diff --git a/clang/test/Analysis/issue-137252.cpp b/clang/test/Analysis/issue-137252.cpp index 8064e3f54d9fd..6cc0af21f1f85 100644 --- a/clang/test/Analysis/issue-137252.cpp +++ b/clang/test/Analysis/issue-137252.cpp @@ -10,20 +10,20 @@ namespace std { #ifdef EMPTY_CLASS + struct default_delete {}; + template <class _Tp, class _Dp = default_delete > +#else + // Class with methods and static members is still empty: template <typename T> class default_delete { T dump(); static T x; }; template <class _Tp, class _Dp = default_delete<_Tp> > -#else - - struct default_delete {}; - template <class _Tp, class _Dp = default_delete > #endif class unique_ptr { - [[__no_unique_address__]] _Tp * __ptr_; - [[__no_unique_address__]] _Dp __deleter_; + [[no_unique_address]] _Tp * __ptr_; + [[no_unique_address]] _Dp __deleter_; public: explicit unique_ptr(_Tp* __p) noexcept @@ -40,6 +40,11 @@ struct X {}; int main() { - std::unique_ptr<X> a(new X()); // previously leak falsely reported - return 0; + // Previously a leak falsely reported here. It was because the + // Static Analyzer engine simulated the initialization of + // `__deleter__` incorrectly. The engine assigned zero to + // `__deleter__`--an empty record sharing offset with `__ptr__`. + // The assignment over wrote `__ptr__`. + std::unique_ptr<X> a(new X()); + return 0; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
