[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Artem Dergachev via Phabricator via cfe-commits]

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)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59054.189594.patch
Type: text/x-patch
Size: 2704 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190306/11c508ab/attachment.bin>