Skip to content

Commit b756c82

Browse files
committed
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 9048c2d. rdar://146753089
1 parent 0d0ef58 commit b756c82

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "clang/AST/ASTContext.h"
1314
#include "clang/AST/AttrIterator.h"
1415
#include "clang/AST/DeclCXX.h"
1516
#include "clang/AST/ParentMap.h"
@@ -23,6 +24,7 @@
2324
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
2425
#include "llvm/ADT/STLExtras.h"
2526
#include "llvm/ADT/Sequence.h"
27+
#include "llvm/Support/Casting.h"
2628
#include <optional>
2729

2830
using namespace clang;
@@ -715,7 +717,11 @@ void ExprEngine::handleConstructor(const Expr *E,
715717
// actually make things worse. Placement new makes this tricky as well,
716718
// since it's then possible to be initializing one part of a multi-
717719
// dimensional array.
718-
State = State->bindDefaultZero(Target, LCtx);
720+
const CXXRecordDecl *TargetHeldRecord =
721+
dyn_cast_or_null<CXXRecordDecl>(CE->getType()->getAsRecordDecl());
722+
723+
if (!TargetHeldRecord || !TargetHeldRecord->isEmpty())
724+
State = State->bindDefaultZero(Target, LCtx);
719725
}
720726

721727
Bldr.generateNode(CE, N, State, /*tag=*/nullptr,

clang/test/Analysis/issue-137252.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s
2+
// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus -verify %s -DEMPTY_CLASS
3+
// UNSUPPORTED: system-windows
4+
// expected-no-diagnostics
5+
6+
// This test reproduces the issue that previously the static analyzer
7+
// initialized an [[no_unique_address]] empty field to zero,
8+
// over-writing a non-empty field with the same offset.
9+
10+
namespace std {
11+
#ifdef EMPTY_CLASS
12+
13+
struct default_delete {};
14+
template <class _Tp, class _Dp = default_delete >
15+
#else
16+
// Class with methods and static members is still empty:
17+
template <typename T>
18+
class default_delete {
19+
T dump();
20+
static T x;
21+
};
22+
template <class _Tp, class _Dp = default_delete<_Tp> >
23+
#endif
24+
class unique_ptr {
25+
[[no_unique_address]] _Tp * __ptr_;
26+
[[no_unique_address]] _Dp __deleter_;
27+
28+
public:
29+
explicit unique_ptr(_Tp* __p) noexcept
30+
: __ptr_(__p),
31+
__deleter_() {}
32+
33+
~unique_ptr() {
34+
delete __ptr_;
35+
}
36+
};
37+
}
38+
39+
struct X {};
40+
41+
int main()
42+
{
43+
// Previously a leak falsely reported here. It was because the
44+
// Static Analyzer engine simulated the initialization of
45+
// `__deleter__` incorrectly. The engine assigned zero to
46+
// `__deleter__`--an empty record sharing offset with `__ptr__`.
47+
// The assignment over wrote `__ptr__`.
48+
std::unique_ptr<X> a(new X());
49+
return 0;
50+
}

0 commit comments

Comments
 (0)