3 files changed, 72 insertions, 2 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index 23b43759a34..db9179e018a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -22,6 +22,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -262,8 +263,19 @@ void DynamicTypePropagation::checkPostCall(const CallEvent &Call,
       if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion()) {
         // We just finished a base constructor. Now we can use the subclass's
         // type when resolving virtual calls.
-        const Decl *D = C.getLocationContext()->getDecl();
-        recordFixedType(Target, cast<CXXConstructorDecl>(D), C);
+        const LocationContext *LCtx = C.getLocationContext();
+
+        // FIXME: In C++17 classes with non-virtual bases may be treated as
+        // aggregates, and in such case no top-frame constructor will be called.
+        // Figure out if we need to do anything in this case.
+        // FIXME: Instead of relying on the ParentMap, we should have the
+        // trigger-statement (InitListExpr in this case) available in this
+        // callback, ideally as part of CallEvent.
+        if (dyn_cast_or_null<InitListExpr>(
+                LCtx->getParentMap().getParent(Ctor->getOriginExpr())))
+          return;
+
+        recordFixedType(Target, cast<CXXConstructorDecl>(LCtx->getDecl()), C);
       }
       return;
     }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 03e0095d0e8..dad93111966 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/StmtCXX.h"
+#include "clang/AST/ParentMap.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
@@ -267,6 +268,23 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
     }
     // FALLTHROUGH
   case CXXConstructExpr::CK_NonVirtualBase:
+    // In C++17, classes with non-virtual bases may be aggregates, so they would
+    // be initialized as aggregates without a constructor call, so we may have
+    // a base class constructed directly into an initializer list without
+    // having the derived-class constructor call on the previous stack frame.
+    // Initializer lists may be nested into more initializer lists that
+    // correspond to surrounding aggregate initializations.
+    // FIXME: For now this code essentially bails out. We need to find the
+    // correct target region and set it.
+    // FIXME: Instead of relying on the ParentMap, we should have the
+    // trigger-statement (InitListExpr in this case) passed down from CFG or
+    // otherwise always available during construction.
+    if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) {
+      MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
+      Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
+      break;
+    }
+    // FALLTHROUGH
   case CXXConstructExpr::CK_Delegating: {
     const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
     Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp
index 6359b93d0a2..55f0a895028 100644
--- a/clang/test/Analysis/initializer.cpp
+++ b/clang/test/Analysis/initializer.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++17 -DCPLUSPLUS17 -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -224,3 +225,42 @@ void testPassListsWithExplicitConstructors() {
   (void)(std::initializer_list<int>){12}; // no-crash
 }
 }
+
+namespace CXX17_aggregate_construction {
+struct A {
+  A();
+};
+
+struct B: public A {
+};
+
+struct C: public B {
+};
+
+struct D: public virtual A {
+};
+
+// In C++17, classes B and C are aggregates, so they will be constructed
+// without actually calling their trivial constructor. Used to crash.
+void foo() {
+  B b = {}; // no-crash
+  const B &bl = {}; // no-crash
+  B &&br = {}; // no-crash
+
+  C c = {}; // no-crash
+  const C &cl = {}; // no-crash
+  C &&cr = {}; // no-crash
+
+  D d = {}; // no-crash
+
+#ifdef CPLUSPLUS17
+  C cd = {{}}; // no-crash
+  const C &cdl = {{}}; // no-crash
+  C &&cdr = {{}}; // no-crash
+
+  const B &bll = {{}}; // no-crash
+  const B &bcl = C({{}}); // no-crash
+  B &&bcr = C({{}}); // no-crash
+#endif
+}
+}