diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -270,6 +270,10 @@ `Issue 58679 `_ - Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of a function pointer. +- Fix a number of recursively-instantiated constraint issues, which would possibly + result in a stack overflow. + `Issue 44304 `_ + `Issue 50891 `_ Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5205,6 +5205,8 @@ def err_template_recursion_depth_exceeded : Error< "recursive template instantiation exceeded maximum depth of %0">, DefaultFatal, NoSFINAE; +def err_constraint_depends_on_self : Error< + "satisfaction of constraint '%0' depends on itself">, NoSFINAE; def note_template_recursion_depth : Note< "use -ftemplate-depth=N to increase recursive template instantiation depth">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7221,7 +7221,43 @@ FunctionDecl *FD, llvm::Optional> TemplateArgs, LocalInstantiationScope &Scope); +private: + // The current stack of constraint satisfactions, so we can exit-early. + llvm::SmallVector SatisfactionStack; + public: + void PushSatisfactionStackEntry(const llvm::FoldingSetNodeID &ID) { + SatisfactionStack.push_back(ID); + } + + void PopSatisfactionStackEntry() { SatisfactionStack.pop_back(); } + + bool SatisfactionStackContains(const llvm::FoldingSetNodeID &ID) const { + return llvm::find(SatisfactionStack, ID) != SatisfactionStack.end(); + } + + // Resets the current SatisfactionStack for cases where we are instantiating + // constraints as a 'side effect' of normal instantiation in a way that is not + // indicative of recursive definition. + class SatisfactionStackResetRAII { + llvm::SmallVector BackupSatisfactionStack; + Sema &SemaRef; + + public: + SatisfactionStackResetRAII(Sema &S) : SemaRef(S) { + SemaRef.SwapSatisfactionStack(BackupSatisfactionStack); + } + + ~SatisfactionStackResetRAII() { + SemaRef.SwapSatisfactionStack(BackupSatisfactionStack); + } + }; + + void + SwapSatisfactionStack(llvm::SmallVectorImpl &NewSS) { + SatisfactionStack.swap(NewSS); + } + const NormalizedConstraint * getNormalizedAssociatedConstraints( NamedDecl *ConstrainedDecl, ArrayRef AssociatedConstraints); diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -146,6 +146,17 @@ return true; } +namespace { +struct SatisfactionStackRAII { + Sema &SemaRef; + SatisfactionStackRAII(Sema &SemaRef, llvm::FoldingSetNodeID FSNID) + : SemaRef(SemaRef) { + SemaRef.PushSatisfactionStackEntry(FSNID); + } + ~SatisfactionStackRAII() { SemaRef.PopSatisfactionStackEntry(); } +}; +} // namespace + template static ExprResult calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr, @@ -258,6 +269,29 @@ return SubstitutedAtomicExpr; } +static bool +DiagRecursiveConstraintEval(Sema &S, llvm::FoldingSetNodeID &ID, const Expr *E, + const MultiLevelTemplateArgumentList &MLTAL) { + E->Profile(ID, S.Context, /*Canonical=*/true); + for (const auto &List : MLTAL) + for (const auto &TemplateArg : List.Args) + TemplateArg.Profile(ID, S.Context); + + // Note that we have to do this with our own collection, because there are + // times where a constraint-expression check can cause us to need to evaluate + // other constriants that are unrelated, such as when evaluating a recovery + // expression, or when trying to determine the constexpr-ness of special + // members. Otherwise we could just use the + // Sema::InstantiatingTemplate::isAlreadyBeingInstantiated function. + if (S.SatisfactionStackContains(ID)) { + S.Diag(E->getExprLoc(), diag::err_constraint_depends_on_self) + << const_cast(E) << E->getSourceRange(); + return true; + } + + return false; +} + static ExprResult calculateConstraintSatisfaction( Sema &S, const NamedDecl *Template, SourceLocation TemplateNameLoc, const MultiLevelTemplateArgumentList &MLTAL, const Expr *ConstraintExpr, @@ -278,6 +312,16 @@ AtomicExpr->getSourceRange()); if (Inst.isInvalid()) return ExprError(); + + llvm::FoldingSetNodeID ID; + if (DiagRecursiveConstraintEval(S, ID, AtomicExpr, MLTAL)) { + Satisfaction.IsSatisfied = false; + Satisfaction.ContainsErrors = true; + return ExprEmpty(); + } + + SatisfactionStackRAII StackRAII(S, ID); + // We do not want error diagnostics escaping here. Sema::SFINAETrap Trap(S); SubstitutedExpression = @@ -414,6 +458,7 @@ if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs, ConvertedConstraints, TemplateArgsLists, TemplateIDRange, *Satisfaction)) { + OutSatisfaction = *Satisfaction; return true; } diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7193,6 +7193,11 @@ bool ConstRHS, CXXConstructorDecl *InheritedCtor = nullptr, Sema::InheritedConstructorInfo *Inherited = nullptr) { + // Suppress duplicate constraint checking here, in case a constraint check + // caused us to decide to do this. Any truely recursive checks will get + // caught during these checks anyway. + Sema::SatisfactionStackResetRAII SSRAII{S}; + // If we're inheriting a constructor, see if we need to call it for this base // class. if (InheritedCtor) { @@ -7289,8 +7294,8 @@ // class is a constexpr function, and for (const auto &B : ClassDecl->bases()) { const RecordType *BaseType = B.getType()->getAs(); - if (!BaseType) continue; - + if (!BaseType) + continue; CXXRecordDecl *BaseClassDecl = cast(BaseType->getDecl()); if (!specialMemberIsConstexpr(S, BaseClassDecl, CSM, 0, ConstArg, InheritedCtor, Inherited)) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -13147,17 +13147,16 @@ namespace { class BuildRecoveryCallExprRAII { Sema &SemaRef; + Sema::SatisfactionStackResetRAII SatStack; + public: - BuildRecoveryCallExprRAII(Sema &S) : SemaRef(S) { + BuildRecoveryCallExprRAII(Sema &S) : SemaRef(S), SatStack(S) { assert(SemaRef.IsBuildingRecoveryCallExpr == false); SemaRef.IsBuildingRecoveryCallExpr = true; } - ~BuildRecoveryCallExprRAII() { - SemaRef.IsBuildingRecoveryCallExpr = false; - } + ~BuildRecoveryCallExprRAII() { SemaRef.IsBuildingRecoveryCallExpr = false; } }; - } /// Attempts to recover from a call where no functions were found. diff --git a/clang/test/SemaTemplate/concepts-GH53213.cpp b/clang/test/SemaTemplate/concepts-GH53213.cpp deleted file mode 100644 --- a/clang/test/SemaTemplate/concepts-GH53213.cpp +++ /dev/null @@ -1,49 +0,0 @@ -// RUN: %clang_cc1 -std=c++20 -verify %s -namespace GH53213 { -template -concept c = requires(T t) { f(t); }; // #CDEF - -auto f(c auto); // #FDEF - -void g() { - f(0); - // expected-error@-1{{no matching function for call to 'f'}} - // expected-note@#FDEF{{constraints not satisfied}} - // expected-note@#FDEF{{because 'int' does not satisfy 'c'}} - // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}} -} -} // namespace GH53213 - -namespace GH45736 { -struct constrained; - -template - struct type { - }; -template - constexpr bool f(type) { - return true; - } - -template - concept matches = f(type()); - - -struct constrained { - template requires matches - explicit constrained(U value) { - } -}; - -bool f(constrained const &) { - return true; -} - -struct outer { - constrained state; -}; - -bool f(outer const & x) { - return f(x.state); -} -} // namespace GH45736 diff --git a/clang/test/SemaTemplate/concepts-recursive-inst.cpp b/clang/test/SemaTemplate/concepts-recursive-inst.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaTemplate/concepts-recursive-inst.cpp @@ -0,0 +1,122 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +namespace GH53213 { +template +concept c = requires(T t) { f(t); }; // #CDEF + +auto f(c auto); // #FDEF + +void g() { + f(0); + // expected-error@-1{{no matching function for call to 'f'}} + // expected-note@#FDEF{{constraints not satisfied}} + // expected-note@#FDEF{{because 'int' does not satisfy 'c'}} + // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}} +} +} // namespace GH53213 + +namespace GH45736 { +struct constrained; + +template + struct type { + }; +template + constexpr bool f(type) { + return true; + } + +template + concept matches = f(type()); + + +struct constrained { + template requires matches + explicit constrained(U value) { + } +}; + +bool f(constrained const &) { + return true; +} + +struct outer { + constrained state; +}; + +bool f(outer const & x) { + return f(x.state); +} +} // namespace GH45736 + +namespace DirectRecursiveCheck { +template +concept NotInf = true; +template +concept Inf = requires(T& v){ // #INF_REQ + {begin(v)}; // #INF_BEGIN_EXPR +}; + +void begin(NotInf auto& v){ } // #NOTINF_BEGIN +// This lookup should fail, since it results in a recursive check. +// However, this is a 'hard failure'(not a SFINAE failure or constraints +// violation), so it needs to cause the entire lookup to fail. +void begin(Inf auto& v){ } // #INF_BEGIN + +struct my_range{ +} rng; + +void baz() { +auto it = begin(rng); // #BEGIN_CALL +// expected-error@#INF_BEGIN {{satisfaction of constraint 'Inf' depends on itself}} +// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}} +// expected-note@#INF_BEGIN_EXPR {{while checking constraint satisfaction for template 'begin' required here}} +// expected-note@#INF_BEGIN_EXPR {{while substituting deduced template arguments into function template 'begin'}} +// expected-note@#INF_BEGIN_EXPR {{in instantiation of requirement here}} +// expected-note@#INF_REQ {{while substituting template arguments into constraint expression here}} +// expected-note@#INF_BEGIN {{while checking the satisfaction of concept 'Inf' requested here}} +// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}} +// expected-note@#BEGIN_CALL {{while checking constraint satisfaction for template 'begin' required here}} +// expected-note@#BEGIN_CALL {{in instantiation of function template specialization}} + +// Fallout of the failure is failed lookup, which is necessary to stop odd +// cascading errors. +// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}} +// expected-note@#NOTINF_BEGIN {{candidate function}} +// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}} +} +} // namespace DirectRecursiveCheck + +namespace GH50891 { + template + concept Numeric = requires(T a) { // #NUMERIC + foo(a); // #FOO_CALL + }; + + struct Deferred { + friend void foo(Deferred); + template operator TO(); // #OP_TO + }; + + static_assert(Numeric); // #STATIC_ASSERT + // expected-error@#OP_TO {{satisfaction of constraint 'Numeric' depends on itself}} + // expected-note@#OP_TO {{while substituting template arguments into constraint expression here}} + // FIXME: The following two refer to type-parameter-0-0, it would be nice to + // see if we could instead diagnose with the sugared name. + // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}} + // expected-note@#FOO_CALL {{while substituting deduced template arguments into function template}} + // expected-note@#FOO_CALL {{in instantiation of requirement here}} + // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}} + // expected-note@#OP_TO {{skipping 2 contexts in backtrace}} + // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}} + // expected-note@#FOO_CALL {{in instantiation of function template specialization}} + // expected-note@#FOO_CALL {{in instantiation of requirement here}} + // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}} + // expected-note@#STATIC_ASSERT{{while checking the satisfaction of concept 'Numeric' requested here}} + + // Fallout of that failure is that deferred does not satisfy numeric, + // which is unfortunate, but about what we can accomplish here. + // expected-error@#STATIC_ASSERT {{static assertion failed}} + // expected-note@#STATIC_ASSERT{{because 'Deferred' does not satisfy 'Numeric'}} + // expected-note@#FOO_CALL {{because 'foo(a)' would be invalid}} +} // namespace GH50891 +