Skip to content

[Exclusivity] Enforce exclusive access for class offsets in KeyPaths #16051

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
Apr 20, 2018
Merged

[Exclusivity] Enforce exclusive access for class offsets in KeyPaths #16051

merged 4 commits into from
Apr 20, 2018

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Apr 20, 2018

Add dynamic enforcement of exclusive access when a KeyPath directly accesses a final
stored property on an instance of a class. For read-only projections, this begins and ends
the access immediately. For mutable projections, this uses the ClassHolder to perform
a long-term access that lasts as long as the lifetime of the ClassHolder.

rdar://problem/31972680

Follow up on: @devincoughlin #15502
and: @gottesmm #16016

@atrick
Copy link
Contributor Author

atrick commented Apr 20, 2018

@swift-ci test.

Copy link
Contributor

@gottesmm gottesmm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 655e57adcd7797871d69604d69bdb188143aee14

gottesmm and others added 4 commits April 19, 2018 21:37
…clusivity".

This is a special @_semantics attribute that preserves exclusivity even if we
are eliminating exclusivity in other parts of a module in release mode. This is
done by:

1. Teaching the Access Marker Elimination pass to skip any function with the
semantics tag.

2. Requiring all functions with the semantics tag to be noinline. This ensures
that the SIL level inliner will not inline these functions into any callers
without the protection of the semantics tag. This is enforced in IRGenPrepare
and ensures that our access markers will live to IRGen time.

3. In IRGenPrepare, we convert these functions from noinline to always
inline. After IRGen this then allows for the LLVM inliner to inline these
trivial functions that just perform the exclusivity checks ensuring that we do
not have extra calls in the fast path.

This ensures that we can fix the keypaths exclusivity issue without having to
enable exclusivity across the entire stdlib and deal with any of the potential
performance issues therein.

rdar://39335800
…ift_dumpTrackedAccesses().

This makes it easy to perform printf debugging in the debugger. I have found it
to be useful in understanding programs around exclusivity quickly.
Add dynamic enforcement of exclusive access when a KeyPath directly accesses a final
stored property on an instance of a class. For read-only projections, this begins and ends
the access immediately. For mutable projections, this uses the ClassHolder to perform
a long-term access that lasts as long as the lifetime of the ClassHolder.

rdar://problem/31972680
Use begin_unpaired_access [no_nested_conflict] for
Builtin.performInstantaneousReadAccess. This can't be optimized away
and is the proper marker to use when the access scope is unknown.

Drop the requirement that
_semantics("optimize.sil.preserve_exclusivity") be @inline(never). We
actually want theses inlined into user code. Verify that the
@_semantic functions are not inlined or otherwise tampered with prior
to serialization.

Make *no* change to propagate @inline(__always) into LLVM. This no longer has
any relationship to this PR and can be investigated seperately.
@atrick
Copy link
Contributor Author

atrick commented Apr 20, 2018

@swift-ci test.

1 similar comment
@atrick
Copy link
Contributor Author

atrick commented Apr 20, 2018

@swift-ci test.

@atrick
Copy link
Contributor Author

atrick commented Apr 20, 2018

@swift-ci test source compatibility.

@atrick
Copy link
Contributor Author

atrick commented Apr 20, 2018

@swift-ci test source compatibility

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.

4 participants