The recent change (https://reviews.llvm.org/D137136) to unconditionally
choose the 64 bit allocator on aarch64 breaks Apple iOS family of devices
which purposely use a smaller address space than is used with macOS.
rdar://102527313
Differential D139030
[asan][aarch64] Don't use 64 bit allocator for Apple ios family rsundahl on Nov 30 2022, 10:07 AM. Authored by
Details The recent change (https://reviews.llvm.org/D137136) to unconditionally rdar://102527313
Diff Detail
Event TimelineComment Actions This looks fine to me. Although this comment: // - SANITIZER_IOS: devices (iOS and iOS-like) // - SANITIZER_WATCHOS // - SANITIZER_TVOS Seems to suggest that SANITIZER_IOS includes all IOS-like devices. And if the logic is that simple can we add it to the existing condition? Edit: or, actually, can we not just add SANITIZER_IOS to existing condition? Comment Actions I didn't want to depend on the overloading of SANITIZER_IOS which means both actual iOS and similar to iOS, so I chose to come at the logic the way I did.
Comment Actions I think we want to actually cover watchOS & tvOS which has the same (or even stricter) constraints...
Comment Actions The change as I originally made it was to use the 32 bit allocator in all of the cases that it had been used in up until things broke. I was conservative and explicit about enabling the 64 bit allocator for macOS because I tested it and it worked. The reviews here tell me that we want to go further but have questions so I'm going to revert to the original behavior of using the 32 bit allocator for SANITIZER_APPLE on aarch64. Comment Actions That's a good strategy, sorry for the distractions! Yes, let's revert to previous state. We can then make incremental improvements once we are back in a working state. |
Let's have a separate branch SANITIZER_APPLE to insulate a bit better from future changes.
I agree, I think we should list the platforms to opt-out to avoid the double negation.
I think there is some additional complications:
I am assuming that the using ALLOCATOR64 is better if we can. The default is "yes" for 64-bit platforms (SANITIZER_WORDSIZE == 64), which is the fall through if no other branch says "don't use". So let's specify all those cases; include DriverKit (just to make sure DriverKit on iOS is covered).