Skip to content

Teach findAccessedStorage about global addressors. #16877

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

Closed
wants to merge 4 commits into from
Closed

Teach findAccessedStorage about global addressors. #16877

wants to merge 4 commits into from

Conversation

atrick
Copy link
Contributor

@atrick atrick commented May 29, 2018

All but the last commit are covered by: #16870.

The only new change here is to enable strict verification of formal access on global variables and add unit tests.

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.

@atrick
Copy link
Contributor Author

atrick commented May 29, 2018

@shajrawi please look at just the last commit in this PR: d72c91a

@atrick
Copy link
Contributor Author

atrick commented May 29, 2018

@swift-ci test.

@atrick
Copy link
Contributor Author

atrick commented May 29, 2018

@swift-ci test source compatibility.

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d72c91a242e8ff53b8f78fff5d37c2fcc4fc4c07

@atrick
Copy link
Contributor Author

atrick commented May 29, 2018

@swift-ci test.

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d72c91a242e8ff53b8f78fff5d37c2fcc4fc4c07

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d72c91a242e8ff53b8f78fff5d37c2fcc4fc4c07

@shajrawi
Copy link

@atrick d72c91a LGTM!

@shajrawi
Copy link

re PR - I do see this problem on the OS X bots:

Assertion failed: (address->getType().is<SILBoxType>()), function findAccessedStorage, file /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/lib/SIL/MemAccessUtils.cpp, line 269.

atrick added 4 commits May 30, 2018 11:40
This information was getting lost in SIL printing/parsing.
Some passes rely on it. Regardless of whether passes should rely on it,
it is totally unacceptable for the SIL passes to have subtle differences
in behavior depending on the frontend mode. So, if we don't want passes
to rely on global variable decls, that needs to be enforced by the API
independent of how the frontend is invoked or how SIL is serialized.
Move the utilities that are required for recognizing SILGlobalVariable access
into SILGlobalVariable.[h|cpp].

Structural SIL properties that are assumed by the optimizer, and thus required
for SIL verification, should never be embedded in SILOptimizer passes, or even
in SILOptimizer/Utils. Structural SIL properties need to be defined in
/SIL. They are as much part of the SIL language as the opcode list.

These particular utilities are required for working with SILGlobalVariables, and
will be used by a whole-module access enforcement optimization.

The primary API for recognizing SIL globals is `getVariableOfGlobalInit`. It is
required to find the association between the addressor SILFunction marked
[global_init], and the SILGlobalVariable being addressed.

Other helper APIs expose more details about the addressor's SIL patterns and are
useful for transforming the initializer itself into an optimized form.
Add stronger SILVerifier support for formal access.

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.

To make verification reliable, teach findAccessedStorage about global
addressors. 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.
@atrick atrick closed this Jun 20, 2018
@atrick atrick deleted the accessglobal branch June 20, 2018 00:07
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.

3 participants