Skip to content

[5.5-05142021][Serialization] Prevent crash when inherited types are invalid #37522

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

Merged

Conversation

bnbarham
Copy link
Contributor

@bnbarham bnbarham commented May 20, 2021

Cherry-picks #37441 (the main change) and #37519 (small test change to prevent conflicts between main and 5.5).

For most cases this is just an assert, but for protocols it can end up being a crash due to the extra check it performs (if (element.getType()->is<ProtocolType>())).


When compiling with allow errors, it's possible to have invalid
inherited types - both null and ErrorType.

Cleaned up the tests a little - moved the majority of
Frontend/allow-errors.swift into separate files in
Serialization/AllowErrors and use split_file.py instead of #defines.

Resolves rdar://78048470

bnbarham added 2 commits May 20, 2021 10:53
When compiling with allow errors, it's possible to have invalid
inherited types - both null and ErrorType.

Cleaned up the tests a little - moved the majority of
Frontend/allow-errors.swift into separate files in
Serialization/AllowErrors and use split_file.py instead of #defines.

Resolves rdar://78048470
There was a change in the diagnostics that isn't in 5.5. The error
message itself doesn't matter too much here, just that there was one.
Simplify the CHECK clause so that it will pass in main and 5.5 to avoid
future conflicts.
@bnbarham
Copy link
Contributor Author

@swift-ci please test

@bnbarham
Copy link
Contributor Author

@swift-ci please nominate

Explanation:
Background indexing now has its own build arena, where source is compiled with an option to allow outputting the .swiftmodule regardless of compiler errors. In this mode, the following code will result in the inherited types being null rather than an ErrorType:

extension undefined {
  class Anything: undefined {...}
...
}

For class/struct/enum, this only causes an assertion and release builds are unaffected. Protocols, however, will end up generating a SIGSEGV. While it's invalid to define a protocol within an extension, it wouldn't be an uncommon case to hit while editing, ie.

extension CurrentlyAdding {
...

protocol AlreadyExists: undefined {...}

Radar/SR Issue: rdar://78048470

Risk: Low. This change adds null checks before using the inherited type.

Testing: Added new test cases to check for these cases (protocol/class/struct/enum with an undefined parent in an undefined extension).

@bnbarham bnbarham added the r5.5 label May 20, 2021
@akyrtzi akyrtzi merged commit 715c666 into swiftlang:release/5.5-05142021 May 20, 2021
@bnbarham bnbarham deleted the beta-1-cherry-rdar78048470 branch May 20, 2021 22:50
@bnbarham bnbarham changed the title [Beta-1][Serialization] Prevent crash when inherited types are invalid [5.5-05142021][Serialization] Prevent crash when inherited types are invalid May 21, 2021
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a03b891

@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants