NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
This is an Objective-C++ specific fix to a problem that's similar to D59573
<https://reviews.llvm.org/D59573> and D59622 <https://reviews.llvm.org/D59622>
(i.e., hitting the same assertion i added in D59054
<https://reviews.llvm.org/D59054>).
This time, surprisingly, the assertion is in fact incorrect: there is a
cornercase in Objective-C++ in which a C++ object is not constructed with a
constructor, but only zero-initialized. Namely, this happens when an
Objective-C message is sent to a `nil` and it is supposed to return a C++
object.
I made sure that the assertion is only relaxed for Objective-C++ but it's hard
to relax it in a more specific way with the amount of information that
`RegionStore` receives. The alternative solution was to conjure a
`LazyCompoundValue` specifically for this case instead, but implementing this
cornercase in `SValBuilder::makeZeroVal()` is hard because there's no good
region to use as the base of that `LazyCompoundVal`, and in the checker (this
modeling currently belongs to CallAndMessageChecker) it's even uglier and is
bad for checker APIs.
Repository:
rC Clang
https://reviews.llvm.org/D60988
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/nil-receiver.mm
Index: clang/test/Analysis/nil-receiver.mm
===================================================================
--- /dev/null
+++ clang/test/Analysis/nil-receiver.mm
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN: -verify %s
+
+#define nil ((id)0)
+
+void clang_analyzer_eval(int);
+
+struct S {
+ int x;
+ S();
+};
+
+@interface I
+@property S s;
+@end
+
+void foo() {
+ // This produces a zero-initialized structure.
+ // FIXME: This very fact does deserve the warning, because zero-initialized
+ // structures aren't always valid in C++. It's particularly bad when the
+ // object has a vtable.
+ S s = ((I *)nil).s;
+ clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2361,7 +2361,14 @@
// In C++17 aggregates may have base classes, handle those as well.
// They appear before fields in the initializer list / compound value.
if (const auto *CRD = dyn_cast<CXXRecordDecl>(RD)) {
- assert(CRD->isAggregate() &&
+ // If the object was constructed with a constructor, its value is a
+ // LazyCompoundVal. If it's a raw CompoundVal, it means that we're
+ // performing aggregate initialization. The only exception from this
+ // rule is sending an Objective-C++ message that returns a C++ object
+ // to a nil receiver; in this case the semantics is to return a
+ // zero-initialized object even if it's a C++ object that doesn't have
+ // this sort of constructor; the CompoundVal is empty in this case.
+ assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) &&
"Non-aggregates are constructed with a constructor!");
for (const auto &B : CRD->bases()) {
Index: clang/test/Analysis/nil-receiver.mm
===================================================================
--- /dev/null
+++ clang/test/Analysis/nil-receiver.mm
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN: -verify %s
+
+#define nil ((id)0)
+
+void clang_analyzer_eval(int);
+
+struct S {
+ int x;
+ S();
+};
+
+@interface I
+@property S s;
+@end
+
+void foo() {
+ // This produces a zero-initialized structure.
+ // FIXME: This very fact does deserve the warning, because zero-initialized
+ // structures aren't always valid in C++. It's particularly bad when the
+ // object has a vtable.
+ S s = ((I *)nil).s;
+ clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2361,7 +2361,14 @@
// In C++17 aggregates may have base classes, handle those as well.
// They appear before fields in the initializer list / compound value.
if (const auto *CRD = dyn_cast<CXXRecordDecl>(RD)) {
- assert(CRD->isAggregate() &&
+ // If the object was constructed with a constructor, its value is a
+ // LazyCompoundVal. If it's a raw CompoundVal, it means that we're
+ // performing aggregate initialization. The only exception from this
+ // rule is sending an Objective-C++ message that returns a C++ object
+ // to a nil receiver; in this case the semantics is to return a
+ // zero-initialized object even if it's a C++ object that doesn't have
+ // this sort of constructor; the CompoundVal is empty in this case.
+ assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) &&
"Non-aggregates are constructed with a constructor!");
for (const auto &B : CRD->bases()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
