Skip to content

Patch a Huge Soundness Hole in Codable Synthesis #36060

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
merged 4 commits into from
Feb 22, 2021
Merged

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Feb 20, 2021

The order of diagnostic emission absolutely does not matter. What this transaction was actually doing was suppressing valid diagnostics. This is a deeply unsound thing to do since if errors are emitted but Codable synthesis succeeds then invalid code can make its way past Sema.

rdar://74392492

Doing this eagerly makes it more difficult to determine when something fails by observing the creation of ErrorTypes
@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 20, 2021

@swift-ci test

@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 20, 2021

@swift-ci test source compatibility

The order of diagnostic emission absolutely does not matter. What this transaction was actually doing was suppressing valid diagnostics. This is a deeply unsound thing to do since if errors are emitted but Codable synthesis succeeds then invalid code can make its way past Sema.

rdar://74392492
@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 20, 2021

@swift-ci smoke test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 07c1c9b1a517e1e758cfd51635cc67bc3ee9f77d

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 07c1c9b1a517e1e758cfd51635cc67bc3ee9f77d

@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 20, 2021

@swift-ci smoke test

1 similar comment
@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 21, 2021

@swift-ci smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Feb 22, 2021

@CodaFi CodaFi merged commit b8329b6 into swiftlang:main Feb 22, 2021
@CodaFi CodaFi deleted the codicil branch February 22, 2021 16:48
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