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

x/mobile: apps built with go 1.12, gomobile and Xcode 10.2 are using non-public APIs #31628

Closed
halseth opened this issue Apr 23, 2019 · 23 comments
Closed

Comments

@halseth
Copy link

@halseth halseth commented Apr 23, 2019

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

$ go version
go version go1.12.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/johan/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/johan/golang"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.4/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/johan/golang/src/github.com/lightningnetwork/lnd/go.mod"
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=/var/folders/kq/3436m_v11sg0l7zqtmv2r1gw0000gn/T/go-build608163721=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

After having been unable to build the lnd framework on Xcode 10.2 because of a previous issue (see #31284), I was able to build it using the latest gomobile (3e0bab5) and Xcode v10.2.1 (10E1001) after this fix was merged: #31284 (comment)

The iOS app (https://github.com/lightninglabs/lightning-app) using the framework was then built, and submitted to App Store connect.

What did you expect to see?

App builds, and app store distribution encounter no problems.

What did you see instead?

App Store submission fails with the following error:

We identified one or more issues with a recent delivery for your app, "Lightning: Fast Bitcoin Wallet". Please correct the following issues, then upload again.

Non-public API usage:

The app references non-public symbols in lightning: _ptrace

If method names in your source code match the private Apple APIs listed above, altering your method names will help prevent this app from being flagged in future submissions. In addition, note that one or more of the above APIs may be located in a static library that was included with your app. If so, they must be removed.

Additional info

Building and app store submission previously encountered no problems, when using Xcode 10.1 together with gomobile@ca80213619811c2fbed3ff8345accbd4ba924d45

@katiehockman katiehockman changed the title [gomobile] apps built with gomobile and Xcode 10.2 are using non-public APIs x/mobile: apps built with gomobile and Xcode 10.2 are using non-public APIs Apr 29, 2019
@gopherbot gopherbot added this to the Unreleased milestone Apr 29, 2019
@gopherbot gopherbot added the mobile label Apr 29, 2019
@katiehockman

This comment has been minimized.

Copy link
Member

@katiehockman katiehockman commented Apr 29, 2019

/cc @hyangah

@tanx

This comment has been minimized.

Copy link

@tanx tanx commented May 2, 2019

Any update on this? We're currently blocked to release an update to Testflight due to this issue. I'm going to try to downgrade to xcode 10.1 to work around the issue for now (see: https://stackoverflow.com/questions/14756026/how-to-downgrade-xcode-to-previous-version). Thanks

@halseth

This comment has been minimized.

Copy link
Author

@halseth halseth commented May 2, 2019

More research seems to indicate it is actually go 1.12 that introduces this failure, my hunch is changes related to: #17490

@halseth halseth changed the title x/mobile: apps built with gomobile and Xcode 10.2 are using non-public APIs x/mobile: apps built with go 1.12, gomobile and Xcode 10.2 are using non-public APIs May 2, 2019
@halseth

This comment has been minimized.

Copy link
Author

@halseth halseth commented May 2, 2019

Possibly related:

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented May 2, 2019

Do you know of documentation about when ptrace is allowed?
I suspect it is ok on OSX but not on iOS. (The manpage on my 10.12 OSX box doesn't mention anything about disallowed/deprecated/...)

It's pretty easy to rip out ptrace support, and we can do that because it is package syscall. The stdlib itself doesn't use ptrace. That said, I'd rather not break people's programs unnecessarily.

@tanx

This comment has been minimized.

Copy link

@tanx tanx commented May 2, 2019

I can confirm releasing to Testflight worked again with go@1.11.6, gomobile@ 3e0bab5405d63a8f5dd9d9764a24c8e5ac4997fa, xcode@10.2.1.

@halseth

This comment has been minimized.

Copy link
Author

@halseth halseth commented May 19, 2019

@randall77 Tried to find any documentation on this, but to no luck.

Maybe best bet is to disable it, then check if that's enough for the app to pass App Store review? I think people's programs are broken already, if they want it on the App Store that is...

@steeve

This comment has been minimized.

Copy link
Contributor

@steeve steeve commented Jun 4, 2019

Any progress on this?
It may need to be back ported go 1.12 if a fix is found, since it renders Go 1.12 unusable in a mobile scenario.
cc @eliasnaur if he has an idea ?

@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Jun 4, 2019

I think the way forward is to disable ptrace in package syscall. It should be easy because the runtime doesn't depend on it according to Keith. And then, test it in Testflight or App Store.
I don't have a paid apple dev account so I can't do it.

@sanderpick

This comment has been minimized.

Copy link

@sanderpick sanderpick commented Jun 4, 2019

I'm hoping to push a quick fix to the app store with the ptrace package disabled. Any guidance on disabling it would be appreciated! I'm taking some blind stabs now.

@sanderpick

This comment has been minimized.

Copy link

@sanderpick sanderpick commented Jun 5, 2019

I've shamelessly removed ptrace from the syscall package here: https://github.com/textileio/go/tree/sander/ptrace-hackery

Our framework is being built with it in CI here: https://github.com/textileio/go-textile/blob/master/.circleci/config.yml#L186

I'll report back when we know whether or not Apple is happy with it.

@steeve

This comment has been minimized.

Copy link
Contributor

@steeve steeve commented Jun 5, 2019

thanks @sanderpick !

@sanderpick

This comment has been minimized.

Copy link

@sanderpick sanderpick commented Jun 8, 2019

I'm happy to report that Apple has accepted the build 🎉. Thanks for the suggestion @eliasnaur.

@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Jun 8, 2019

Great! Can you send a CL with your changes so the fix can be included in Go 1.13?

@eliasnaur eliasnaur modified the milestones: Unreleased, Go1.13 Jun 12, 2019
@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Jun 12, 2019

Marking as Go 1.13 because this needs to go into the go standard library, not gomobile.

@sanderpick

This comment has been minimized.

Copy link

@sanderpick sanderpick commented Jun 12, 2019

Here's a comparison (changes added on top of go1.12.5 tag): https://github.com/textileio/go/compare/3a1b4e7..ef7e9bd

Since this is just an issue with the app store, maybe a flag or env var with gomobile could be used to indicate to go that it ought to strip ptrace from the syscall package... ?

@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Jun 12, 2019

I think it is better to change syscall.Ptrace to always return an error (for example syscall.ENAVAIL) on darwin/arm and darwin/arm64, leaving darwin/386 and darwin/amd64 alone.

Note that you can submit your changes as a Github pull request.

@cryptix

This comment has been minimized.

Copy link

@cryptix cryptix commented Jun 12, 2019

I think it is better to change syscall.Ptrace to always return an error

Would this remove the reference from the syscall trampoline, though? The App store complains about it if it is present at all. It's not a dynamic analysis if it is actually called or not.

@eliasnaur

This comment has been minimized.

Copy link
Contributor

@eliasnaur eliasnaur commented Jun 12, 2019

Sure, the trampolines and libc references need to go as well. They are already gone with @sanderpick's changes; I merely suggested the GOARCH dependent approach.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 13, 2019

Change https://golang.org/cl/182297 mentions this issue: syscall: disable ptrace on iOS

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 13, 2019

Change https://golang.org/cl/182317 mentions this issue: unix: disable ptrace on iOS

@sanderpick

This comment has been minimized.

Copy link

@sanderpick sanderpick commented Jun 13, 2019

I was too slow... thanks for tackling this @eliasnaur!

@gopherbot gopherbot closed this in 06e34e5 Jun 15, 2019
@halseth

This comment has been minimized.

Copy link
Author

@halseth halseth commented Jun 17, 2019

Thanks a lot!

cryptix added a commit to cryptix/golang_x_sys that referenced this issue Aug 12, 2019
The ptrace system call is blocked by the App Store.

Updates golang/go#31628

Change-Id: I88977cb2f0892661a7221bc822dd513a951cbf67
gopherbot pushed a commit to golang/sys that referenced this issue Aug 12, 2019
The ptrace system call is blocked by the App Store.

Updates golang/go#31628

Change-Id: I88977cb2f0892661a7221bc822dd513a951cbf67
Reviewed-on: https://go-review.googlesource.com/c/sys/+/182317
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.