[ClangImporter] Protect against re-entrant bridging header loading #27045
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If, while loading a bridging header, we pick up a Clang module that claims to have an overlay Swift module, and that Swift module turns out to have a bridging header, we can end up reallocating the array of modules to process while we're looping over it. Be defensive against this occurrence. This just fixes a crash; it does not at all solve the problem of this being broken in several ways: - Accidentally naming your module the same as a system module shadows the latter (if the system module is a Swift module) or *makes your module into an overlay* (if the system module is a Clang module). - Bridging headers are only officially supported on executable targets and unit tests, but this isn't really enforced. - Implicit inclusion of a bridging header *when you import a Swift module* is a hack to begin with, and a hack that worsens when the main module also has a bridging header. (All the bridging headers get folded together into the "same" module, which leads to more visibility than desired as well as cycles in the import graph.) - Combining all of these can result in some pretty bizarre behavior. rdar://problem/54581756
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
harlanhaskins
approved these changes
Sep 5, 2019
|
After discovering the cause (app target named the same as a system framework) I went and implemented a whole different thing (trying to avoid thinking the app module was the overlay for the system framework) and it didn't actually fix the crash. I'll throw up the PR for that too but it was kind of a mess and doesn't fix everything so I'm not sure it's worth it. |
|
This is the force-push bug, I think. @swift-ci Please test Windows |
xymus
approved these changes
Sep 5, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If, while loading a bridging header, we pick up a Clang module that claims to have an overlay Swift module, and that Swift module turns out to have a bridging header, we can end up reallocating the array of modules to process while we're looping over it. Be defensive against this occurrence.
This just fixes a crash; it does not at all solve the problem of this being broken in several ways:
Accidentally naming your module the same as a system module shadows the latter (if the system module is a Swift module) or makes your module into an overlay (if the system module is a Clang module).
Bridging headers are only officially supported on executable targets and unit tests, but this isn't really enforced.
Implicit inclusion of a bridging header when you import a Swift module is a hack to begin with, and a hack that worsens when the main module also has a bridging header. (All the bridging headers get folded together into the "same" module, which leads to more visibility than desired as well as cycles in the import graph.)
Combining all of these can result in some pretty bizarre behavior.
rdar://problem/54581756