Skip to content
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

Fix the IR attributes of swift_getObjectType. #13390

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

eeckstein
Copy link
Member

It's not readnone, because it reads the metatype from an object.
Readnone would let the llvm ARC optimizer reschedule the call with a release-call for the object.

fixes SR-6560.
rdar://problem/35998785

It's not readnone, because it reads the metatype from an object.
Readnone would let the llvm ARC optimizer reschedule the call with a release-call for the object.

fixes SR-6560.
@eeckstein
Copy link
Member Author

@swift-ci test

Copy link
Member

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

Your reasoning seems sound. Essentially, anything that depends on object liveness can be thought of as reading the reference count of those objects.

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 70aeb71

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 70aeb71

@eeckstein
Copy link
Member Author

@swift-ci test

@jrose-apple
Copy link
Contributor

If only we had a notion of "this can be done earlier, but not later".

@eeckstein
Copy link
Member Author

@swift-ci nominate

• Explanation: Fixes a use-after-free issue related to class types
• Scope of Issue: may show up if dealing with class types
• Origination: it's an old bug
• Risk: Low
• Testing: CI bots

@eeckstein eeckstein merged commit 82e08e0 into apple:swift-4.1-branch Dec 13, 2017
@eeckstein eeckstein deleted the fix-attrs-4.1 branch December 13, 2017 02:59
@atrick
Copy link
Member

atrick commented Dec 13, 2017

I'm not opposed to the conservative fix, but it does indicate a problem or deficiency in the LLVM ARC pass. Reference counts are part of the "runtime memory", so LLVM doesn't see them. Any pass that reorders runtime calls needs a different interpretation of the memory effects.

@eeckstein
Copy link
Member Author

Any LLVM pass could reorder this operations if the function is marked as readnone.
Memory effects of ref count operations are visible in LLVM.

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.

None yet

5 participants