[driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system

[arphaman]

Summary

This patch improves the driver by making sure that it picks the system version for the deployment target when the version of the macOS SDK is newer than the system version.

[inouehrs]

lib/Driver/ToolChains/Darwin.cpp

1133 ↗

(On Diff #102425)

What these magic numbers mean (especially minor and micro versions)?
You may need some comments.

[arphaman]

lib/Driver/ToolChains/Darwin.cpp

1133 ↗

(On Diff #102425)

I actually just took it from the function below which checks if the macOS version is valid (so we want 10.XX.XX). But looking at it again, I think that we don't really them here as the SDK version should will probably be valid. I'll update the patch and remove them from the code.

[arphaman]

Remove redundant driver version correctness checks.

[dexonsmith]

LGTM.

[kubamracek]

Nice!

cfe/trunk/lib/Driver/ToolChains/Darwin.cpp

if (isTargetWatchOSBased()) {
     // the OS X builtins library.
     if (isMacosxVersionLT(10, 5))
       AddLinkRuntimeLib(Args, CmdArgs, "libclang_rt.10.4.a");
     else
       AddLinkRuntimeLib(Args, CmdArgs, "libclang_rt.osx.a");
   }
 }
 
+/// Returns the most appropriate macOS target version for the current process.
+///
+/// If the macOS SDK version is the same or earlier than the system version,
+/// then the SDK version is returned. Otherwise the system version is returned.
+static std::string getSystemOrSDKMacOSVersion(StringRef MacOSSDKVersion) {
+  unsigned Major, Minor, Micro;
+  llvm::Triple(llvm::sys::getProcessTriple())
+      .getMacOSXVersion(Major, Minor, Micro);
+  VersionTuple SystemVersion(Major, Minor, Micro);
+  bool HadExtra;
+  if (!Driver::GetReleaseVersion(MacOSSDKVersion, Major, Minor, Micro,
+                                 HadExtra))
+    return MacOSSDKVersion;
+  VersionTuple SDKVersion(Major, Minor, Micro);
+  if (SDKVersion > SystemVersion)
+    return SystemVersion.getAsString();
+  return MacOSSDKVersion;
+}
+
 void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
   const OptTable &Opts = getDriver().getOpts();
 
   // Support allowing the SDKROOT environment variable used by xcrun and other
   // Xcode tools to define the default sysroot, by making it the default for
   // isysroot.
   if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
     // Warn if the path does not exist.
if (OSXTarget.empty() && iOSTarget.empty() && WatchOSTarget.empty() &&
           size_t StartVer = SDK.find_first_of("0123456789");
           size_t EndVer = SDK.find_last_of("0123456789");
           if (StartVer != StringRef::npos && EndVer > StartVer) {
             StringRef Version = SDK.slice(StartVer, EndVer + 1);
             if (SDK.startswith("iPhoneOS") ||
                 SDK.startswith("iPhoneSimulator"))
               iOSTarget = Version;
             else if (SDK.startswith("MacOSX"))
-              OSXTarget = Version;
+              OSXTarget = getSystemOrSDKMacOSVersion(Version);
             else if (SDK.startswith("WatchOS") ||
                      SDK.startswith("WatchSimulator"))
               WatchOSTarget = Version;
             else if (SDK.startswith("AppleTVOS") ||
                      SDK.startswith("AppleTVSimulator"))
               TvOSTarget = Version;
           }
         }

cfe/trunk/test/Driver/darwin-sdkroot.c

 // RUN: env SDKROOT=%t/SDKs/MacOSX10.10.0.sdk %clang -target x86_64-apple-darwin %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MACOSX %s
 //
 // CHECK-MACOSX: clang
 // CHECK-MACOSX: "-cc1"
 // CHECK-MACOSX: "-triple" "x86_64-apple-macosx10.10.0"
 // CHECK-MACOSX: ld
 // CHECK-MACOSX: "-macosx_version_min" "10.10.0"
+
+// Ensure that we never pick a version that's based on the SDK that's newer than
+// the system version:
+// RUN: rm -rf %t/SDKs/MacOSX10.99.99.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.99.99.sdk
+// RUN: %clang -target x86_64-apple-darwin -isysroot %t/SDKs/MacOSX10.99.99.sdk %s -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MACOSX-SYSTEM-VERSION %s
+
+// CHECK-MACOSX-SYSTEM-VERSION-NOT: 10.99.99"