[asan][aarch64] Don't use 64 bit allocator for Apple ios family

[rsundahl]

Summary

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

[thetruestblue]

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.
Can we not do (SANITIZER_APPLE && SANITIZER_IOS) ?

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?

[rsundahl]

In D139030#3961123, @thetruestblue wrote:

Can we not do (SANITIZER_APPLE && SANITIZER_IOS) ?

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.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

289–290

// - SANITIZER_APPLE: all Apple code
//   - TARGET_OS_OSX: macOS
//   - SANITIZER_IOS: devices (iOS and iOS-like)
//     - SANITIZER_WATCHOS
//     - SANITIZER_TVOS
//   - SANITIZER_IOSSIM: simulators (iOS and iOS-like)
//   - SANITIZER_DRIVERKIT

In D139030#3961123, @thetruestblue wrote:

And if the logic is that simple can we add it to the existing condition?

Let's have a separate branch SANITIZER_APPLE to insulate a bit better from future changes.

Edit: or, actually, can we not just add SANITIZER_IOS to existing condition?

I agree, I think we should list the platforms to opt-out to avoid the double negation.

I think there is some additional complications:

• For DriverKit, we can't tell if we are compiling for iPhone or macOS.
• Did we consider the simulators?

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).

[yln]

In D139030#3961252, @rsundahl wrote:

In D139030#3961123, @thetruestblue wrote:

Can we not do (SANITIZER_APPLE && SANITIZER_IOS) ?

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.

I think we want to actually cover watchOS & tvOS which has the same (or even stricter) constraints...

[thetruestblue]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

289–290

+1 on this.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

289–290

[rsundahl]

Integrate suggestions from review.

[rsundahl]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

289–290

This will have the effect of unconditionally using the 32 bit allocator on Driverkit for x86_64 platforms.

[thetruestblue]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

289–290

Why do we need to specify DriverKit at all? Wouldn't the allocator be determined elsewhere in this logic? i.e. platform/wordsize?

[rsundahl]

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.

[rsundahl]

Revert to original use of 32 bit allocator in all SANITIZER_APPLE on aarch64

[yln]

In D139030#3961494, @rsundahl wrote:

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.

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.