[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

650

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

635

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

655

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

635

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

652

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

655

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

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

655

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

655

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
 //
 //===----------------------------------------------------------------------===//
 //
if (SANITIZER_IOSSIM) {
     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);
+    } else if (internal_strlen(vers) < 3) {
+      // Fallback for cases when sanitizer initialization runs before sysctl is
+      // setup.  Assume we run on a "very new" version of the OS.
+      len = internal_snprintf(vers, sizeof(VersStr), "99.0");
     }
   }
   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);
   const char *p = vers;
   *major = internal_simple_strtoll(p, &p, /*base=*/10);
   CHECK_EQ(*p, '.');
   p += 1;
   *minor = internal_simple_strtoll(p, &p, /*base=*/10);