[clang] b756c82 - Re-land "[analyzer] Make it a noop when initializing a field of empty record" (#138951)
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Wed May 7 15:56:11 PDT 2025
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;
+}
More information about the cfe-commits
mailing list