NoQ updated this revision to Diff 189791.
NoQ added a comment.
Added this as a comment.
If the patch is not obvious at a glance, feel free to demand comments! Every
second you spend trying to figure out how the code works is multiplied by the
number of people who will have to do the same in the future.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59054/new/
https://reviews.llvm.org/D59054
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/array-struct-region.cpp
Index: clang/test/Analysis/array-struct-region.cpp
===================================================================
--- clang/test/Analysis/array-struct-region.cpp
+++ clang/test/Analysis/array-struct-region.cpp
@@ -1,7 +1,21 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -x c++ -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -DINLINE -x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -DINLINE -x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN: -analyzer-checker=debug.ExprInspection -verify\
+// RUN: -DINLINE -x c++ -std=c++17 %s
void clang_analyzer_eval(int);
@@ -196,4 +210,49 @@
}
}
+#if __cplusplus >= 201703L
+namespace aggregate_inheritance_cxx17 {
+struct A {
+ int x;
+};
+
+struct B {
+ int y;
+};
+
+struct C: B {
+ int z;
+};
+
+struct D: A, C {
+ int w;
+};
+
+void foo() {
+ D d{1, 2, 3, 4};
+ clang_analyzer_eval(d.x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(d.y == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(d.z == 3); // expected-warning{{TRUE}}
+ clang_analyzer_eval(d.w == 4); // expected-warning{{TRUE}}
+}
+} // namespace aggregate_inheritance_cxx17
+#endif
+
+namespace flex_array_inheritance_cxx17 {
+struct A {
+ int flexible_array[];
+};
+
+struct B {
+ long cookie;
+};
+
+struct C : B {
+ A a;
+};
+
+void foo() {
+ C c{}; // no-crash
+}
+} // namespace flex_array_inheritance_cxx17
#endif
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2334,12 +2334,57 @@
if (V.isUnknown() || !V.getAs<nonloc::CompoundVal>())
return bindAggregate(B, R, UnknownVal());
+ // The raw CompoundVal is essentially a symbolic InitListExpr: an (immutable)
+ // list of other values. It appears pretty much only when there's an actual
+ // initializer list expression in the program, and the analyzer tries to
+ // unwrap it as soon as possible.
+ // This code is where such unwrap happens: when the compound value is put into
+ // the object that it was supposed to initialize (it's an *initializer* list,
+ // after all), instead of binding the whole value to the whole object, we bind
+ // sub-values to sub-objects. Sub-values may themselves be compound values,
+ // and in this case the procedure becomes recursive.
+ // FIXME: The annoying part about compound values is that they don't carry
+ // any sort of information about which value corresponds to which sub-object.
+ // It's simply a list of values in the middle of nowhere; we expect to match
+ // them to sub-objects, essentially, "by index": first value binds to
+ // the first field, second value binds to the second field, etc.
+ // It would have been much safer to organize non-lazy compound values as
+ // a mapping from fields/bases to values.
const nonloc::CompoundVal& CV = V.castAs<nonloc::CompoundVal>();
nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
- RecordDecl::field_iterator FI, FE;
RegionBindingsRef NewB(B);
+ // 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() &&
+ "Non-aggregates are constructed with a constructor!");
+
+ for (const auto &B : CRD->bases()) {
+ // (Multiple inheritance is fine though.)
+ assert(!B.isVirtual() && "Aggregates cannot have virtual base classes!");
+
+ if (VI == VE)
+ break;
+
+ QualType BTy = B.getType();
+ assert(BTy->isStructureOrClassType() && "Base classes must be classes!");
+
+ const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
+ assert(BRD && "Base classes must be C++ classes!");
+
+ const CXXBaseObjectRegion *BR =
+ MRMgr.getCXXBaseObjectRegion(BRD, R, /*IsVirtual=*/false);
+
+ NewB = bindStruct(NewB, BR, *VI);
+
+ ++VI;
+ }
+ }
+
+ RecordDecl::field_iterator FI, FE;
+
for (FI = RD->field_begin(), FE = RD->field_end(); FI != FE; ++FI) {
if (VI == VE)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
