Author: erichkeane Date: Tue Dec 11 13:54:52 2018 New Revision: 348899 URL: http://llvm.org/viewvc/llvm-project?rev=348899&view=rev Log: 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 Modified: cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/assign.c Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=348899&r1=348898&r2=348899&view=diff ============================================================================== --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Tue Dec 11 13:54:52 2018 @@ -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; } Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=348899&r1=348898&r2=348899&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Dec 11 13:54:52 2018 @@ -11114,30 +11114,39 @@ static void DiagnoseRecursiveConstFields 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; } } Modified: cfe/trunk/test/Sema/assign.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/assign.c?rev=348899&r1=348898&r2=348899&view=diff ============================================================================== --- cfe/trunk/test/Sema/assign.c (original) +++ cfe/trunk/test/Sema/assign.c Tue Dec 11 13:54:52 2018 @@ -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'}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits