Skip to content

[4.2] ASTContext: Recompute the insert position in getSpecializedConformance after the SpecializedProtocolConformance constructor #17916

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.

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

Conversation

aschwaighofer
Copy link
Contributor

The constructor can modify the SpecializedConformances FoldingSet so that the
insertPos is no longer valid. It calls computeConditionalRequirements which
calls Type::subst which recursively can call getSpecializedConformance again.

Therefore, we need to recompute the insertPos after calling the
constructor.

This bug manifest itself either as memory error with libgmalloc or as
spurious errors later on.

  • Explanation: Memory errors causing spurious failure later in the
    compiler on our CI tests

  • Scope: Memory errors are bad. This bug dates back to when conditional
    conformance were added sometime during 2017.

  • Risk: Low. A call to recompute the insertion point (a map lookup) is
    added.

*Testing: Existing Swift CI test caught this

rdar://42082352

…ormance

after the SpecializedProtocolConformance constructor

The constructor can modify the SpecializedConformances FoldingSet so that the
insertPos is no longer valid. It calls computeConditionalRequirements which
calls Type::subst which recursively can call getSpecializedConformance again.

Therefore, we need to recompute the insertPos after calling the
constructor.

This bug manifest itself either as memory error with libgmalloc or as
spurious errors later on.

* Explanation: Memory errors causing spurious failure later in the
compiler on our CI tests

* Scope: Memory errors are bad. This bug dates back to when conditional
conformance were added sometime during 2017.

* Risk: Low. A call to recompute the insertion point (a map lookup) is
added.

*Testing: Existing Swift CI test caught this

rdar://42082352
@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please nominate

@aschwaighofer aschwaighofer merged commit e4ddba2 into swiftlang:swift-4.2-branch Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants