thakis added a comment.
(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.)
Repository:
rL LLVM
https://reviews.llvm.org/D27827
___
thakis added a comment.
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. clan
On Thu, Aug 3, 2017 at 4:13 AM, Alex Lorenz via Phabricator via cfe-commits
wrote:
> arphaman added a comment.
>
> In https://reviews.llvm.org/D27827#829661, @thakis wrote:
>
> > We just noticed that if you call __builtin_available() for the first
> time after activating your app's sandbox, the f
arphaman added a comment.
In https://reviews.llvm.org/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
thakis added a comment.
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
This revision was automatically updated to reflect the committed changes.
Closed by commit rL296015: [ObjC][CodeGen] CodeGen support for @available.
(authored by epilk).
Changed prior to commit:
https://reviews.llvm.org/D27827?vs=89305&id=89556#toc
Repository:
rL LLVM
https://reviews.llvm.o
arphaman added a comment.
Thanks for the explanation, now I get it!
LGTM, with one request for change in the tests
Comment at: test/CodeGenObjC/availability-check.m:22
+ // CHECK: br i1 true
+ if (__builtin_available(macos 10.11, *))
+;
Please add a line
erik.pilkington added inline comments.
Comment at: test/CodeGenObjC/availability-check.m:16
+ // CHECK: br i1 true
+ if (__builtin_available(ios 10, *))
+;
arphaman wrote:
> Shouldn't this be `br i1 false`, since we are building for macOS so we have
> no i
arphaman added inline comments.
Comment at: test/CodeGenObjC/availability-check.m:16
+ // CHECK: br i1 true
+ if (__builtin_available(ios 10, *))
+;
Shouldn't this be `br i1 false`, since we are building for macOS so we have no
iOS support at all?
https:
erik.pilkington updated this revision to Diff 89305.
erik.pilkington added a comment.
This new patch addresses all of Alex's comments:
- Remove `ObjC` from function names
- Rename `_IsOSVersionAtLeast` -> `__isOSVersionAtLeast`
- Improve testcase
https://reviews.llvm.org/D27827
Files:
lib/Co
> On 2017-Feb-20, at 13:11, Alex Lorenz via Phabricator
> wrote:
>
> arphaman added inline comments.
>
>
>
> Comment at: lib/CodeGen/CodeGenFunction.h:2479
>
> + llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef Args);
> +
>
> I think it's better to treat th
arphaman added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.h:2479
+ llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef Args);
+
I think it's better to treat this as a builtin in its own right, without
including the ObjC part in the names. This func
erik.pilkington updated this revision to Diff 89067.
erik.pilkington added a comment.
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 th
arphaman added a comment.
In https://reviews.llvm.org/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 rese
https://developer.apple.com/library/content/releasenotes/General/CarbonCoreDeprecations/index.html#//apple_ref/doc/uid/TP40012224-CH1-SW16
explicitly suggests sysctl as replacement when targeting 10.8+, which
strongly suggests that it'll work.
On Fri, Dec 16, 2016 at 4:37 PM, Erik Pilkington via P
erik.pilkington added a comment.
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
arphaman added a comment.
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 bet
thakis added a comment.
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 invest
erik.pilkington created this revision.
erik.pilkington added reviewers: manmanren, dexonsmith, rjmccall, thakis.
erik.pilkington added a subscriber: cfe-commits.
This patch adds CodeGen support for `@available` on macos. This is done by
compiling @available predicates into calls to `clang.is_os_v
19 matches
Mail list logo
