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

Rename GetMacosVersion() -> GetMacosAlignedVersion() to better reflect
how this is used on non-MacOS platforms.

[delcypher]

LGTM, minus the problems in previous patches.

[delcypher]

Looks good apart from the missing check for underflow. This patch is of course broken for the simulators but it was before this patch and I know in future patches that you plan to fix this.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

610–615

Maybe should have a check to make sure we don't perform unsigned underflow..

Maybe something like this?

auto kernel_version = GetDarwinKernelVersion();
const u16 version_offset = 4;
CHECK_GE(kernel_version.major, version_offset);
return MacosVersion(10, kernel_verson.major - version_offset);

[yln]

Add version underflow check.

[delcypher]

LGTM other than the use of uint32_t.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

618–628

This should probably be u32 (sanitizer internal) rather than uint32_t which comes from a C library header.

Similarly in GetMacosAlignedVersion() too.

[yln]

Use atomic_uint32_t::Type instead of uint32_t.

[yln]

*ping* @delcypher

[delcypher]

I have an optional suggestion to improve this patch. Other than that LGTM.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

630

Suggestion: I'm not a big fan of all the reinterpret_cast<>s which assume there's no alignment padding in the MacosVersion struct. One way around this is to add methods to MacosVersion that allow for serializing/deserializing from a u32.

template <typename VersionType>
struct VersionBase {
  // ...
  static VersionBase<VersionType> FromU32(u32 repr) {
     u16 major =  repr & 0xff;
     u16 minor = (repr >> 16) & 0xff;
     return VersionBase<VersionType>(major, minor);
  }

  u32 ToU32() {
    u32 repr = static_cast<u32>(major) + (static_cast<u32>(minor) >> 16);
    CHECK_EQ(VersionBase<VersionType>::FromU32(repr), *this);
    return repr;
  }
};

then you can have

MacosVersion GetMacosAlignedVersion() {
  atomic_uint32_t::Type result =
      atomic_load(&cached_macos_version, memory_order_acquire);
  if (!result) {
    MacosVersion version = GetMacosAlignedVersionInternal();
    result = version::ToU32();
    atomic_store(&cached_macos_version, result, memory_order_release);
  }
  return MacosVersion::FromU32(result);
}

However, you have a nice static_assert which will catch if this assumption about alignment breaks so I guess this is okay. So please consider the above a non-blocking suggestion. If you like it you can adopt it but feel free to land this patch as it is.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

630

Yes, I feel that reinterpret_cast is always a wart. I will consider a follow-up (since it was already there in the old code). Best would be std::atomic<MacosVersion> but we can’t since its compiler-rt…

What do you think about removing the caching altogether? I am pretty sure it’s a case of premature optimization: I looked at all the call sites and none of them is in a loop; the function is used in initialization, or when printing an error/walking the stack. One use is in the setjmp/longjmp interceptor.

Another option would be to remove the atomic part from the caching. 1) It’s first queried during initialization, which is single-threaded and even if we would race (due to code changes) in the future; the query really should always return the same result.

compiler-rt/lib/sanitizer_common/sanitizer_mac.h

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_LION,  // macOS 10.7; oldest currently supported
-  MACOS_VERSION_MOUNTAIN_LION,
-  MACOS_VERSION_MAVERICKS,
-  MACOS_VERSION_YOSEMITE,
-  MACOS_VERSION_EL_CAPITAN,
-  MACOS_VERSION_SIERRA,
-  MACOS_VERSION_HIGH_SIERRA,
-  MACOS_VERSION_MOJAVE,
-  MACOS_VERSION_CATALINA,
-  MACOS_VERSION_UNKNOWN_NEWER
-};
-
-struct DarwinKernelVersion {
+template <typename VersionType>
+struct VersionBase {
   u16 major;
   u16 minor;
 
-  DarwinKernelVersion(u16 major, u16 minor) : major(major), minor(minor) {}
+  VersionBase(u16 major, u16 minor) : major(major), minor(minor) {}
 
-  bool operator==(const DarwinKernelVersion &other) const {
+  bool operator==(const VersionType &other) const {
     return major == other.major && minor == other.minor;
   }
-  bool operator>=(const DarwinKernelVersion &other) const {
+  bool operator>=(const VersionType &other) const {
     return major >= other.major ||
            (major == other.major && minor >= other.minor);
   }
 };
 
-MacosVersion GetMacosVersion();
+struct MacosVersion : VersionBase<MacosVersion> {
+  MacosVersion(u16 ten, u16 major) : VersionBase(ten, major) {
+    CHECK_EQ(ten, 10);
+  }
+};
+
+struct DarwinKernelVersion : VersionBase<DarwinKernelVersion> {
+  DarwinKernelVersion(u16 major, u16 minor) : VersionBase(major, minor) {}
+};
+
+MacosVersion GetMacosAlignedVersion();
 DarwinKernelVersion GetDarwinKernelVersion();
 
 char **GetEnviron();
 
 void RestrictMemoryToMaxAddress(uptr max_address);
 
 }  // namespace __sanitizer
 

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

 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 ((GetMacosAlignedVersion() >= 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() {
-  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>)
-  CHECK_GE(len, 3);
-  const char *p = version;
-  int major = internal_simple_strtoll(p, &p, /*base=*/10);
-  if (*p != '.') return MACOS_VERSION_UNKNOWN;
-  p += 1;
-  int minor = internal_simple_strtoll(p, &p, /*base=*/10);
-  if (*p != '.') return MACOS_VERSION_UNKNOWN;
-
-  switch (major) {
-    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: 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;
-  }
+static MacosVersion GetMacosAlignedVersionInternal() {
+  u16 kernel_major = GetDarwinKernelVersion().major;
+  const u16 version_offset = 4;
+  CHECK_GE(kernel_major, version_offset);
+  u16 macos_major = kernel_major - version_offset;
+  return MacosVersion(10, macos_major);
+}
+
+static_assert(sizeof(MacosVersion) == sizeof(atomic_uint32_t::Type),
+              "MacosVersion cache size");
+static atomic_uint32_t cached_macos_version;
+
+MacosVersion GetMacosAlignedVersion() {
+  atomic_uint32_t::Type result =
+      atomic_load(&cached_macos_version, memory_order_acquire);
+  if (!result) {
+    MacosVersion version = GetMacosAlignedVersionInternal();
+    result = *reinterpret_cast<atomic_uint32_t::Type *>(&version);
+    atomic_store(&cached_macos_version, result, memory_order_release);
   }
-
-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;
+  return *reinterpret_cast<MacosVersion *>(&result);
 }
 
 DarwinKernelVersion GetDarwinKernelVersion() {
   char buf[100];
   size_t len = sizeof(buf);
   int res = internal_sysctlbyname("kern.osrelease", buf, &len, nullptr, 0);
   CHECK_EQ(res, 0);
 
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 (GetMacosAlignedVersion() >= 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",

 
 bool DyldNeedsEnvVariable() {
   // Although sanitizer support was added to LLVM on OS X 10.7+, GCC users
   // still may want use them on older systems. On older Darwin platforms, dyld
   // doesn't export dyldVersionNumber symbol and we simply return true.
   if (!&dyldVersionNumber) return true;
   // If running on OS X 10.11+ or iOS 9.0+, dyld will interpose even if
   // DYLD_INSERT_LIBRARIES is not set. However, checking OS version via
-  // GetMacosVersion() doesn't work for the simulator. Let's instead check
+  // GetMacosAlignedVersion() doesn't work for the simulator. Let's instead check
   // `dyldVersionNumber`, which is exported by dyld, against a known version
   // number from the first OS release where this appeared.
   return dyldVersionNumber < kMinDyldVersionWithAutoInterposition;
 }
 
 void MaybeReexec() {
   // FIXME: This should really live in some "InitializePlatform" method.
   MonotonicNanoTime();

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

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 (GetMacosAlignedVersion() == 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

#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 (GetMacosAlignedVersion() >= 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)
+    ((GetMacosAlignedVersion() >= 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;
   sp = (uptr)ptrauth_auth_data((void *)sp, ptrauth_key_asdb,