[Darwin] Improve runtime OS version checks

[yln]

Summary

Use a struct to represent numerical versions instead of encoding release
names in an enumeration. This avoids the need to extend the enumeration
every time there is a new release.

[delcypher]

LGTM except for a few minor nits.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

623

If we're going to use a CHECK here this should be CHECK_EQ so that we get a better error message if the check fails.

This is also a behaviour change. Are we happy with faulting here where previously we would just return MACOS_VERSION_UNKNOWN?

637

Nice cleanup!

compiler-rt/lib/sanitizer_common/sanitizer_mac.h

56

Nit: I'm wondering if we should rename GetMacosVersion() given then we use it on platforms that are not macOS? GetMacosAlignedVersion() would be a more accurate name. I know you didn't change this line but given you're having to touch all call sites now would be a good time to re-consider how this function is named.

[yln]

GetMacosVersion() -> GetMacosAlignedVersion(). Use CHECK_EQ.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

623

This is also a behaviour change. Are we happy with faulting here where previously we would just return MACOS_VERSION_UNKNOWN?

Yes, I think so. This asserts that the version string follows the expected format. Note that we already assert on the system call above CHECK_NE(internal_sysctl(...), -1);. Considering this my best guess is that MACOS_VERSION_UNKNOWN is an artifact of the old implementation via enum+switch: there should be a value for the default case.

[kubamracek]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

630

This makes it easy(-ier) to make a mistake if for some reason some new code tries to check for 10.13.6 - it won't work (this code claims all 10.13.4+ versions are 10.13.4).

[kubamracek]

compiler-rt/lib/sanitizer_common/sanitizer_mac.h

43

Drop this TODO. There's no "bug" in this code and I don't think we need to make notes for us to adopt new language feature we can't use today.

[kubamracek]

Overall LGTM - would be great to hear feedback from @eugenis though.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

630

Good catch! I am assumoing this special mapping should apply for kernel 17.5, as follows:

kernel -> macOS
17.4 -> 10.13.4
17.5 -> 10.13.4
17.6 -> 10.13.6

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

637

@yln Random thought. Could you check the code that clang generates here? I don't know how clang implements the locking here and I'd like to be sure it doesn't generate code that could be dangerous for us or introduce a new runtime dependency.

[yln]

   // Kernel version 17.5 maps to High Sierra 10.13.4.
-  if (major == 17 && minor >= 5)
+  if (major == 17 && minor == 5)

Drop TODO.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

637

C++ local statics use __cxa_guard_acquire/release/abort [1]. The implementation in the runtime has different locking strategies; currently a global mutex, in the future "futex"es. So there might be an interaction with TSan. There is a test in the Tsan test suite [1] (still passes for me).

[1] https://github.com/llvm/llvm-project/blob/c8ac29ab1d798f16999541cd9f08d58ebaa1b4c1/libcxxabi/src/cxa_guard_impl.h#L546
[2] https://github.com/llvm/llvm-project/blob/master/compiler-rt/test/tsan/cxa_guard_acquire.cpp#L1

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

637

Scrap it. I have decided that this change is not worth the risk (possible TSan interactions). I will revert this part to minimize the risk (and diff) of this patch. Thanks Dan for pointing this out!

[yln]

Avoid local static var due to potential interactions with TSan.

[delcypher]

LGTM except the issue with 10.13.5.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

630

@yln

kernel -> macOS
17.4 -> 10.13.4
17.5 -> 10.13.4
17.6 -> 10.13.6

This mapping sounds odd (mapping two different kernel versions to the same macOS version). Can you verify this is correct? It looks like GetMacosAlignedVersion() cannot return MacosVersion(10, 13, 5) even if the host OS was 10.13.5. That doesn't sound right.

[yln]

Superseded by https://reviews.llvm.org/D79970

compiler-rt/lib/sanitizer_common/sanitizer_mac.h

 //===-- sanitizer_mac.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
struct MemoryMappingLayoutData {
   u32 current_filetype;
   ModuleArch current_arch;
   u8 current_uuid[kModuleUUIDSize];
   int current_load_cmd_count;
   const char *current_load_cmd_addr;
   bool current_instrumented;
 };
 
-enum MacosVersion {
-  MACOS_VERSION_UNINITIALIZED = 0,
-  MACOS_VERSION_UNKNOWN,
-  MACOS_VERSION_LEOPARD,
-  MACOS_VERSION_SNOW_LEOPARD,
-  MACOS_VERSION_LION,
-  MACOS_VERSION_MOUNTAIN_LION,
-  MACOS_VERSION_MAVERICKS,
-  MACOS_VERSION_YOSEMITE,
-  MACOS_VERSION_EL_CAPITAN,
-  MACOS_VERSION_SIERRA,
-  MACOS_VERSION_HIGH_SIERRA,
-  MACOS_VERSION_HIGH_SIERRA_DOT_RELEASE_4,
-  MACOS_VERSION_MOJAVE,
-  MACOS_VERSION_CATALINA,
-  MACOS_VERSION_UNKNOWN_NEWER
+struct MacosVersion {
+  u16 major;
+  u16 minor;
+
+  MacosVersion(u16 ten, u16 major, u16 minor = 0) {
+    CHECK_EQ(ten, 10);
+    this->major = major;
+    this->minor = minor;
+  }
+
+  // TODO(c++20): use compiler-provided comparison operators:
+  // bool operator==(const MacosVersion &) const = default;
+  // auto operator<=>(const MacosVersion &) const = default;
+
+  bool operator==(const MacosVersion &other) const {
+    return major == other.major && minor == other.minor;
+  }
+  bool operator>=(const MacosVersion &other) const {
+    return major >= other.major ||
+           (major == other.major && minor >= other.minor);
+  }
 };
 
 MacosVersion GetMacosVersion();
 
 char **GetEnviron();
 
 void RestrictMemoryToMaxAddress(uptr max_address);
 
 }  // namespace __sanitizer
 
 extern "C" {

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

 //===-- sanitizer_mac.cpp -------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom) {
   CHECK(stack_top);
   CHECK(stack_bottom);
   uptr stacksize = pthread_get_stacksize_np(pthread_self());
   // pthread_get_stacksize_np() returns an incorrect stack size for the main
   // thread on Mavericks. See
   // https://github.com/google/sanitizers/issues/261
-  if ((GetMacosVersion() >= MACOS_VERSION_MAVERICKS) && at_initialization &&
+  if ((GetMacosVersion() >= MacosVersion(10, 9)) && at_initialization &&
       stacksize == (1 << 19))  {
     struct rlimit rl;
     CHECK_EQ(getrlimit(RLIMIT_STACK, &rl), 0);
     // Most often rl.rlim_cur will be the desired 8M.
     if (rl.rlim_cur < kMaxThreadStackSize) {
       stacksize = rl.rlim_cur;
     } else {
       stacksize = kMaxThreadStackSize;
     }
HandleSignalMode GetHandleSignalMode(int signum) {
   if ((SANITIZER_WATCHOS || SANITIZER_TVOS) && !(SANITIZER_IOSSIM))
     return kHandleSignalNo;
   HandleSignalMode result = GetHandleSignalModeImpl(signum);
   if (result == kHandleSignalYes && !common_flags()->allow_user_segv_handler)
     return kHandleSignalExclusive;
   return result;
 }
 
-MacosVersion cached_macos_version = MACOS_VERSION_UNINITIALIZED;
-
-MacosVersion GetMacosVersionInternal() {
+static MacosVersion GetMacosVersionInternal() {
   int mib[2] = { CTL_KERN, KERN_OSRELEASE };
   char version[100];
   uptr len = 0, maxlen = sizeof(version) / sizeof(version[0]);
   for (uptr i = 0; i < maxlen; i++) version[i] = '\0';
   // Get the version length.
   CHECK_NE(internal_sysctl(mib, 2, 0, &len, 0, 0), -1);
   CHECK_LT(len, maxlen);
   CHECK_NE(internal_sysctl(mib, 2, version, &len, 0, 0), -1);
 
-  // Expect <major>.<minor>(.<patch>)
+  // Expect <major>.<minor>.<patch>
   CHECK_GE(len, 3);
   const char *p = version;
   int major = internal_simple_strtoll(p, &p, /*base=*/10);
-  if (*p != '.') return MACOS_VERSION_UNKNOWN;
+  CHECK(*p == '.');
   p += 1;
   int minor = internal_simple_strtoll(p, &p, /*base=*/10);
-  if (*p != '.') return MACOS_VERSION_UNKNOWN;
+  CHECK(*p == '.');
 
-  switch (major) {
-    case 9: return MACOS_VERSION_LEOPARD;
-    case 10: return MACOS_VERSION_SNOW_LEOPARD;
-    case 11: return MACOS_VERSION_LION;
-    case 12: return MACOS_VERSION_MOUNTAIN_LION;
-    case 13: return MACOS_VERSION_MAVERICKS;
-    case 14: return MACOS_VERSION_YOSEMITE;
-    case 15: return MACOS_VERSION_EL_CAPITAN;
-    case 16: return MACOS_VERSION_SIERRA;
-    case 17:
-      // Not a typo, 17.5 Darwin Kernel Version maps to High Sierra 10.13.4.
-      if (minor >= 5)
-        return MACOS_VERSION_HIGH_SIERRA_DOT_RELEASE_4;
-      return MACOS_VERSION_HIGH_SIERRA;
-    case 18: return MACOS_VERSION_MOJAVE;
-    case 19: return MACOS_VERSION_CATALINA;
-    default:
-      if (major < 9) return MACOS_VERSION_UNKNOWN;
-      return MACOS_VERSION_UNKNOWN_NEWER;
-  }
+  // Kernel version 17.5 maps to High Sierra 10.13.4.
+  if (major == 17 && minor >= 5)
+    return MacosVersion(10, 13, 4);
+
+  int macos_major = major - 4;
+  return MacosVersion(10, macos_major, minor);
 }
 
 MacosVersion GetMacosVersion() {
-  atomic_uint32_t *cache =
-      reinterpret_cast<atomic_uint32_t*>(&cached_macos_version);
-  MacosVersion result =
-      static_cast<MacosVersion>(atomic_load(cache, memory_order_acquire));
-  if (result == MACOS_VERSION_UNINITIALIZED) {
-    result = GetMacosVersionInternal();
-    atomic_store(cache, result, memory_order_release);
-  }
-  return result;
+  static MacosVersion cache = GetMacosVersionInternal();
+  return cache;
 }
 
 bool PlatformHasDifferentMemcpyAndMemmove() {
   // On OS X 10.7 memcpy() and memmove() are both resolved
   // into memmove$VARIANT$sse42.
   // See also https://github.com/google/sanitizers/issues/34.
   // TODO(glider): need to check dynamically that memcpy() and memmove() are
   // actually the same function.
-  return GetMacosVersion() == MACOS_VERSION_SNOW_LEOPARD;
+  return GetMacosVersion() == MacosVersion(10, 6);
 }
 
 uptr GetRSS() {
   struct task_basic_info info;
   unsigned count = TASK_BASIC_INFO_COUNT;
   kern_return_t result =
       task_info(mach_task_self(), TASK_BASIC_INFO, (task_info_t)&info, &count);
   if (UNLIKELY(result != KERN_SUCCESS)) {
void LogMessageOnPrintf(const char *str) {
   if (common_flags()->abort_on_error)
     CRAppendCrashLogMessage(str);
 }
 
 void LogFullErrorReport(const char *buffer) {
 #if !SANITIZER_GO
   // Log with os_trace. This will make it into the crash log.
 #if SANITIZER_OS_TRACE
-  if (GetMacosVersion() >= MACOS_VERSION_YOSEMITE) {
+  if (GetMacosVersion() >= MacosVersion(10, 10)) {
     // os_trace requires the message (format parameter) to be a string literal.
     if (internal_strncmp(SanitizerToolName, "AddressSanitizer",
                          sizeof("AddressSanitizer") - 1) == 0)
       os_trace("Address Sanitizer reported a failure.");
     else if (internal_strncmp(SanitizerToolName, "UndefinedBehaviorSanitizer",
                               sizeof("UndefinedBehaviorSanitizer") - 1) == 0)
       os_trace("Undefined Behavior Sanitizer reported a failure.");
     else if (internal_strncmp(SanitizerToolName, "ThreadSanitizer",

 }
 
 void SignalContext::InitPcSpBp() { GetPcSpBp(context, &pc, &sp, &bp); }
 
 void InitializePlatformEarly() {
   // Only use xnu_fast_mmap when on x86_64 and the OS supports it.
   use_xnu_fast_mmap =
 #if defined(__x86_64__)
-      GetMacosVersion() >= MACOS_VERSION_HIGH_SIERRA_DOT_RELEASE_4;
+      GetMacosVersion() >= MacosVersion(10, 13, 4);
 #else
       false;
 #endif
 }
 
 #if !SANITIZER_GO
 static const char kDyldInsertLibraries[] = "DYLD_INSERT_LIBRARIES";
 LowLevelAllocator allocator_for_env;

compiler-rt/lib/sanitizer_common/sanitizer_malloc_mac.inc

INTERCEPTOR(malloc_zone_t *, malloc_create_zone,
                              vm_size_t start_size, unsigned zone_flags) {
   COMMON_MALLOC_ENTER();
   uptr page_size = GetPageSizeCached();
   uptr allocated_size = RoundUpTo(sizeof(sanitizer_zone), page_size);
   COMMON_MALLOC_MEMALIGN(page_size, allocated_size);
   malloc_zone_t *new_zone = (malloc_zone_t *)p;
   internal_memcpy(new_zone, &sanitizer_zone, sizeof(sanitizer_zone));
   new_zone->zone_name = NULL;  // The name will be changed anyway.
-  if (GetMacosVersion() >= MACOS_VERSION_LION) {
+  if (GetMacosVersion() >= MacosVersion(10, 7)) {
     // Prevent the client app from overwriting the zone contents.
     // Library functions that need to modify the zone will set PROT_WRITE on it.
     // This matches the behavior of malloc_create_zone() on OSX 10.7 and higher.
     mprotect(new_zone, allocated_size, PROT_READ);
   }
   // We're explicitly *NOT* registering the zone.
   return new_zone;
 }
 
 INTERCEPTOR(void, malloc_destroy_zone, malloc_zone_t *zone) {
   COMMON_MALLOC_ENTER();
   // We don't need to do anything here.  We're not registering new zones, so we
   // don't to unregister.  Just un-mprotect and free() the zone.
-  if (GetMacosVersion() >= MACOS_VERSION_LION) {
+  if (GetMacosVersion() >= MacosVersion(10, 7)) {
     uptr page_size = GetPageSizeCached();
     uptr allocated_size = RoundUpTo(sizeof(sanitizer_zone), page_size);
     mprotect(zone, allocated_size, PROT_READ | PROT_WRITE);
   }
   if (zone->zone_name) {
     COMMON_MALLOC_FREE((void *)zone->zone_name);
   }
   COMMON_MALLOC_FREE(zone);

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

 //===-- sanitizer_symbolizer_mac.cpp --------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
private:
   }
 
   void GetArgV(const char *path_to_binary,
                const char *(&argv)[kArgVMax]) const override {
     int i = 0;
     argv[i++] = path_to_binary;
     argv[i++] = "-p";
     argv[i++] = &pid_str_[0];
-    if (GetMacosVersion() == MACOS_VERSION_MAVERICKS) {
+    if (GetMacosVersion() == MacosVersion(10, 9)) {
       // On Mavericks atos prints a deprecation warning which we suppress by
       // passing -d. The warning isn't present on other OSX versions, even the
       // newer ones.
       argv[i++] = "-d";
     }
     argv[i++] = nullptr;
   }
 

compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp

 //===-- tsan_platform_mac.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
#if !SANITIZER_GO
 
   CHECK_EQ(main_thread_identity, 0);
   main_thread_identity = (uptr)pthread_self();
 
   prev_pthread_introspection_hook =
       pthread_introspection_hook_install(&my_pthread_introspection_hook);
 #endif
 
-  if (GetMacosVersion() >= MACOS_VERSION_MOJAVE) {
+  if (GetMacosVersion() >= MacosVersion(10, 14)) {
     // Libsystem currently uses a process-global key; this might change.
     const unsigned kTLSLongjmpXorKeySlot = 0x7;
     longjmp_xor_key = (uptr)pthread_getspecific(kTLSLongjmpXorKeySlot);
   }
 }
 
 #ifdef __aarch64__
 # define LONG_JMP_SP_ENV_SLOT \
-    ((GetMacosVersion() >= MACOS_VERSION_MOJAVE) ? 12 : 13)
+    ((GetMacosVersion() >= MacosVersion(10, 14)) ? 12 : 13)
 #else
 # define LONG_JMP_SP_ENV_SLOT 2
 #endif
 
 uptr ExtractLongJmpSp(uptr *env) {
   uptr mangled_sp = env[LONG_JMP_SP_ENV_SLOT];
   uptr sp = mangled_sp ^ longjmp_xor_key;
 #if __has_feature(ptrauth_calls)