[ObjC] CodeGen support for @available on macOS

[erik.pilkington]

Summary

This patch adds CodeGen support for @available on macos. This is done by compiling @available predicates into calls to clang.is_os_version_at_least. This function first populates 3 global variables which hold the major, minor and subminor with the operating system version, using core services' Gestalt() (https://developer.apple.com/reference/coreservices/1471624-gestalt?language=objc), and grand central dispatch's dispatch_once_f to make sure that this is only done once. After that, it compares the version that was passed to it with the host's OS version.

Unfortunately, Gestalt() is only available on macOS. In a follow-up patch, we can add support for other platforms. I think that the best way of doing that is to depend on Objective-C's NSProcessInfo. I'm not super familiar with this part of the compiler, and in particular I'm unsure of whether we should be directly calling into library functions like Gestalt and dispatch_once_f, so please tear this patch apart!

This patch is part of a feature that I proposed last summer here:
http://lists.llvm.org/pipermail/cfe-dev/2016-July/049851.html

Thanks,
Erik

[thakis]

Huh, I'm surprised this has a runtime component, I missed that. I thought this would be a statically-checked thing. But having this does seem useful :-)

I'm also surprised that this compiles down to Gestalt – Gestalt is deprecated in newer SDKs, and in Chromium we invested some time to move off of it when recently bumping our min SDK target. So it seems strange the compiler would add a dependency to it. (We've also observed that Gestalt is pretty heavy-weight at least in macOS 10.6 in that it can spawn threads).

Things we've done instead, in various contexts:

• uname() (https://cs.chromium.org/chromium/src/base/mac/mac_util.mm?q=MacOSXMinorVersion&sq=package:chromium&dr=C&l=365)
• -[NSProcessInfo operatingSystemVersion] if it's available (it's documented available in 10.10+, but actually exists from 10.9.2+) (rhs on https://codereview.chromium.org/1842213002/diff/40001/base/sys_info_mac.mm)
• sysctl() https://github.com/google/google-toolbox-for-mac/blob/master/Foundation/GTMServiceManagement.c#L63 That's what's recommended but might need 10.8

[arphaman]

I don't think that generating calls to dispatch_once and Gestalt is the right approach. Swift generates a call to _stdlib_isOSVersionAtLeast, so maybe we could add some function like that into compiler-rt?

Gestalt is also deprecated, we should use something better. Swift's _stdlib_isOSVersionAtLeast calls _swift_stdlib_operatingSystemVersion (https://github.com/apple/swift/blob/2fe4254cb712fa101a220f95b6ade8f99f43dc74/stdlib/public/core/Availability.swift) which in turn either calls NSProcessInfo.operatingSystemVersion or loads the version from a PLIST file (https://github.com/apple/swift/blob/3328592c20ac51a7c525a439af778a75521ab781/stdlib/public/stubs/Availability.mm). I don't think we can use Objective-C in compiler-rt though, so we probably have to use another solution. Maybe use sysctlbyname("kern.osrelease", ...) that returns the version of the kernel, which can be remapped into the OS version (although that seems hacky)?

lib/CodeGen/CGExprScalar.cpp

305 ↗

(On Diff #81647)

You can move this line after the if below since these Min and SMin are used only after the if.

[erik.pilkington]

I seem to remember that mapping from kernel versions to marketing versions was tossed around as a potential alternative to Gestalt. I think that the problem was Apple sometimes introduces (or reserves the right to introduce) new SDKs in a patch release (ie, Z in X.Y.Z), which wouldn't necessary imply a new kernel version, and would still need to be queried by @available. This makes it impossible to use kernel version in the general case (Or at least I think, it would be nice if someone internal to Apple could confirm this?). This rules out using sysctl() and the like.

AFAIK this just leaves Gestalt() and Objective-C's NSProcessInfo, the latter would mean pulling in the Objective-C runtime, which would be unfortunate for C users (using the __builtin_available spelling). I don't think Gestalt() is in any danger of actually being removed, so we might as well use it. The only alternative I know of to those would be manually parsing the SystemVersion.plist XML file, but I think that might cause problems with sandboxed processes (right?). Any thoughts here would be much appreciated, Gestalt() is by no means a perfect solution.

Compiler-rt does seem like a good place it put this, should I move the runtime code there instead?

Thanks for taking a look!
Erik

[arphaman]

In D27827#625438, @erik.pilkington wrote:

I seem to remember that mapping from kernel versions to marketing versions was tossed around as a potential alternative to Gestalt. I think that the problem was Apple sometimes introduces (or reserves the right to introduce) new SDKs in a patch release (ie, Z in X.Y.Z), which wouldn't necessary imply a new kernel version, and would still need to be queried by @available. This makes it impossible to use kernel version in the general case (Or at least I think, it would be nice if someone internal to Apple could confirm this?). This rules out using sysctl() and the like.

This sounds like a legitimate concern, I'll try to get some information about this.

AFAIK this just leaves Gestalt() and Objective-C's NSProcessInfo, the latter would mean pulling in the Objective-C runtime, which would be unfortunate for C users (using the __builtin_available spelling). I don't think Gestalt() is in any danger of actually being removed, so we might as well use it. The only alternative I know of to those would be manually parsing the SystemVersion.plist XML file, but I think that might cause problems with sandboxed processes (right?). Any thoughts here would be much appreciated, Gestalt() is by no means a perfect solution.

Sandboxed applications are allowed to read that PLIST file, since https://developer.apple.com/library/content/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html states the applications are permitted to read files that are world readable in /System. I confirmed that with a test sandboxed app as well. It would seem that reading that PLIST file is a good solution if we can get access to the PLIST APIs from compiler-rt (+ we can always fallback to Gestalt/sysctl if something goes wrong).

Compiler-rt does seem like a good place it put this, should I move the runtime code there instead?

Yes, please.

Btw, compiler-rt already has code that checks which version of macOS it's running on (https://github.com/llvm-project/compiler-rt/blob/master/lib/sanitizer_common/sanitizer_mac.cc#L410). It uses sysctl as well. I'm not sure if we can somehow refactor and reuse this code or not (since ours will be in builtins), but at least it's a good starting point.

Thanks for taking a look!
Erik

[erik.pilkington]

This new patch just generates a call to the function _IsOSVersionAtLeast and branches on the result. _IsOSVersionAtLeast is going through review here: https://reviews.llvm.org/D30136.

I decided to parse the SystemVersion.plist file, I think mapping from kernel versions to marketing versions works for macOS, but not for iOS, as iOS tends to cling on to darwin versions for longer. For example, darwin 14.0.0 was used from iOS 7.0 until iOS 8.4.1, (based on the table here: https://www.theiphonewiki.com/wiki/Kernel) which is not fine grain enough for use with @available.

Thanks for taking a look!
Erik

[arphaman]

lib/CodeGen/CodeGenFunction.h

2479 ↗

(On Diff #89067)

I think it's better to treat this as a builtin in its own right, without including the ObjC part in the names. This function could be renamed to something like EmitBuiltinAvailable and the variable that holds the function pointer should be renamed appropriately as well.

test/CodeGenObjC/availability-check.m

5 ↗

(On Diff #89067)

I think the function name should have the lowercase is

7 ↗

(On Diff #89067)

Can you add a call to the builtin that has a non-zero sub-minor version as well?

[erik.pilkington]

This new patch addresses all of Alex's comments:

• Remove ObjC from function names
• Rename _IsOSVersionAtLeast -> __isOSVersionAtLeast
• Improve testcase

[arphaman]

test/CodeGenObjC/availability-check.m

16 ↗

(On Diff #89305)

Shouldn't this be br i1 false, since we are building for macOS so we have no iOS support at all?

[erik.pilkington]

test/CodeGenObjC/availability-check.m

16 ↗

(On Diff #89305)

No, this is intentional. If the platform we're targeting isn't mentioned, we take the * case, and emit -Wunguarded-availability diagnostics in the body of the if using the minimum deployment target. The idea is that if a new OS is released it will be forked from an existing one and use existing APIs, and it would be unfortunate for everyone to have to add the new platform to their existing @available calls. This is probably the most counterintuative part of this feature, and is the reason for the somewhat bizarre * syntax, to call out this control flow.

[arphaman]

Thanks for the explanation, now I get it!

LGTM, with one request for change in the tests

test/CodeGenObjC/availability-check.m

22 ↗

(On Diff #89305)

Please add a line that verifies that @available() uses the builtin as well.

[thakis]

We just noticed that if you call __builtin_available() for the first time after activating your app's sandbox, the function will fail:

SandboxViolation: crdmg(15489) deny file-read-data /System/Library/CoreServices/SystemVersion.plist
Violation: deny file-read-data /System/Library/CoreServices/SystemVersion.plist
Process: crdmg [15489]
Path: /Volumes/Build/src/./out/release/crdmg

Thread 0 (id: 421251):
0 libsystem_kernel.dylib 0x00007fffe94a1a86 __open_nocancel + 10
1 crdmg 0x000000010444be98 parseSystemVersionPList + 360
2 0xec83485354415541

[arphaman]

In D27827#829661, @thakis wrote:

We just noticed that if you call __builtin_available() for the first time after activating your app's sandbox, the function will fail:

SandboxViolation: crdmg(15489) deny file-read-data /System/Library/CoreServices/SystemVersion.plist
Violation: deny file-read-data /System/Library/CoreServices/SystemVersion.plist
Process: crdmg [15489]
Path: /Volumes/Build/src/./out/release/crdmg

Thread 0 (id: 421251):
0 libsystem_kernel.dylib 0x00007fffe94a1a86 __open_nocancel + 10
1 crdmg 0x000000010444be98 parseSystemVersionPList + 360
2 0xec83485354415541

Hmm, never saw this before. Please post your exact configuration - clang/compiler-rt versions, OS version, toolchain & SDK. Is it possible to get a reproducer?

[thakis]

Looks like the email reply didn't make it to phab, so here it is again:

It's in this program, which is pretty stand-alone: https://cs.chromium.org/chromium/src/chrome/utility/safe_browsing/mac/crdmg.cc?q=crdmg&sq=package:chromium&l=95 EnableSandbox() is on line 134. clang, compiler-rt are trunk from 2 weeks ago, SDK is 10.12, os 10.12.5. I don't think the particular version numbers matter too much though. Here's a standalone demo:

thakis-macpro:src thakis$ cat foo.cc
#include <sandbox.h>
int main() {

const char sbox[] = "(version 1) (deny default)";
char* err;
::sandbox_init(sbox, 0, &err);
if (__builtin_available(macos 10.10, *))
  return 32;
else
  return 14;

}
thakis-macpro:src thakis$ third_party/llvm-build/Release+Asserts/bin/clang -o foo foo.cc -isysroot $(xcrun -show-sdk-path) -mmacosx-version-min=10.9 -w && ./foo
thakis-macpro:src thakis$ echo $?
14
thakis-macpro:src thakis$ sw_vers -productVersion
10.12.5

After running that, look for "sandbox" in console.app to find the "deny file-read-data".

[thakis]

(Our workaround is to call __builtin_available() once before engaging the sandbox, which isn't so bad. Just thought I'd let you know about it; this isn't a serious bug for us.)

cfe/trunk/lib/CodeGen/CGExprScalar.cpp

public:
   }
 
   Value *VisitObjCIsaExpr(ObjCIsaExpr *E) {
     LValue LV = CGF.EmitObjCIsaExpr(E);
     Value *V = CGF.EmitLoadOfLValue(LV, E->getExprLoc()).getScalarVal();
     return V;
   }
 
+  Value *VisitObjCAvailabilityCheckExpr(ObjCAvailabilityCheckExpr *E) {
+    VersionTuple Version = E->getVersion();
+
+    // If we're checking for a platform older than our minimum deployment
+    // target, we can fold the check away.
+    if (Version <= CGF.CGM.getTarget().getPlatformMinVersion())
+      return llvm::ConstantInt::get(Builder.getInt1Ty(), 1);
+
+    Optional<unsigned> Min = Version.getMinor(), SMin = Version.getSubminor();
+    llvm::Value *Args[] = {
+        llvm::ConstantInt::get(CGF.CGM.Int32Ty, Version.getMajor()),
+        llvm::ConstantInt::get(CGF.CGM.Int32Ty, Min ? *Min : 0),
+        llvm::ConstantInt::get(CGF.CGM.Int32Ty, SMin ? *SMin : 0),
+    };
+
+    return CGF.EmitBuiltinAvailable(Args);
+  }
+
   Value *VisitArraySubscriptExpr(ArraySubscriptExpr *E);
   Value *VisitShuffleVectorExpr(ShuffleVectorExpr *E);
   Value *VisitConvertVectorExpr(ConvertVectorExpr *E);
   Value *VisitMemberExpr(MemberExpr *E);
   Value *VisitExtVectorElementExpr(Expr *E) { return EmitLoadOfLValue(E); }
   Value *VisitCompoundLiteralExpr(CompoundLiteralExpr *E) {
     return EmitLoadOfLValue(E);
   }

cfe/trunk/lib/CodeGen/CGObjC.cpp

CodeGenFunction::EmitBlockCopyAndAutorelease(llvm::Value *Block, QualType Ty) {
   Val = Result.getScalarVal();
   Result = Runtime.GenerateMessageSend(*this, ReturnValueSlot(),
                                        Ty, AutoreleaseSelector,
                                        Val, CallArgList(), nullptr, nullptr);
   Val = Result.getScalarVal();
   return Val;
 }
 
+llvm::Value *
+CodeGenFunction::EmitBuiltinAvailable(ArrayRef<llvm::Value *> Args) {
+  assert(Args.size() == 3 && "Expected 3 argument here!");
+
+  if (!CGM.IsOSVersionAtLeastFn) {
+    llvm::FunctionType *FTy =
+        llvm::FunctionType::get(Int32Ty, {Int32Ty, Int32Ty, Int32Ty}, false);
+    CGM.IsOSVersionAtLeastFn =
+        CGM.CreateRuntimeFunction(FTy, "__isOSVersionAtLeast");
+  }
+
+  llvm::Value *CallRes =
+      EmitNounwindRuntimeCall(CGM.IsOSVersionAtLeastFn, Args);
+
+  return Builder.CreateICmpNE(CallRes, llvm::Constant::getNullValue(Int32Ty));
+}
 
 CGObjCRuntime::~CGObjCRuntime() {}

cfe/trunk/lib/CodeGen/CodeGenFunction.h

llvm::Value *EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
                                           const CallExpr *E);
 
 private:
   enum class MSVCIntrin;
 
 public:
   llvm::Value *EmitMSVCBuiltinExpr(MSVCIntrin BuiltinID, const CallExpr *E);
 
+  llvm::Value *EmitBuiltinAvailable(ArrayRef<llvm::Value *> Args);
+
   llvm::Value *EmitObjCProtocolExpr(const ObjCProtocolExpr *E);
   llvm::Value *EmitObjCStringLiteral(const ObjCStringLiteral *E);
   llvm::Value *EmitObjCBoxedExpr(const ObjCBoxedExpr *E);
   llvm::Value *EmitObjCArrayLiteral(const ObjCArrayLiteral *E);
   llvm::Value *EmitObjCDictionaryLiteral(const ObjCDictionaryLiteral *E);
   llvm::Value *EmitObjCCollectionLiteral(const Expr *E,
                                 const ObjCMethodDecl *MethodWithObjects);
   llvm::Value *EmitObjCSelectorExpr(const ObjCSelectorExpr *E);

cfe/trunk/lib/CodeGen/CodeGenModule.h

CGCUDARuntime &getCUDARuntime() {
     return *CUDARuntime;
   }
 
   ObjCEntrypoints &getObjCEntrypoints() const {
     assert(ObjCData != nullptr);
     return *ObjCData;
   }
 
+  // Version checking function, used to implement ObjC's @available:
+  // i32 @__isOSVersionAtLeast(i32, i32, i32)
+  llvm::Constant *IsOSVersionAtLeastFn = nullptr;
+
   InstrProfStats &getPGOStats() { return PGOStats; }
   llvm::IndexedInstrProfReader *getPGOReader() const { return PGOReader.get(); }
 
   CoverageMappingModuleGen *getCoverageMapping() const {
     return CoverageMapping.get();
   }
 
   llvm::Constant *getStaticLocalDeclAddress(const VarDecl *D) {

cfe/trunk/test/CodeGenObjC/availability-check.m

+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11 -emit-llvm -o - %s | FileCheck %s
+
+void use_at_available() {
+  // CHECK: call i32 @__isOSVersionAtLeast(i32 10, i32 12, i32 0)
+  // CHECK-NEXT: icmp ne
+  if (__builtin_available(macos 10.12, *))
+    ;
+
+  // CHECK: call i32 @__isOSVersionAtLeast(i32 10, i32 12, i32 0)
+  // CHECK-NEXT: icmp ne
+  if (@available(macos 10.12, *))
+    ;
+
+  // CHECK: call i32 @__isOSVersionAtLeast(i32 10, i32 12, i32 42)
+  // CHECK-NEXT: icmp ne
+  if (__builtin_available(ios 10, macos 10.12.42, *))
+    ;
+
+  // CHECK-NOT: call i32 @__isOSVersionAtLeast
+  // CHECK: br i1 true
+  if (__builtin_available(ios 10, *))
+    ;
+
+  // This check should be folded: our deployment target is 10.11.
+  // CHECK-NOT: call i32 @__isOSVersionAtLeast
+  // CHECK: br i1 true
+  if (__builtin_available(macos 10.11, *))
+    ;
+}
+
+// CHECK: declare i32 @__isOSVersionAtLeast(i32, i32, i32)