Skip to content

Commit ed25eb3

Browse files
steakhalyuxuanchen1997
authored andcommitted
[analyzer] Don't invalidate the super region when a std object ctor runs (#100405)
Summary: CPP-5269 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250642
1 parent 23bae0d commit ed25eb3

File tree

2 files changed

+134
-0
lines changed

2 files changed

+134
-0
lines changed

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,12 +923,31 @@ SVal AnyCXXConstructorCall::getCXXThisVal() const {
923923
return UnknownVal();
924924
}
925925

926+
static bool isWithinStdNamespace(const Decl *D) {
927+
const DeclContext *DC = D->getDeclContext();
928+
while (DC) {
929+
if (const auto *NS = dyn_cast<NamespaceDecl>(DC);
930+
NS && NS->isStdNamespace())
931+
return true;
932+
DC = DC->getParent();
933+
}
934+
return false;
935+
}
936+
926937
void AnyCXXConstructorCall::getExtraInvalidatedValues(ValueList &Values,
927938
RegionAndSymbolInvalidationTraits *ETraits) const {
928939
SVal V = getCXXThisVal();
929940
if (SymbolRef Sym = V.getAsSymbol(true))
930941
ETraits->setTrait(Sym,
931942
RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
943+
944+
// Standard classes don't reinterpret-cast and modify super regions.
945+
const bool IsStdClassCtor = isWithinStdNamespace(getDecl());
946+
if (const MemRegion *Obj = V.getAsRegion(); Obj && IsStdClassCtor) {
947+
ETraits->setTrait(
948+
Obj, RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
949+
}
950+
932951
Values.push_back(V);
933952
}
934953

clang/test/Analysis/call-invalidation.cpp

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
22

3+
template <class T> void clang_analyzer_dump(T);
34
void clang_analyzer_eval(bool);
45

56
void usePointer(int * const *);
@@ -165,3 +166,117 @@ void testMixedConstNonConstCalls() {
165166
useFirstNonConstSecondConst(&(s2.x), &(s2.y));
166167
clang_analyzer_eval(s2.y == 1); // expected-warning{{UNKNOWN}}
167168
}
169+
170+
namespace std {
171+
class Opaque {
172+
public:
173+
Opaque();
174+
int nested_member;
175+
};
176+
} // namespace std
177+
178+
struct StdWrappingOpaque {
179+
std::Opaque o; // first member
180+
int uninit;
181+
};
182+
struct StdWrappingOpaqueSwapped {
183+
int uninit; // first member
184+
std::Opaque o;
185+
};
186+
187+
int testStdCtorDoesNotInvalidateParentObject() {
188+
StdWrappingOpaque obj;
189+
int x = obj.o.nested_member; // no-garbage: std::Opaque::ctor might initialized this
190+
int y = obj.uninit; // FIXME: We should have a garbage read here. Read the details.
191+
// As the first member ("obj.o") is invalidated, a conjured default binding is bound
192+
// to the offset 0 within cluster "obj", and this masks every uninitialized fields
193+
// that follows. We need a better store with extents to fix this.
194+
return x + y;
195+
}
196+
197+
int testStdCtorDoesNotInvalidateParentObjectSwapped() {
198+
StdWrappingOpaqueSwapped obj;
199+
int x = obj.o.nested_member; // no-garbage: std::Opaque::ctor might initialized this
200+
int y = obj.uninit; // expected-warning {{Assigned value is garbage or undefined}}
201+
return x + y;
202+
}
203+
204+
class UserProvidedOpaque {
205+
public:
206+
UserProvidedOpaque(); // might reinterpret_cast(this)
207+
int nested_member;
208+
};
209+
210+
struct WrappingUserProvidedOpaque {
211+
UserProvidedOpaque o; // first member
212+
int uninit;
213+
};
214+
struct WrappingUserProvidedOpaqueSwapped {
215+
int uninit; // first member
216+
UserProvidedOpaque o;
217+
};
218+
219+
int testUserProvidedCtorInvalidatesParentObject() {
220+
WrappingUserProvidedOpaque obj;
221+
int x = obj.o.nested_member; // no-garbage: UserProvidedOpaque::ctor might initialized this
222+
int y = obj.uninit; // no-garbage: UserProvidedOpaque::ctor might reinterpret_cast(this) and write to the "uninit" member.
223+
return x + y;
224+
}
225+
226+
int testUserProvidedCtorInvalidatesParentObjectSwapped() {
227+
WrappingUserProvidedOpaqueSwapped obj;
228+
int x = obj.o.nested_member; // no-garbage: same as above
229+
int y = obj.uninit; // no-garbage: same as above
230+
return x + y;
231+
}
232+
233+
struct WrappingStdWrappingOpaqueOuterInits {
234+
int first = 1;
235+
std::Opaque second;
236+
int third = 3;
237+
WrappingStdWrappingOpaqueOuterInits() {
238+
clang_analyzer_dump(first); // expected-warning {{1 S32b}}
239+
clang_analyzer_dump(second.nested_member); // expected-warning {{derived_}}
240+
clang_analyzer_dump(third); // expected-warning {{3 S32b}}
241+
}
242+
};
243+
244+
struct WrappingUserProvidedOpaqueOuterInits {
245+
int first = 1; // Potentially overwritten by UserProvidedOpaque::ctor
246+
UserProvidedOpaque second; // Invalidates the object so far.
247+
int third = 3; // Happens after UserProvidedOpaque::ctor, thus preserved!
248+
WrappingUserProvidedOpaqueOuterInits() {
249+
clang_analyzer_dump(first); // expected-warning {{derived_}}
250+
clang_analyzer_dump(second.nested_member); // expected-warning {{derived_}}
251+
clang_analyzer_dump(third); // expected-warning {{3 S32b}}
252+
}
253+
};
254+
255+
extern "C++" {
256+
namespace std {
257+
inline namespace v1 {
258+
namespace custom_ranges {
259+
struct Fancy {
260+
struct iterator {
261+
struct Opaque {
262+
Opaque();
263+
int nested_member;
264+
}; // struct Opaque
265+
}; // struct iterator
266+
}; // struct Fancy
267+
} // namespace custom_ranges
268+
} // namespace v1
269+
} // namespace std
270+
} // extern "C++"
271+
272+
struct StdWrappingFancyOpaque {
273+
int uninit;
274+
std::custom_ranges::Fancy::iterator::Opaque o;
275+
};
276+
277+
int testNestedStdNamespacesAndRecords() {
278+
StdWrappingFancyOpaque obj;
279+
int x = obj.o.nested_member; // no-garbage: ctor
280+
int y = obj.uninit; // expected-warning {{Assigned value is garbage or undefined}}
281+
return x + y;
282+
}

0 commit comments

Comments
 (0)