Skip to content

[ASAN] Removed special case controlling allocator consts for __aarch64 #6294

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

Conversation

wrotki
Copy link

@wrotki wrotki commented Feb 16, 2023

This patch should land before D137136 to make sure that the leak sanitizer allocator works correctly. This patch is NFC without D137136.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D137265

(cherry picked from commit a8604f2)

…ch64__.

This patch should land before D137136 to make sure that the leak sanitizer allocator works correctly. This patch is NFC without D137136.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D137265

(cherry picked from commit a8604f2)
@wrotki wrotki requested review from yln and kubamracek February 16, 2023 00:50
@yln
Copy link

yln commented Feb 16, 2023

My understanding of the issue is: there was an open source change consisting of multiple commits. The Swift stable branch “branched off” in between: we have some but not all of these commits that should land “as a unit”.

I would prefer to revert the other commits of this set of commits, instead of cherry-picking this one because of what you said:

However, picking this up would just cause the allocator to start at different fixed area (0x600000000000ULL per asan_allocator.h), which happens to fall into HighMemory and hence works, although I feel it’s somewhat accidental, and might fail again on various devices.

@wrotki
Copy link
Author

wrotki commented Feb 21, 2023

Decided to go with solution proposed by yln (reverting), therefore closing this without merging

@wrotki wrotki closed this Feb 21, 2023
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