Skip to content

SIL: fix bugs in the MemBehavior cache invalidation mechanism. #35263

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 1 commit into from
Jan 7, 2021

Conversation

eeckstein
Copy link
Contributor

When a non-value instruction (e.g. a destroy_addr) was deleted, the corresponding cache entry in MemoryBehaviorCache was not invalidated.
If a new instruction was allocated at the same memory location, the old - and invalid - cache entry was re-used.

This bug triggered a SIL memory lifetime failure in TempRValueElimination.

https://bugs.swift.org/browse/SR-13985
rdar://problem/72614608

This change also fixes another problem (which I found by inspection): Individual result values of MultipleValueInstructions were not invalidated correctly.

Thanks to @compnerd for reducing the test case.

@eeckstein eeckstein requested a review from atrick January 5, 2021 15:35
@eeckstein
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented Jan 5, 2021

Build failed
Swift Test OS X Platform
Git Sha - a273aeda5fcba173429286f7f1ddeb8c7ad514e7

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Thanks!

In the test file, it's not clear how the SIL exposes a corner case. There's no check to indicate what optimization temp-rvalue needs to do to expose the issue.

When a non-value instruction (e.g. a destroy_addr) was deleted, the corresponding cache entry in MemoryBehaviorCache was not invalidated.
If a new instruction was allocated at the same memory location, the old - and invalid - cache entry was re-used.

This bug triggered a SIL memory lifetime failure in TempRValueElimination.

https://bugs.swift.org/browse/SR-13985
rdar://problem/72614608

This change also fixes another problem (which I found by inspection): Individual result values of MultipleValueInstructions were not invalidated correctly.
@eeckstein
Copy link
Contributor Author

@swift-ci test macOS

@swift-ci
Copy link
Contributor

swift-ci commented Jan 6, 2021

Build failed
Swift Test OS X Platform
Git Sha - a273aeda5fcba173429286f7f1ddeb8c7ad514e7

@eeckstein eeckstein force-pushed the fix-mb-cache-invalidation branch from a273aed to f7296ed Compare January 6, 2021 14:22
@eeckstein
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented Jan 6, 2021

Build failed
Swift Test OS X Platform
Git Sha - f7296ed

@eeckstein
Copy link
Contributor Author

@swift-ci test macOS

@swift-ci
Copy link
Contributor

swift-ci commented Jan 6, 2021

Build failed
Swift Test OS X Platform
Git Sha - f7296ed

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test macOS

@eeckstein
Copy link
Contributor Author

@swift-ci test macOS

@eeckstein eeckstein merged commit e45dd64 into swiftlang:main Jan 7, 2021
@eeckstein eeckstein deleted the fix-mb-cache-invalidation branch January 7, 2021 07:00
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