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

Disable these tests under ASAN #75637

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

tbkka
Copy link
Contributor

@tbkka tbkka commented Aug 1, 2024

RemoteMirror Reflection tests are incompatible with ASAN. I accidentally checked in a couple of new tests that allowed ASAN.

Resolves rdar://133012386

@tbkka tbkka requested a review from slavapestov as a code owner August 1, 2024 23:37
@tbkka
Copy link
Contributor Author

tbkka commented Aug 1, 2024

@swift-ci Please test

@tbkka tbkka enabled auto-merge August 1, 2024 23:37
@slavapestov
Copy link
Contributor

Do we know why they don't work under ASAN?

@tbkka
Copy link
Contributor Author

tbkka commented Aug 2, 2024

@swift-ci Please test Linux Platform

@tbkka tbkka merged commit 68deec2 into swiftlang:main Aug 2, 2024
5 checks passed
@tbkka
Copy link
Contributor Author

tbkka commented Aug 5, 2024

Do we know why they don't work under ASAN?

Yes. The test process requests large blocks of raw memory and then feeds bits of those into RemoteMirror's type and value analyses. Those blocks can span allocated & deallocated blocks in the heap, which ASAN does not like at all.

An earlier version of the test process only requested the exact minimum number of bytes needed for any specific RemoteMirror request; these never include deallocated memory, so were ASAN-safe. But the new version is more than 50 times faster (when built in debug), so I think it's worth giving up ASAN validation.

@tbkka
Copy link
Contributor Author

tbkka commented Aug 5, 2024

Here are details on the changes to the test harness: #75415

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.

2 participants