Skip to content

Commit 34531eb

Browse files
ziqingluo-90petrhosek
authored andcommitted
[analyzer] Make it a noop when initializing a field of empty record (llvm#138594)
Previously, Static Analyzer initializes empty type fields with zeroes. This can cause problems when those fields have no unique addresses. For example, llvm#137252. rdar://146753089
1 parent 3d5cf6d commit 34531eb

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

Lines changed: 6 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"
@@ -715,7 +716,11 @@ void ExprEngine::handleConstructor(const Expr *E,
715716
// actually make things worse. Placement new makes this tricky as well,
716717
// since it's then possible to be initializing one part of a multi-
717718
// dimensional array.
718-
State = State->bindDefaultZero(Target, LCtx);
719+
const CXXRecordDecl *TargetHeldRecord =
720+
cast<CXXRecordDecl>(CE->getType()->getAsRecordDecl());
721+
722+
if (!TargetHeldRecord || !TargetHeldRecord->isEmpty())
723+
State = State->bindDefaultZero(Target, LCtx);
719724
}
720725

721726
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)