Replace Const-Member checking with non-recursive version.

As reported in PR39946, these two implementations cause stack overflows
to occur when a type recursively contains itself.  While this only
happens when an incomplete version of itself is used by membership (and
thus an otherwise invalid program), the crashes might be surprising.

The solution here is to replace the recursive implementation with one
that uses a std::vector as a queue.  Old values are kept around to
prevent re-checking already checked types.

Change-Id: I582bb27147104763d7daefcfee39d91f408b9fa8

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@348899 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index 89ab2ce1fcc75f5c7f8c6006298a6a258a90bf7b..b4e82d90264461680933f169b239d5137f883646 100644 (file)
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -3166,14 +3166,23 @@ bool TagType::isBeingDefined() const {
 }
 
 bool RecordType::hasConstFields() const {
-  for (FieldDecl *FD : getDecl()->fields()) {
-    QualType FieldTy = FD->getType();
-    if (FieldTy.isConstQualified())
-      return true;
-    FieldTy = FieldTy.getCanonicalType();
-    if (const auto *FieldRecTy = FieldTy->getAs<RecordType>())
-      if (FieldRecTy->hasConstFields())
+  std::vector<const RecordType*> RecordTypeList;
+  RecordTypeList.push_back(this);
+  unsigned NextToCheckIndex = 0;
+
+  while (RecordTypeList.size() > NextToCheckIndex) {
+    for (FieldDecl *FD :
+         RecordTypeList[NextToCheckIndex]->getDecl()->fields()) {
+      QualType FieldTy = FD->getType();
+      if (FieldTy.isConstQualified())
         return true;
+      FieldTy = FieldTy.getCanonicalType();
+      if (const auto *FieldRecTy = FieldTy->getAs<RecordType>()) {
+        if (llvm::find(RecordTypeList, FieldRecTy) == RecordTypeList.end())
+          RecordTypeList.push_back(FieldRecTy);
+      }
+    }
+    ++NextToCheckIndex;
   }
   return false;
 }

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index ab8806302d02b59935ba60e9dc556d24b16f00e0..29a2f05aea1e4afb8416caa44c1dee076ff694b5 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -11114,30 +11114,39 @@ static void DiagnoseRecursiveConstFields(Sema &S, const ValueDecl *VD,
                                          const RecordType *Ty,
                                          SourceLocation Loc, SourceRange Range,
                                          OriginalExprKind OEK,
-                                         bool &DiagnosticEmitted,
-                                         bool IsNested = false) {
+                                         bool &DiagnosticEmitted) {
+  std::vector<const RecordType *> RecordTypeList;
+  RecordTypeList.push_back(Ty);
+  unsigned NextToCheckIndex = 0;
+  // TODO: MAKE THIS NOT RECURSIVE
   // We walk the record hierarchy breadth-first to ensure that we print
   // diagnostics in field nesting order.
-  // First, check every field for constness.
-  for (const FieldDecl *Field : Ty->getDecl()->fields()) {
-    if (Field->getType().isConstQualified()) {
-      if (!DiagnosticEmitted) {
-        S.Diag(Loc, diag::err_typecheck_assign_const)
-            << Range << NestedConstMember << OEK << VD
-            << IsNested << Field;
-        DiagnosticEmitted = true;
+  while (RecordTypeList.size() > NextToCheckIndex) {
+    bool IsNested = NextToCheckIndex > 0;
+    for (const FieldDecl *Field :
+         RecordTypeList[NextToCheckIndex]->getDecl()->fields()) {
+      // First, check every field for constness.
+      QualType FieldTy = Field->getType();
+      if (FieldTy.isConstQualified()) {
+        if (!DiagnosticEmitted) {
+          S.Diag(Loc, diag::err_typecheck_assign_const)
+              << Range << NestedConstMember << OEK << VD
+              << IsNested << Field;
+          DiagnosticEmitted = true;
+        }
+        S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
+            << NestedConstMember << IsNested << Field
+            << FieldTy << Field->getSourceRange();
+      }
+
+      // Then we append it to the list to check next in order.
+      FieldTy = FieldTy.getCanonicalType();
+      if (const auto *FieldRecTy = FieldTy->getAs<RecordType>()) {
+        if (llvm::find(RecordTypeList, FieldRecTy) == RecordTypeList.end())
+          RecordTypeList.push_back(FieldRecTy);
       }
-      S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
-          << NestedConstMember << IsNested << Field
-          << Field->getType() << Field->getSourceRange();
     }
-  }
-  // Then, recurse.
-  for (const FieldDecl *Field : Ty->getDecl()->fields()) {
-    QualType FTy = Field->getType();
-    if (const RecordType *FieldRecTy = FTy->getAs<RecordType>())
-      DiagnoseRecursiveConstFields(S, VD, FieldRecTy, Loc, Range,
-                                   OEK, DiagnosticEmitted, true);
+    ++NextToCheckIndex;
   }
 }
 

diff --git a/test/Sema/assign.c b/test/Sema/assign.c
index 28d7b9f3bbdfc180d96ab6e198fbf7379ffea129..4d305aca4b5ac15e1617f71b31afbae3fdeee76a 100644 (file)
--- a/test/Sema/assign.c
+++ b/test/Sema/assign.c
@@ -59,3 +59,37 @@ void testK1_(K k, J j) {
 void testK2_(K k, I i) {
   k.j->i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
 }
+
+// PR39946: Recursive checking of hasConstFields caused stack overflow.
+struct L { // expected-note {{definition of 'struct L' is not complete until the closing '}'}}
+  struct L field; // expected-error {{field has incomplete type 'struct L'}}
+};
+void testL(struct L *l) {
+  *l = 0; // expected-error {{assigning to 'struct L' from incompatible type 'int'}}
+}
+
+// Additionally, this example overflowed the stack when figuring out the field.
+struct M1; // expected-note {{forward declaration of 'struct M1'}}
+struct M2 {
+  //expected-note@+1 {{nested data member 'field' declared const here}}
+  const struct M1 field; // expected-error {{field has incomplete type 'const struct M1'}}
+};
+struct M1 {
+  struct M2 field;
+};
+
+void testM(struct M1 *l) {
+  *l = 0; // expected-error {{cannot assign to lvalue with nested const-qualified data member 'field'}}
+}
+
+struct N1; // expected-note {{forward declaration of 'struct N1'}}
+struct N2 {
+  struct N1 field; // expected-error {{field has incomplete type 'struct N1'}}
+};
+struct N1 {
+  struct N2 field;
+};
+
+void testN(struct N1 *l) {
+  *l = 0; // expected-error {{assigning to 'struct N1' from incompatible type 'int'}}
+}