[compiler-rt] assert max virtual address is <= mmap range size

[aralisza]

Summary

If these sizes do not match, asan will not work as expected.

If possible, assert at compile time that the vm size is less than or equal to mmap range.
If a compile time assert is not possible, check at run time (for iOS)

rdar://76477969

[aralisza]

static

[aralisza]

rebase

[aralisza]

remove trailing whitesepace

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1187–1189

Needs braces or CHECK_LE moved before if.

[aralisza]

fix bug

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1188–1189

There's an inconsistency here. The static checks are checking with -1 already subtracted. The runtime check here is testing without -1 subtracted.

I think we should be checking whatever we are going to return at runtime.

1193

Why do we we need the static keyword here?

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1188–1189

I think we should be checking whatever we are going to return at runtime.

+1

While I usually really like the idea of compile-time checks (always better to catch mistakes earlier rather than later), I don't think it's worth the complexity/visual noise in this specific case.

Having one check for the final runtime value seems the clearest expression of intent here.

[aralisza]

remove runtime check for ios

[aralisza]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1188–1189

moved the runtime check to separate diff per offline discussion. should I still remove the static checks? I think they still add value, especially if we're not going to add the runtime checks until a later time

[aralisza]

remove static keyword for max vm constants

[delcypher]

LGTM apart from the static keyword which "I think" shouldn't be necessary here.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1188–1189

I'm fine with the static checks here. I think failing earlier at compile time when the constants are wrong is a good thing :)

[delcypher]

LGTM

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

static uptr GetTaskInfoMaxAddress() {
   __sanitizer_task_vm_info vm_info = {} /* zero initialize */;
   mach_msg_type_number_t count = __SANITIZER_TASK_VM_INFO_COUNT;
   int err = task_info(mach_task_self(), TASK_VM_INFO, (int *)&vm_info, &count);
   return err ? 0 : vm_info.max_address;
 }
 
 uptr GetMaxUserVirtualAddress() {
   static uptr max_vm = GetTaskInfoMaxAddress();
-  if (max_vm != 0)
+  if (max_vm != 0) {
     return max_vm - 1;
+  }
 
   // xnu cannot provide vm address limit
 # if SANITIZER_WORDSIZE == 32
-  return 0xffe00000 - 1;
+  constexpr uptr fallback_max_vm = 0xffe00000 - 1;
 # else
-  return 0x200000000 - 1;
+  constexpr uptr fallback_max_vm = 0x200000000 - 1;
 # endif
+  static_assert(fallback_max_vm <= SANITIZER_MMAP_RANGE_SIZE,
+                "Max virtual address must be less than mmap range size.");
+  return fallback_max_vm;
 }
 
 #else // !SANITIZER_IOS
 
 uptr GetMaxUserVirtualAddress() {
 # if SANITIZER_WORDSIZE == 64
-  return (1ULL << 47) - 1;  // 0x00007fffffffffffUL;
+  constexpr uptr max_vm = (1ULL << 47) - 1;  // 0x00007fffffffffffUL;
 # else // SANITIZER_WORDSIZE == 32
   static_assert(SANITIZER_WORDSIZE == 32, "Wrong wordsize");
-  return (1ULL << 32) - 1;  // 0xffffffff;
+  constexpr uptr max_vm = (1ULL << 32) - 1;  // 0xffffffff;
 # endif
+  static_assert(max_vm <= SANITIZER_MMAP_RANGE_SIZE,
+                "Max virtual address must be less than mmap range size.");
+  return max_vm;
 }
 #endif
 
 uptr GetMaxVirtualAddress() {
   return GetMaxUserVirtualAddress();
 }
 
 uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,