Skip to content

[5.3] Fix dynamic member lookup index-while-building crash #32168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jun 4, 2020
Merged

[5.3] Fix dynamic member lookup index-while-building crash #32168

merged 2 commits into from
Jun 4, 2020

Conversation

nathawes
Copy link
Contributor

@nathawes nathawes commented Jun 3, 2020 •
edited
Loading

  • Explanation:
    Index-while-building (which is enabled by default in Xcode) records the set of overridden decls for each decl it outputs in the index data. Protocols marked with the @dynamicMemberLookup attribute require conforming types to include a subscript(dynamicMember: KeyPath<..., ...>) member. If a default implementation of this member was provided, the compiler would crash trying to compute its overridden decls.
  • Scope of Issue: Crashes the compiler when compiling any source code with @dynamicMemberLookup specified on a protocol that also provides a default implementation of the required subscript member (i.e. valid code).
  • Origination: The change to add @main support modified the caller of the problematic code in the crashing case to pass a valid constraint system Locator with a null anchor rather than a null Locator like it used to: f7e2abe#diff-ba9f048d1813343154c6702f3188c8b8R4554
  • Risk: Low. The fix is a simple null check on the Locator's anchor before using it. This matches the behavior of the equivalent code on the master branch (where the anchor's representation has been changed and it does an equivalent check).
  • Testing: Added a regression test and all existing tests pass.
  • Reviewer: Pavel Yaskevich (@xedin)

PR for master (with just the test, since it doesn't reproduce there): #32165

Resolves rdar://problem/63558609

@nathawes nathawes added the r5.3 label Jun 3, 2020
@nathawes nathawes changed the title [WIP] Fix dynamic member lookup index-while-building crash [WIP][5.3] Fix dynamic member lookup index-while-building crash Jun 3, 2020
@nathawes
Copy link
Contributor Author

nathawes commented Jun 3, 2020

@swift-ci please test

@nathawes nathawes requested a review from xedin June 3, 2020 22:31
xedin
xedin approved these changes Jun 3, 2020
View reviewed changes
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to use isa_and_nonnull here instead of checking for anchor. We have this covered on master with isExpr null check.

[Sema] Add null checks in ConstraintSystem::bindOverloadType to preve…
851f372 
…nt a crash during indexing.

locator->getAnchor() may return a null Expr*, but was being passed directly to
isa<KeyPathExpr>(..) without a null check in two places.

Resovles rdar://problem/63558609
@nathawes nathawes force-pushed the dynamic-member-lookup-indexing-crash branch from 858b891 to 851f372 Compare June 3, 2020 23:53
@nathawes
Copy link
Contributor Author

nathawes commented Jun 3, 2020

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Jun 3, 2020

Build failed
Swift Test OS X Platform
Git Sha - 858b8914bb06d8bd50e752af2bf16259dda6e0c3

@swift-ci
Copy link
Contributor

swift-ci commented Jun 3, 2020

Build failed
Swift Test Linux Platform
Git Sha - 858b8914bb06d8bd50e752af2bf16259dda6e0c3

@nathawes nathawes changed the title [WIP][5.3] Fix dynamic member lookup index-while-building crash [5.3] Fix dynamic member lookup index-while-building crash Jun 4, 2020
@nathawes nathawes marked this pull request as ready for review June 4, 2020 00:07
@nathawes nathawes requested a review from a team as a code owner June 4, 2020 00:07
@nathawes
Copy link
Contributor Author

nathawes commented Jun 4, 2020

@swift-ci please nominate

@tkremenek tkremenek merged commit edf8c20 into swiftlang:release/5.3 Jun 4, 2020
@AnthonyLatsis AnthonyLatsis added swift 5.3 🍒 release cherry pick Flag: Release branch cherry picks labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin approved these changes

@tkremenek tkremenek tkremenek approved these changes

Assignees
No one assigned
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nathawes @swift-ci @xedin @tkremenek @AnthonyLatsis