r348899 - Replace Const-Member checking with non-recursive version.
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 11 13:54:52 PST 2018
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 at +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'}}
+}
More information about the cfe-commits
mailing list