[Darwin] Fix OS version checks inside simulators

[yln]

Summary

compiler-rt checks OS versions by querying the Darwin kernel version.
This is not necessarily correct inside the simulators if the simulator
runtime is not aligned with the host macOS.

Note that we still use the old code path as a fallback in case the
SIMULATOR_RUNTIME_VERSION environment variable isn't set.

rdar://63031937

[delcypher]

I don't think calling os_system_version_get_current_version is safe.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

645

Calling this potentially unsafe.

For the simulators this function appears to call asprintf() (which mallocs) and free() which means we can't use this function during ASan/TSan's init. This would mean we couldn't do version checks during init which sounds like something we'd want to do.

Libxpc appears to implement this function by looking at IPHONE_SIMULATOR_ROOT in the environment and then reading a plist file rooted at that path. Reimplementing that sounds painful. Instead we could just try to parse the SIMULATOR_RUNTIME_VERSION environment variable if its set.

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

619

I'm not a huge fan of macros. Given that we only need to do this stuff inside GetMacosAlignedVersionInternal can we just do this without them?

[yln]

Don't call os_system_version_get_current_version(), but check SIMULATOR_RUNTIME_VERSION env var instead.

[yln]

Switched to checking the SIMULATOR_RUNTIME_VERSION env var. If it doesn't exist (which should never happen?!) we fallback to the old code path which approximates the simulator runtime version by querying the kernel version. Should we fail here instead?

If SIMULATOR_RUNTIME_VERSION is mis-formatted/lying we are out of luck: we simple interpret what's there. I think this is okay.

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

645

Just to make sure I understand: we are not allowed to call anything that allocates during initialization of the sanitizers?

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

619

Trying to avoid macros/#ifdefs. Using constexpr instead.

[yln]

Cleanup test code.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

633

Print warning and fallback. Should we error out instead?

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

611

Do we really want to fatally fail here? All is needed is for SIMULATOR_RUNTIME_VERSION to be set inappropriately and then process won't even start.

Perhaps return a boolean indicating success and we can decide in GetMacosAlignedVersionInternal() how handle failure.

619

Nice.

645

Kind of. There is a point in time during initialization where it becomes safe to call malloc but it is unsafe before then. IIRC it's safe to call malloc when Symbolizer::LateInitialize() is called or later.

compiler-rt/lib/sanitizer_common/tests/sanitizer_mac_test.cpp

37

We don't build the unit tests for the simulators IIRC so I'm sure this will actually be executed.

[yln]

@delcypher: I explained my reasoning for preferring a fatal error here. Can we proceed?

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

611

Yes, I think we want a fatal error here. I think crashing on an ill-formatted env var is better than proceeding and going into a scenario that can give rise to *very* subtle bugs because the version checking off. Initially, I even thought about having a fatal error when we are in the simulator and the env var is not present at all (see comment below). I reckon you would argue agains that?

I also believe that this scenario of: 1) running in simulator, 2) env var present, 3) but ill-formatted should really never happen. Do you anticipate this happening?

Note: we still can get into a tricky situation, if the env var is well formatted, but lying. I don't think we can do anything about that.

compiler-rt/lib/sanitizer_common/tests/sanitizer_mac_test.cpp

37

That's right, this is here for expressing intent (and maybe we will run the unit tests in the simulator one day?).
Any suggestions on how to better test this?

I just did an additional manual test for GetMacosAlignedVersion() with a small HelloWorld plus a Printf int the runtime to print the parsed version.

[yln]

Rebase/compensate for recent version scheme change (macOS 11).

[yln]

Add UNREACHABLE case for macOS to GetDarwinKernelMajorFromOSMajor() to document that it is not supported.

[delcypher]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

636

This function name and API is leaking the implementation detail that it only needs the major. Maybe just have it take a DarwinKernelVersion argument and rename it to something like GetMacosVersionFromDarwinKernelVersion?

This change is optional.

compiler-rt/lib/sanitizer_common/tests/sanitizer_mac_test.cpp

37

Given that this won't execute I think this deserves a

TODO(yln): rdar://problem/<number> Build sanitizer unit tests for the Darwin simulators.

[yln]

I accidentally pushed this. Reverting and creating another revision for this.

[yln]

Reverted in bd88991a011be895ba73fe07015df25e1e55992e.

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

636

The kernel_major for the two call sites come from different sources:

1. GetDarwinKernelVersion().major
2. GetEnv("SIMULATOR_RUNTIME_VERSION") (os major) -> GetDarwinKernelMajorFromOSMajor()

While there is a predictable mapping from Darwin kernel major to os major, this is not the case for the minor component. So we can't create a DarwinKernelVersion in this case.

This is also the reason why we can only really check for major OS versions (the macOS 10.x version scheme makes this a bit confusing). But we can check for <major>.<minor> kernel versions.

compiler-rt/lib/sanitizer_common/tests/sanitizer_mac_test.cpp

37

I added this in the new revision.

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
 //
 //===----------------------------------------------------------------------===//
 //
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;
 }
 
+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);
+}
+
+constexpr u16 GetMacosVersionOffset() {
+  if (SANITIZER_IOS || SANITIZER_TVOS) return 2;
+  if (SANITIZER_WATCHOS) return 9;
+  return 0;
+}
+
 static MacosVersion GetMacosAlignedVersionInternal() {
+  if (SANITIZER_IOSSIM) {
+    if (auto vers = GetEnv("SIMULATOR_RUNTIME_VERSION")) {
+      u16 major, minor;
+      ParseVersion(vers, &major, &minor);
+      return MacosVersion(10, major + GetMacosVersionOffset());
+    }
+    Report("WARNING: Running in simulator but SIMULATOR_RUNTIME_VERSION env "
+           "var is not set.\n");
+  }
+
   u16 kernel_major = GetDarwinKernelVersion().major;
-  const u16 version_offset = 4;
+  u16 version_offset = 4;
   CHECK_GE(kernel_major, version_offset);
-  u16 macos_major = kernel_major - version_offset;
-  return MacosVersion(10, macos_major);
+  return MacosVersion(10, kernel_major - version_offset);
 }
 
 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);
   }
   return *reinterpret_cast<MacosVersion *>(&result);
 }
 
-void ParseVersion(const char *vers, u16 *major, u16 *minor) {
-  // Format: <major>.<minor>.<patch>\0
-  CHECK_GE(internal_strlen(vers), 5);
-  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);
-}
-
 DarwinKernelVersion GetDarwinKernelVersion() {
-  char buf[100];
-  size_t len = sizeof(buf);
-  int res = internal_sysctlbyname("kern.osrelease", buf, &len, nullptr, 0);
+  char vers[100];
+  size_t len = sizeof(vers);
+  int res = internal_sysctlbyname("kern.osrelease", vers, &len, nullptr, 0);
   CHECK_EQ(res, 0);
 
   u16 major, minor;
-  ParseVersion(buf, &major, &minor);
+  ParseVersion(vers, &major, &minor);
 
   return DarwinKernelVersion(major, minor);
 }
 
 uptr GetRSS() {
   struct task_basic_info info;
   unsigned count = TASK_BASIC_INFO_COUNT;
   kern_return_t result =

compiler-rt/lib/sanitizer_common/tests/sanitizer_mac_test.cpp

 //===-- sanitizer_mac_test.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
 //
 //===----------------------------------------------------------------------===//
 //
 // Tests for sanitizer_mac.{h,cpp}
 //
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_platform.h"
 #if SANITIZER_MAC
 
 #include "sanitizer_common/sanitizer_mac.h"
 
 #include "gtest/gtest.h"
 
 #include <sys/sysctl.h>  // sysctlbyname
 #include <mach/kern_return.h>  // KERN_SUCCESS
 
 namespace __sanitizer {
 
-TEST(SanitizerMac, GetMacosAlignedVersion) {
-  MacosVersion vers = GetMacosAlignedVersion();
-  EXPECT_EQ(vers.major, 10);
-  EXPECT_EQ(vers.minor, GetDarwinKernelVersion().major - 4);
-}
-
 void ParseVersion(const char *vers, u16 *major, u16 *minor);
 
 TEST(SanitizerMac, ParseVersion) {
   u16 major, minor;
+
   ParseVersion("11.22.33", &major, &minor);
-  EXPECT_EQ(major, 11);
-  EXPECT_EQ(minor, 22);
+  EXPECT_EQ(major, 11); EXPECT_EQ(minor, 22);
+
+  ParseVersion("1.2", &major, &minor);
+  EXPECT_EQ(major, 1); EXPECT_EQ(minor, 2);
+}
+
+#if SANITIZER_IOSSIM
+TEST(SanitizerMac, GetMacosAlignedVersion) {
+  if (SANITIZER_IOSSIM) {
+    const char *vers;
+    if (SANITIZER_IOS || SANITIZER_TVOS) {
+      vers = "13.0";
+    } else if (SANITIZER_WATCHOS) {
+      vers = "6.5"
+    } else {
+      FAIL() << "unsupported simulator runtime";
+    }
+    setenv("SIMULATOR_RUNTIME_VERSION", vers, /*overwrite=*/1);
+  }
+
+  MacosVersion vers = GetMacosAlignedVersion();
+  EXPECT_EQ(vers.major, 10);
+  EXPECT_EQ(vers.minor, 15);
+}
+#else
+TEST(SanitizerMac, GetMacosAlignedVersion) {
+  MacosVersion vers = GetMacosAlignedVersion();
+  EXPECT_EQ(vers.major, 10);
+  EXPECT_EQ(vers.minor, GetDarwinKernelVersion().major - 4);
 }
+#endif
 
 TEST(SanitizerMac, GetDarwinKernelVersion) {
   DarwinKernelVersion vers = GetDarwinKernelVersion();
   std::ostringstream oss;
   oss << vers.major << '.' << vers.minor;
   std::string actual = oss.str();
 
   char buf[100];