[sanitizer][Darwin] GetMacosAlignedVersion() fails if sysctl is not setup

[yln]

Summary

GetMacosAlignedVersion() fails for ASan-ified launchd because the
sanitizer initialization code runs before sysctl has been setup by
launchd. In this situation, sysctl kern.osproductversion returns a
non-empty string that does not match our expectations of a
well-formatted version string.

Retrieving the kernel version (via sysctl kern.osrelease) still works,
so we can use it to add a fallback for this corner case.

[kubamracek]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

658

strlcpy instead?

[kubamracek]

Can you add a comment to the header of the public function that eventually surfaced the "99.0" version (I guess that's GetMacosAlignedVersionInternal)? Something like "might return version 99 in rare cases".

With that, this looks reasonable.

[yln]

In D94190#2485887, @kubamracek wrote:

Can you add a comment to the header of the public function that eventually surfaced the "99.0" version (I guess that's GetMacosAlignedVersionInternal)? Something like "might return version 99 in rare cases".

With that, this looks reasonable.

Thanks! Alex and I will do one more test (Dan also suggested this): falling back to the kernel version sysctl kern.osrelease. In case this works, we can avoid returning a bogus value.

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

653

Maybe we should do the internal_strlen(vers) < 3 check just after the call to internal_sysctlbyname. Then if it fails set res to 1 so that we then take the fallback path that relies on the kernel version.

Although, we'd have to adapt the fallback path to support macOS 11... which makes things complicated.

[yln]

Fallback to kernel version instead of returning bogus version. Add assert for launchd case getpid() == 1.

[kubamracek]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

663

The way I'm reading this, it seems that if no_os_version is true, then vers is likely going to be an empty string too, and we'll set launchd to true (wrongly), and then fail this CHECK_EQ assert. What am I missing?

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

653

Turns out sysctl kern.osversion (kernel version) works even in launchd and this is possible!

660

You read this correctly, for launchd we have:

• res == 0 "no error"
• len != strlen(vers), I would consider the syscall buggy in this case. It should either return an error, or at least set the out parameters into a consistent state, e.g., len == 0 && vers == "\0".

663

Assert that we only ever encounter this crazy case for launchd.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

663

Yes, of course... I am not allowed to move the CHECK_EQ here!

[yln]

Fix faulty assert.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

663

I carelessly moved the assert if (launchd) CHECK_EQ(internal_getpid(), 1); from the block below. Thanks for catching this Kuba!

It's now guarded with "no error".

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
 //
 //===----------------------------------------------------------------------===//
 //

 constexpr u16 GetOSMajorKernelOffset() {
   if (TARGET_OS_OSX) return 4;
   if (TARGET_OS_IOS || TARGET_OS_TV) return 6;
   if (TARGET_OS_WATCH) return 13;
 }
 
 using VersStr = char[64];
 
+// Approximate OS version via kernel version.
+static uptr GetOSVersionFallback(VersStr vers) {
+  u16 kernel_major = GetDarwinKernelVersion().major;
+  u16 offset = GetOSMajorKernelOffset();
+  CHECK_GE(kernel_major, offset);
+  u16 os_major = kernel_major - offset;
+
+  const char *format = "%d.0";
+  if (TARGET_OS_OSX) {
+    if (os_major >= 16) {  // macOS 11+
+      os_major -= 5;
+    } else {  // macOS 10.15 and below
+      format = "10.%d";
+    }
+  }
+  return internal_snprintf(vers, sizeof(VersStr), format, os_major);
+}
+
 static void GetOSVersion(VersStr vers) {
   uptr len = sizeof(VersStr);
   if (SANITIZER_IOSSIM) {
     const char *vers_env = GetEnv("SIMULATOR_RUNTIME_VERSION");
     if (!vers_env) {
       Report("ERROR: Running in simulator but SIMULATOR_RUNTIME_VERSION env "
           "var is not set.\n");
       Die();
     }
     len = internal_strlcpy(vers, vers_env, len);
   } else {
     int res =
         internal_sysctlbyname("kern.osproductversion", vers, &len, nullptr, 0);
-    if (res) {
-      // Fallback for XNU 17 (macOS 10.13) and below that do not provide the
-      // `kern.osproductversion` property.
-      u16 kernel_major = GetDarwinKernelVersion().major;
-      u16 offset = GetOSMajorKernelOffset();
-      CHECK_LE(kernel_major, 17);
-      CHECK_GE(kernel_major, offset);
-      u16 os_major = kernel_major - offset;
 
-      auto format = TARGET_OS_OSX ? "10.%d" : "%d.0";
-      len = internal_snprintf(vers, len, format, os_major);
+    // XNU 17 (macOS 10.13) and below do not provide the sysctl
+    // `kern.osproductversion` entry (res != 0).
+    bool no_os_version = res != 0;
+
+    // For launchd, sanitizer initialization runs before sysctl is setup
+    // (res == 0 && len != strlen(vers), vers is not a valid version).  However,
+    // the kernel version `kern.osrelease` is available.
+    bool launchd = (res == 0 && internal_strlen(vers) < 3);
+    if (launchd) CHECK_EQ(internal_getpid(), 1);
+
+    if (no_os_version || launchd) {
+      len = GetOSVersionFallback(vers);
     }
   }
   CHECK_LT(len, sizeof(VersStr));
 }
 
 void ParseVersion(const char *vers, u16 *major, u16 *minor) {
   // Format: <major>.<minor>[.<patch>]\0
   CHECK_GE(internal_strlen(vers), 3);
if (*major >= 16) {  // macOS 11+
     *major -= 5;
   } else {  // macOS 10.15 and below
     *minor = *major;
     *major = 10;
   }
 }
 
 static MacosVersion GetMacosAlignedVersionInternal() {
-  VersStr vers;
+  VersStr vers = {};
   GetOSVersion(vers);
 
   u16 major, minor;
   ParseVersion(vers, &major, &minor);
   MapToMacos(&major, &minor);
 
   return MacosVersion(major, minor);
 }
if (!result) {
     MacosVersion version = GetMacosAlignedVersionInternal();
     result = *reinterpret_cast<atomic_uint32_t::Type *>(&version);
     atomic_store(&cached_macos_version, result, memory_order_release);
   }
   return *reinterpret_cast<MacosVersion *>(&result);
 }
 
 DarwinKernelVersion GetDarwinKernelVersion() {
-  VersStr vers;
+  VersStr vers = {};
   uptr len = sizeof(VersStr);
   int res = internal_sysctlbyname("kern.osrelease", vers, &len, nullptr, 0);
   CHECK_EQ(res, 0);
   CHECK_LT(len, sizeof(VersStr));
 
   u16 major, minor;
   ParseVersion(vers, &major, &minor);