Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall: darwin arm{,64} libSystem syscalls are not compliant with the App Store submissions #28984

Closed
s-s opened this issue Nov 28, 2018 · 17 comments

Comments

@s-s
Copy link

@s-s s-s commented Nov 28, 2018

What version of Go are you using (go version)?

go version devel +bf870f3d54 Tue Nov 27 18:04:27 2018 +0300 darwin/amd64

Does this issue reproduce with the latest release?

Only master branch affected, go 1.11.2 and 1.10.5 are fine (as they use bare Syscall functions).

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="off"
GOEXE=""
GOFLAGS="-ldflags=-s -ldflags=-w"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="<...>/.build"
GOPROXY=""
GORACE=""
GOROOT="<...>/go"
GOTMPDIR=""
GOTOOLDIR="<...>/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build998597890=/tmp/go-build -gno-record-gcc-switches -fno-common"

Actually, I'm using gomobile, so GOARCHs are arm, arm64 (and 386, amd64 for simulator).

What did you do?

I'm building an iOS framework using gomobile and Go from master branch. Then I submit an app with this framework to the App Store Connect. After submission some (unknown) post processing happens on Apple's side.

What did you expect to see?

Binary should be processed successfully. (It is for go 1.11.2 and 1.10.5)

What did you see instead?

Binary is rejected for "Non-public API usage" - The app references non-public symbols: ___getdirentries64, _fstat64, _lstat64, _stat64

This happens because of these changes: a3b0144, c9762b8 in master branch

PS: I've solved it by patching syscall/mksyscall.pl - reverting back bare Syscall usage for mentioned symbols and generating updated zsyscall_darwin_{arm,arm64,386,amd64}.{go,s} using syscall/mksyscall.pl and syscall/mkasm_darwin.go.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 28, 2018

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Nov 28, 2018

Because it is a regression, I took the liberty and made this issue a release blocker.

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Nov 28, 2018

According to the macOS stat(2) man page, platforms released after widening inodes to 64-bit do not have the *64 variants available, and the non-postfixed system calls are already 64-bit compliant:

"Platforms that were released after these updates only have the newer variants available to them. These platforms have the macro _DARWIN_FEATURE_ONLY_64_BIT_INODE defined."

I checked with a simple Xcode project that _DARWIN_FEATURE_ONLY_64_BIT_INODE is indeed defined for iOS builds, but not for macOS builds.

The fix for fstat, lstat, stat seems to be to drop the "64" postfix on iOS.

Getdirentries is more complicated. According to its man page,

"As of Mac OS X 10.6, getdirentries() is deprecated, and it is recommended that applications use readdir(3) rather than using
getdirentries() directly. Due to limitations with the system call, getdirentries() will not work with 64-bit inodes; in order to use
getdirentries(), _DARWIN_NO_64_BIT_INODE must be defined. See stat(2) for more information on _DARWIN_NO_64_BIT_INODE and its other
effects."

Since _DARWIN_NO_64_BIT_INODE must be defined to access getdirentries, and since iOS has _DARWIN_FEATURE_ONLY_64_BIT_INODE defined, it is likely that getdirentries is not (supposed to be) available on iOS in any form.

So it seems that getdirentries has to be replaced, at least on iOS. I don't know whether that's realistic. Perhaps getdirentries could be removed from package syscall on iOS altogether, even though it breaks the Go compatibility guarantee.

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Nov 29, 2018

I worked a bit on this; removing the "64" postfixes was easy, but getdirentries is a central system call for directory listing, and I couldn't think of a straightforward replacement for it. The man pages suggest the (fd)opendir/readdir/closedir API but those calls operate on directory stream pointers, not file descriptors. I'm leaving this for more capable hands.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 29, 2018

@eliasnaur Want to send the fixes you have? After we get those in, changing syscall.ReadDirent to not use Getdirentries on Darwin should be doable. I'm not too concerned about mobile programs calling syscall.Getdirentries directly.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 30, 2018

Change https://golang.org/cl/151938 mentions this issue: syscall: avoid "64"-postfixed libSystem syscalls on iOS

gopherbot pushed a commit that referenced this issue Dec 1, 2018
The stat(2) man page contain this comment about the 64-bit versions
of the system file functions:

"Platforms that were released after these updates only have the
newer variants available to them.  These platforms have the macro
_DARWIN_FEATURE_ONLY_64_BIT_INODE defined."

It turns out that on iOS the _DARWIN_FEATURE_ONLY_64_BIT_INODE is
defined and that even though the "64"-postfixed versions are
accessible they are deemed private. Apps that refer to private
API are not admissible on App Store, and after the Go runtime
started using libSystem instead of direct syscalls, the App Store
submission checks reject apps built with Go tip.

The fix is simple: use the non-postfixed versions on iOS.

getdirentries(2) is not changed; it is not available at all on iOS
and needs replacement.

Updates #28984

Change-Id: Icb8d44e271456acaa1913ba486fcf5b569722fa9
Reviewed-on: https://go-review.googlesource.com/c/151938
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 9, 2018

Change https://golang.org/cl/153338 mentions this issue: runtime,os,syscall,internal/poll: replace getdirentries on iOS

@gopherbot gopherbot closed this in 9eb383e Dec 13, 2018
@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Dec 13, 2018

@s-s, can you please verify that the issue is fixed in go tip (or the beta release that is imminent)?

@s-s
Copy link
Author

@s-s s-s commented Dec 14, 2018

@eliasnaur, checked with tip version - everything is ok now! Thank you!
PS: go version devel +38e7177c94 Fri Dec 14 05:48:18 2018 +0000 darwin/amd64

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 18, 2018

Change https://golang.org/cl/154658 mentions this issue: unix: avoid "64"-postfixed libSystem calls on iOS

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 18, 2018

Change https://golang.org/cl/154659 mentions this issue: unix: remove Getdirentries on iOS

gopherbot pushed a commit to golang/sys that referenced this issue Dec 20, 2018
Parallel to CL 151938 for package x/sys/unix instead of syscall.

iOS needs to use these functions without the "64" postfix.
(The functions do exist, but the App Store bans their use.)

Updates golang/go#28984

Change-Id: I6b82950700cc8a1afca612844b05fa007574e008
Reviewed-on: https://go-review.googlesource.com/c/154658
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/sys that referenced this issue Dec 20, 2018
This system call doesn't exist on iOS.

Update golang/go#28984

Change-Id: I92eb6fd3eb263863a31338bc18c9826a2242434b
Reviewed-on: https://go-review.googlesource.com/c/154659
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 20, 2019

Change https://golang.org/cl/168479 mentions this issue: syscall: deprecate Getdirentries on darwin

gopherbot pushed a commit that referenced this issue Apr 3, 2019
Getdirentries is implemented with the __getdirentries64 function
in libSystem.dylib. That function works, but it's on Apple's
can't-be-used-in-an-app-store-application list.

Implement Getdirentries using the underlying fdopendir/readdir_r/closedir.
The simulation isn't faithful, and could be slow, but it should handle
common cases.

Don't use Getdirentries in the stdlib, use fdopendir/readdir_r/closedir
instead (via (*os.File).readdirnames).

Fixes #30933

Update #28984

RELNOTE=yes

Change-Id: Ia6b5d003e5bfe43ba54b1e1d9cfa792cc6511717
Reviewed-on: https://go-review.googlesource.com/c/go/+/168479
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 3, 2019

Change https://golang.org/cl/170640 mentions this issue: [release-branch.go1.12] syscall: avoid _getdirentries64 on darwin

gopherbot pushed a commit that referenced this issue Apr 5, 2019
Getdirentries is implemented with the __getdirentries64 function
in libSystem.dylib. That function works, but it's on Apple's
can't-be-used-in-an-app-store-application list.

Implement Getdirentries using the underlying fdopendir/readdir_r/closedir.
The simulation isn't faithful, and could be slow, but it should handle
common cases.

Don't use Getdirentries in the stdlib, use fdopendir/readdir_r/closedir
instead (via (*os.File).readdirnames).

(Incorporates CL 170837 and CL 170698, which were small fixes to the
original tip CL.)

Fixes #31244

Update #28984

RELNOTE=yes

Change-Id: Ia6b5d003e5bfe43ba54b1e1d9cfa792cc6511717
Reviewed-on: https://go-review.googlesource.com/c/go/+/168479
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 9da6530)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170640
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Sep 19, 2019

@eliasnaur does this need a corresponding commit in x/sys?

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Sep 19, 2019

Do you mean https://golang.org/cl/154659 or something else?

@eliasnaur eliasnaur reopened this Sep 19, 2019
@eliasnaur eliasnaur closed this Sep 19, 2019
@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Sep 19, 2019

@eliasnaur That CL is for iOS-only, right? The syscall change later got ported to macOS (x86, amd64) too, due to similar appstore restrictions there. But I don't see a corresponding x/sys change for macOS.

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Sep 19, 2019

@golang golang locked and limited conversation to collaborators Sep 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.