Skip to content

Commit 68ee5ec

Browse files
[Analyzer] Fix assumptions about const field with member-initializer
Essentially, having a default member initializer for a constant member does not necessarily imply the member will have the given default value. Remove part of a2e0536 ([analyzer] Treat more const variables and fields as known contants., 2018-05-04). Fix #47878 Reviewed By: r.stahl, steakhal Differential Revision: https://reviews.llvm.org/D124621
1 parent 4e5e042 commit 68ee5ec

File tree

2 files changed

+121
-7
lines changed

2 files changed

+121
-7
lines changed

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1983,15 +1983,9 @@ SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B,
19831983
if (const Optional<SVal> &V = B.getDirectBinding(R))
19841984
return *V;
19851985

1986-
// Is the field declared constant and has an in-class initializer?
1986+
// If the containing record was initialized, try to get its constant value.
19871987
const FieldDecl *FD = R->getDecl();
19881988
QualType Ty = FD->getType();
1989-
if (Ty.isConstQualified())
1990-
if (const Expr *Init = FD->getInClassInitializer())
1991-
if (Optional<SVal> V = svalBuilder.getConstantVal(Init))
1992-
return *V;
1993-
1994-
// If the containing record was initialized, try to get its constant value.
19951989
const MemRegion* superR = R->getSuperRegion();
19961990
if (const auto *VR = dyn_cast<VarRegion>(superR)) {
19971991
const VarDecl *VD = VR->getDecl();
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
2+
3+
// This tests false-positive issues related to PR48534.
4+
//
5+
// Essentially, having a default member initializer for a constant member does
6+
// not necessarily imply the member will have the given default value.
7+
8+
struct WithConstructor {
9+
int *const ptr = nullptr;
10+
WithConstructor(int *x) : ptr(x) {}
11+
12+
static auto compliant() {
13+
WithConstructor c(new int);
14+
return *(c.ptr); // no warning
15+
}
16+
17+
static auto compliantWithParam(WithConstructor c) {
18+
return *(c.ptr); // no warning
19+
}
20+
21+
static auto issue() {
22+
WithConstructor c(nullptr);
23+
return *(c.ptr); // expected-warning{{Dereference of null pointer (loaded from field 'ptr')}}
24+
}
25+
};
26+
27+
struct RegularAggregate {
28+
int *const ptr = nullptr;
29+
30+
static int compliant() {
31+
RegularAggregate c{new int};
32+
return *(c.ptr); // no warning
33+
}
34+
35+
static int issue() {
36+
RegularAggregate c;
37+
return *(c.ptr); // expected-warning{{Dereference of null pointer (loaded from field 'ptr')}}
38+
}
39+
};
40+
41+
struct WithConstructorAndArithmetic {
42+
int const i = 0;
43+
WithConstructorAndArithmetic(int x) : i(x + 1) {}
44+
45+
static int compliant(int y) {
46+
WithConstructorAndArithmetic c(0);
47+
return y / c.i; // no warning
48+
}
49+
50+
static int issue(int y) {
51+
WithConstructorAndArithmetic c(-1);
52+
return y / c.i; // expected-warning{{Division by zero}}
53+
}
54+
};
55+
56+
struct WithConstructorDeclarationOnly {
57+
int const i = 0;
58+
WithConstructorDeclarationOnly(int x); // definition not visible.
59+
60+
static int compliant1(int y) {
61+
WithConstructorDeclarationOnly c(0);
62+
return y / c.i; // no warning
63+
}
64+
65+
static int compliant2(int y) {
66+
WithConstructorDeclarationOnly c(-1);
67+
return y / c.i; // no warning
68+
}
69+
};
70+
71+
// NonAggregateFP is not an aggregate (j is a private non-static field) and has no custom constructor.
72+
// So we know i and j will always be 0 and 42, respectively.
73+
// That being said, this is not implemented because it is deemed too rare to be worth the complexity.
74+
struct NonAggregateFP {
75+
public:
76+
int const i = 0;
77+
78+
private:
79+
int const j = 42;
80+
81+
public:
82+
static int falsePositive1(NonAggregateFP c) {
83+
return 10 / c.i; // FIXME: Currently, no warning.
84+
}
85+
86+
static int falsePositive2(NonAggregateFP c) {
87+
return 10 / (c.j - 42); // FIXME: Currently, no warning.
88+
}
89+
};
90+
91+
struct NonAggregate {
92+
public:
93+
int const i = 0;
94+
95+
private:
96+
int const j = 42;
97+
98+
NonAggregate(NonAggregate const &); // not provided, could set i and j to arbitrary values.
99+
100+
public:
101+
static int compliant1(NonAggregate c) {
102+
return 10 / c.i; // no warning
103+
}
104+
105+
static int compliant2(NonAggregate c) {
106+
return 10 / (c.j - 42); // no warning
107+
}
108+
};
109+
110+
struct WithStaticMember {
111+
static int const i = 0;
112+
113+
static int issue1(WithStaticMember c) {
114+
return 10 / c.i; // expected-warning{{division by zero is undefined}} expected-warning{{Division by zero}}
115+
}
116+
117+
static int issue2() {
118+
return 10 / WithStaticMember::i; // expected-warning{{division by zero is undefined}} expected-warning{{Division by zero}}
119+
}
120+
};

0 commit comments

Comments
 (0)