From fb7f2d0ff2cf6e6c21f921fd556c4824809a2e20 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Thu, 31 Jul 2025 18:49:45 +0100 Subject: [PATCH] [CS] Limit the number of chained `@dynamicMemberLookup` lookups Set an upper bound on the number of chained lookups we attempt to avoid spinning while trying to recursively apply the same dynamic member lookup to itself. rdar://157288911 --- include/swift/AST/DiagnosticsSema.def | 5 ++++ include/swift/Basic/LangOptions.h | 4 +++ include/swift/Option/FrontendOptions.td | 6 ++++ include/swift/Sema/CSFix.h | 30 +++++++++++++++++++ lib/Frontend/CompilerInvocation.cpp | 2 ++ lib/Sema/CSDiagnostics.cpp | 6 ++++ lib/Sema/CSDiagnostics.h | 11 +++++++ lib/Sema/CSFix.cpp | 12 ++++++++ lib/Sema/CSSimplify.cpp | 1 + lib/Sema/TypeOfReference.cpp | 24 +++++++++++---- .../dynamic_member_lookup_limit.swift | 21 +++++++++++++ test/attr/attr_dynamic_member_lookup.swift | 30 +++++++++++++++++++ .../IDE/crashers/fd52bd37cd5c96.swift | 13 -------- .../IDE/crashers_fixed/fd52bd37cd5c96.swift | 8 +++++ .../a5fbf4336df96867.swift | 6 +--- 15 files changed, 156 insertions(+), 23 deletions(-) create mode 100644 test/Frontend/dynamic_member_lookup_limit.swift delete mode 100644 validation-test/IDE/crashers/fd52bd37cd5c96.swift create mode 100644 validation-test/IDE/crashers_fixed/fd52bd37cd5c96.swift rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/a5fbf4336df96867.swift (51%) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 2b1622edaec..d0ae00504ae 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1735,6 +1735,11 @@ ERROR(dynamic_member_lookup_candidate_inaccessible,none, "enclosing type", (ValueDecl *)) +ERROR(too_many_dynamic_member_lookups,none, + "could not find member %0; exceeded the maximum number of nested " + "dynamic member lookups", + (DeclNameRef)) + ERROR(string_index_not_integer,none, "String must not be indexed with %0, it has variable size elements", (Type)) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index 0b650a7d1f6..09b398763c0 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -952,6 +952,10 @@ namespace swift { /// (It's arbitrary, but will keep the compiler from taking too much time.) unsigned SwitchCheckingInvocationThreshold = 200000; + /// The maximum number of `@dynamicMemberLookup`s that can be chained to + /// resolve a member reference. + unsigned DynamicMemberLookupDepthLimit = 100; + /// If true, the time it takes to type-check each function will be dumped /// to llvm::errs(). bool DebugTimeFunctionBodies = false; diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index dfe39ff374a..a032b6caecc 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -895,6 +895,12 @@ def disable_invalid_ephemeralness_as_error : def switch_checking_invocation_threshold_EQ : Joined<["-"], "switch-checking-invocation-threshold=">; +def dynamic_member_lookup_depth_limit_EQ + : Joined<["-"], "dynamic-member-lookup-depth-limit=">, + HelpText< + "The maximum number of dynamic member lookups that can be chained " + "to resolve a member reference">; + def enable_new_operator_lookup : Flag<["-"], "enable-new-operator-lookup">, HelpText<"Enable the new operator decl and precedencegroup lookup behavior">; diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index a4c138d7d8f..f7c8fcb46cc 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -488,6 +488,9 @@ enum class FixKind : uint8_t { /// the type it's attempting to bind to. AllowInlineArrayLiteralCountMismatch, + /// Reached the limit of @dynamicMemberLookup depth. + TooManyDynamicMemberLookups, + /// Ignore that a conformance is isolated but is not allowed to be. IgnoreIsolatedConformance, }; @@ -3881,6 +3884,33 @@ public: } }; +class TooManyDynamicMemberLookups : public ConstraintFix { + DeclNameRef Name; + + TooManyDynamicMemberLookups(ConstraintSystem &cs, DeclNameRef name, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::TooManyDynamicMemberLookups, locator), + Name(name) {} + +public: + std::string getName() const override { + return "too many dynamic member lookups"; + } + + bool diagnose(const Solution &solution, bool asNote = false) const override; + + bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override { + return diagnose(*commonFixes.front().first); + } + + static TooManyDynamicMemberLookups * + create(ConstraintSystem &cs, DeclNameRef name, ConstraintLocator *locator); + + static bool classof(const ConstraintFix *fix) { + return fix->getKind() == FixKind::TooManyDynamicMemberLookups; + } +}; + class IgnoreIsolatedConformance : public ConstraintFix { ProtocolConformance *conformance; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index f07a6659eba..130b6149431 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1897,6 +1897,8 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args, Opts.WarnLongExpressionTypeChecking); setUnsignedIntegerArgument(OPT_solver_expression_time_threshold_EQ, Opts.ExpressionTimeoutThreshold); + setUnsignedIntegerArgument(OPT_dynamic_member_lookup_depth_limit_EQ, + Opts.DynamicMemberLookupDepthLimit); setUnsignedIntegerArgument(OPT_switch_checking_invocation_threshold_EQ, Opts.SwitchCheckingInvocationThreshold); setUnsignedIntegerArgument(OPT_debug_constraints_attempt, diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 96f2003fa1c..c7dbef1748d 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -9628,6 +9628,12 @@ bool IncorrectInlineArrayLiteralCount::diagnoseAsError() { return true; } +bool TooManyDynamicMemberLookupsFailure::diagnoseAsError() { + emitDiagnostic(diag::too_many_dynamic_member_lookups, Name) + .highlight(getSourceRange()); + return true; +} + bool DisallowedIsolatedConformance::diagnoseAsError() { emitDiagnostic(diag::isolated_conformance_with_sendable_simple, conformance->getType(), diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 1c58ae5862b..5c65d8a23c0 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -3310,6 +3310,17 @@ public: bool diagnoseAsError() override; }; +class TooManyDynamicMemberLookupsFailure final : public FailureDiagnostic { + DeclNameRef Name; + +public: + TooManyDynamicMemberLookupsFailure(const Solution &solution, DeclNameRef name, + ConstraintLocator *locator) + : FailureDiagnostic(solution, locator), Name(name) {} + + bool diagnoseAsError() override; +}; + /// Diagnose when an isolated conformance is used in a place where one cannot /// be, e.g., due to a Sendable or SendableMetatype requirement on the /// corresponding type parameter. diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 91f7254a03f..816e2b28d9e 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -2796,6 +2796,18 @@ bool AllowInlineArrayLiteralCountMismatch::diagnose(const Solution &solution, return failure.diagnose(asNote); } +TooManyDynamicMemberLookups * +TooManyDynamicMemberLookups::create(ConstraintSystem &cs, DeclNameRef name, + ConstraintLocator *locator) { + return new (cs.getAllocator()) TooManyDynamicMemberLookups(cs, name, locator); +} + +bool TooManyDynamicMemberLookups::diagnose(const Solution &solution, + bool asNote) const { + TooManyDynamicMemberLookupsFailure failure(solution, Name, getLocator()); + return failure.diagnose(asNote); +} + IgnoreIsolatedConformance * IgnoreIsolatedConformance::create(ConstraintSystem &cs, ConstraintLocator *locator, diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 762e06c5731..eff7db0f95e 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -16034,6 +16034,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::IgnoreOutOfPlaceThenStmt: case FixKind::IgnoreMissingEachKeyword: case FixKind::AllowInlineArrayLiteralCountMismatch: + case FixKind::TooManyDynamicMemberLookups: case FixKind::IgnoreIsolatedConformance: llvm_unreachable("handled elsewhere"); } diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index 529ac394f66..815da69f313 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -2191,11 +2191,25 @@ void ConstraintSystem::bindOverloadType(const SelectedOverload &overload, // FIXME: Should propagate name-as-written through. : DeclNameRef(choice.getName()); - addValueMemberConstraint( - LValueType::get(rootTy), memberName, memberTy, useDC, - isSubscriptRef ? FunctionRefInfo::doubleBaseNameApply() - : FunctionRefInfo::unappliedBaseName(), - /*outerAlternatives=*/{}, keyPathLoc); + // Check the current depth of applied dynamic member lookups, if we've + // exceeded the limit then record a fix and set a hole for the member. + unsigned lookupDepth = [&]() { + auto path = keyPathLoc->getPath(); + auto iter = path.begin(); + (void)keyPathLoc->findFirst(iter); + return path.end() - iter; + }(); + if (lookupDepth > ctx.TypeCheckerOpts.DynamicMemberLookupDepthLimit) { + (void)recordFix(TooManyDynamicMemberLookups::create( + *this, DeclNameRef(choice.getName()), locator)); + recordTypeVariablesAsHoles(memberTy); + } else { + addValueMemberConstraint( + LValueType::get(rootTy), memberName, memberTy, useDC, + isSubscriptRef ? FunctionRefInfo::doubleBaseNameApply() + : FunctionRefInfo::unappliedBaseName(), + /*outerAlternatives=*/{}, keyPathLoc); + } // In case of subscript things are more complicated comparing to "dot" // syntax, because we have to get "applicable function" constraint diff --git a/test/Frontend/dynamic_member_lookup_limit.swift b/test/Frontend/dynamic_member_lookup_limit.swift new file mode 100644 index 00000000000..1b8103c7d80 --- /dev/null +++ b/test/Frontend/dynamic_member_lookup_limit.swift @@ -0,0 +1,21 @@ +// RUN: %target-typecheck-verify-swift -dynamic-member-lookup-depth-limit=2 + +@dynamicMemberLookup +struct Lens { + init() {} + subscript(dynamicMember kp: KeyPath) -> U { + fatalError() + } +} + +struct S { + var x: Int +} + +_ = \Lens.x // Fine +_ = \Lens>.x // Also fine +_ = \Lens>>.x // expected-error {{could not find member 'x'; exceeded the maximum number of nested dynamic member lookups}} + +_ = Lens().x // Fine +_ = Lens>().x // Also fine +_ = Lens>>().x // expected-error {{could not find member 'x'; exceeded the maximum number of nested dynamic member lookups}} diff --git a/test/attr/attr_dynamic_member_lookup.swift b/test/attr/attr_dynamic_member_lookup.swift index 780d51123bd..a9c3aa3b674 100644 --- a/test/attr/attr_dynamic_member_lookup.swift +++ b/test/attr/attr_dynamic_member_lookup.swift @@ -978,3 +978,33 @@ struct WithSendable { get { false } } } + +// Make sure we enforce a limit on the number of chained dynamic member lookups. +@dynamicMemberLookup +struct SelfRecursiveLookup { + init(_: () -> T) {} + init(_: () -> KeyPath) {} + subscript(dynamicMember kp: KeyPath) -> SelfRecursiveLookup {} +} +let selfRecurse1 = SelfRecursiveLookup { selfRecurse1.e } +// expected-error@-1 {{could not find member 'e'; exceeded the maximum number of nested dynamic member lookups}} + +let selfRecurse2 = SelfRecursiveLookup { selfRecurse2[a: 0] } +// expected-error@-1 {{could not find member 'subscript'; exceeded the maximum number of nested dynamic member lookups}} + +let selfRecurse3 = SelfRecursiveLookup { \.e } +// expected-error@-1 {{could not find member 'e'; exceeded the maximum number of nested dynamic member lookups}} + +let selfRecurse4 = SelfRecursiveLookup { \.[a: 0] } +// expected-error@-1 {{could not find member 'subscript'; exceeded the maximum number of nested dynamic member lookups}} + +extension SelfRecursiveLookup where T == SelfRecursiveLookup>> { + var terminator: T { fatalError() } + subscript(terminator terminator: Int) -> T { fatalError() } +} + +let selfRecurse5 = SelfRecursiveLookup { selfRecurse5.terminator } +let selfRecurse6 = SelfRecursiveLookup { selfRecurse6[terminator: 0] } + +let selfRecurse7 = SelfRecursiveLookup { \.terminator } +let selfRecurse8 = SelfRecursiveLookup { \.[terminator: 0] } diff --git a/validation-test/IDE/crashers/fd52bd37cd5c96.swift b/validation-test/IDE/crashers/fd52bd37cd5c96.swift deleted file mode 100644 index 9ca5e95f9f5..00000000000 --- a/validation-test/IDE/crashers/fd52bd37cd5c96.swift +++ /dev/null @@ -1,13 +0,0 @@ -// {"kind":"complete","original":"ea806f48","signature":"swift::Type::transformRec(llvm::function_ref (swift::TypeBase*)>) const"} -// The issue here is that the solver attempts to recursively apply the same -// dynamic member lookup until eventually it overflows the stack. Make sure -// we either timeout or crash. -// RUN: not %{python} %S/../../../test/Inputs/timeout.py 60 \ -// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s || \ -// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s -@dynamicMemberLookup -struct a b^subscript(dynamicMember e: WritableKeyPath) a } -let binding = a -{ buffer #^^#?? -binding.0 diff --git a/validation-test/IDE/crashers_fixed/fd52bd37cd5c96.swift b/validation-test/IDE/crashers_fixed/fd52bd37cd5c96.swift new file mode 100644 index 00000000000..4c7bd628394 --- /dev/null +++ b/validation-test/IDE/crashers_fixed/fd52bd37cd5c96.swift @@ -0,0 +1,8 @@ +// {"kind":"complete","original":"ea806f48","signature":"swift::Type::transformRec(llvm::function_ref (swift::TypeBase*)>) const"} +// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s +@dynamicMemberLookup +struct a b^subscript(dynamicMember e: WritableKeyPath) a } +let binding = a +{ buffer #^^#?? +binding.0 diff --git a/validation-test/compiler_crashers_2/a5fbf4336df96867.swift b/validation-test/compiler_crashers_2_fixed/a5fbf4336df96867.swift similarity index 51% rename from validation-test/compiler_crashers_2/a5fbf4336df96867.swift rename to validation-test/compiler_crashers_2_fixed/a5fbf4336df96867.swift index fe108279f9d..c3d405f027e 100644 --- a/validation-test/compiler_crashers_2/a5fbf4336df96867.swift +++ b/validation-test/compiler_crashers_2_fixed/a5fbf4336df96867.swift @@ -1,9 +1,5 @@ // {"kind":"typecheck","signature":"swift::TypeTransform (swift::TypeBase*)>) const::Transform>::doIt(swift::Type, swift::TypePosition)"} -// The issue here is that the solver attempts to recursively apply the same -// dynamic member lookup until eventually it overflows the stack. Make sure -// we either timeout or crash. -// RUN: not %{python} %S/../../test/Inputs/timeout.py 60 %target-swift-frontend -typecheck %s || \ -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s @dynamicMemberLookup struct S { init(_: () -> T) {} subscript(dynamicMember d: WritableKeyPath) -> S {}