NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs,
mikhail.ramalho, Szelethus, baloghadamsoftware.
Herald added subscribers: cfe-commits, Charusso, jdoerfert, dkrupp, donat.nagy,
a.sidorin, szepet.
Herald added a project: clang.
I'll add more tests soon, to see if it is actually initialized *correctly* now,
but for now here's the code that fixes the crash in
https://bugs.llvm.org/show_bug.cgi?id=40022 .
It turns out that C++17 aggregate initialization with base classes wasn't
really ever implemented for the case when it *doesn't* involve calling any
sub-object constructors. That it, when the aggregate is initialized with a
simple initializer list and the resulting value is a `nonloc::CompoundVal`, we
simply didn't know that we need to perform the store bind for the base classes
as well. Now it should work.
The original bug report causes us to crash when there's a flexible array at the
end of the structure (which is a non-standard thing), which made me
underestimate the problem a bit :)
The case where we *do* have constructors in base classes is, of course, still
unimplemented; it was last touched in D40841 <https://reviews.llvm.org/D40841>.
Repository:
rC Clang
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,9 @@
// 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 -verify -x c++
-std=c++17 -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,debug.ExprInspection -DINLINE -verify -x c++
-std=c++17 -analyzer-config c++-inlining=constructors %s
void clang_analyzer_eval(int);
@@ -196,4 +198,21 @@
}
}
+namespace flex_array_inheritance_cxx17 {
+struct A {
+ int flexible_array[];
+};
+
+struct B {
+ long cookie;
+};
+
+struct C : B {
+ A a;
+};
+
+void k() {
+ C c{};
+}
+} // 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,37 @@
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());
+
+ // Virtual bases still aren't allowed. Multiple bases are fine though.
+ for (auto B : CRD->bases()) {
+ assert(B.isVirtual() == false);
+
+ if (VI == VE)
+ break;
+
+ QualType BTy = B.getType();
+ assert(BTy->isStructureOrClassType());
+
+ const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
+ assert(BRD);
+
+ 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)
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,9 @@
// 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 -verify -x c++ -std=c++17 -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,debug.ExprInspection -DINLINE -verify -x c++ -std=c++17 -analyzer-config c++-inlining=constructors %s
void clang_analyzer_eval(int);
@@ -196,4 +198,21 @@
}
}
+namespace flex_array_inheritance_cxx17 {
+struct A {
+ int flexible_array[];
+};
+
+struct B {
+ long cookie;
+};
+
+struct C : B {
+ A a;
+};
+
+void k() {
+ C c{};
+}
+} // 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,37 @@
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());
+
+ // Virtual bases still aren't allowed. Multiple bases are fine though.
+ for (auto B : CRD->bases()) {
+ assert(B.isVirtual() == false);
+
+ if (VI == VE)
+ break;
+
+ QualType BTy = B.getType();
+ assert(BTy->isStructureOrClassType());
+
+ const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
+ assert(BRD);
+
+ 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
