Author: Ziqing Luo Date: 2025-05-07T15:55:52-07:00 New Revision: b756c82bfacb2822cd516c32ae3c406e71448c0a
URL: https://github.com/llvm/llvm-project/commit/b756c82bfacb2822cd516c32ae3c406e71448c0a DIFF: https://github.com/llvm/llvm-project/commit/b756c82bfacb2822cd516c32ae3c406e71448c0a.diff LOG: Re-land "[analyzer] Make it a noop when initializing a field of empty record" (#138951) The original commit assumes that `CXXConstructExpr->getType()->getAsRecordDecl()` is always a `CXXRecordDecl` but it is not true for ObjC programs. This relanding changes `cast<CXXRecordDecl>(CXXConstructExpr->getType()->getAsRecordDecl())` to `dyn_cast_or_null<CXXRecordDecl>(CXXConstructExpr->getType()->getAsRecordDecl())` This reverts commit 9048c2d4f239cb47fed17cb150e2bbf3934454c2. rdar://146753089 Added: clang/test/Analysis/issue-137252.cpp Modified: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 92ce3fa2225c8..e07e24faa3490 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" @@ -23,6 +24,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/Sequence.h" +#include "llvm/Support/Casting.h" #include <optional> using namespace clang; @@ -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 = + dyn_cast_or_null<CXXRecordDecl>(CE->getType()->getAsRecordDecl()); + + 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..6ca3e20ccbbca --- /dev/null +++ b/clang/test/Analysis/issue-137252.cpp @@ -0,0 +1,50 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s -DEMPTY_CLASS +// UNSUPPORTED: system-windows +// 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 + + 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> > +#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() +{ + // 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