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
llvm-svn: 348899

diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 89ab2ce..b4e82d9 100644 (file)
--- a/clang/lib/AST/Type.cpp
+++ b/clang/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/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ab88063..29a2f05 100644 (file)
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/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/clang/test/Sema/assign.c b/clang/test/Sema/assign.c
index 28d7b9f..4d305ac 100644 (file)
--- a/clang/test/Sema/assign.c
+++ b/clang/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'}}
+}