NoQ updated this revision to Diff 189781.
NoQ marked 5 inline comments as done.
NoQ added a comment.
Thx! Also added the more direct test.
> I'll take the time to understand the infrastructure behind it
Well, roughly, the `CompoundVal` kind of value is used very rarely, unlike
`LazyCompoundVal`. 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.
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; the Store expects to match them to
sub-objects, essentially, by *index*: first value binds to the first field,
second value binds to the second field, etc. The crash was occuring when it was
accidentally trying to bind a base-class value to a field object - because it
simply didn't know that base objects are a thing.
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
@@ -2337,9 +2337,38 @@
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
