-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Enable strict verification of begin_access patterns in all SIL passes. #17534
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
AccessedStorage now properly represents access to global variables, even if they haven't been fully optimized down to global_addr instructions. This is essential for optimizing dynamic exclusivity checks. As a verified SIL property, all access to globals and class properties needs to be identifiable.
@swift-ci test. |
@swift-ci source compatibility test. |
@swift-ci test source compatibility. |
Build failed |
Ensure that all formal access follows recognizable patterns at all points in the SIL pipeline. This is important to run acccess enforcement optimization late in the pipeline.
@swift-ci test. |
Build failed |
@swift-ci test. |
@shajrawi, @devincoughlin, @rjmccall. These changes were reviewed in an earlier PR. The only thing that changed since then is that DiagnoseStaticExclusivity now ignores access markers with "unsafe" enforcement. Unsafe accesses are not required to be from identifiable addresses. e.g. in lldb we simply load from a RawPointer. That can't be allowed with enforced access. This works around failures caused by the original PR, which prevented DiagnoseStaticExclusivity from bailing out on unrecognized patterns. This pass, along with any optimistic exclusivity optimizations, must be sound. They cannot allow for any unknown or unrecognized access patterns. Otherwise future changes to SIL could weaken exclusivity with no way to catch the problem. |
This makes sense to me, but I don't understand how it's not causing major problems with addressors and materializeForSet, since we don't emit |
Okay. Do you want to take that and actually do the test updates? I just wrote it up to unblock work on my PR, which doesn't have a known timeline yet. |
I already stole your commit and disabled the SIL verifier check on trunk. So I'll be pushing your commit before reenabling the verification. |
This also adds handling additional cases like global addressors.