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

Compilation fails on aarch64-apple-darwin #1063

Closed
est31 opened this issue Oct 16, 2020 · 30 comments
Closed

Compilation fails on aarch64-apple-darwin #1063

est31 opened this issue Oct 16, 2020 · 30 comments

Comments

@est31
Copy link

est31 commented Oct 16, 2020

I noticed it in the CI output of rust-lang/rustup#2517 :

  running "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-arch" "arm64" "-I" "include" "-Wall" "-Wextra" "-pedantic" "-pedantic-errors" "-Wall" "-Wextra" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wenum-compare" "-Wfloat-equal" "-Wformat=2" "-Winline" "-Winvalid-pch" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wundef" "-Wuninitialized" "-Wwrite-strings" "-fno-strict-aliasing" "-fvisibility=hidden" "-fstack-protector" "-gfull" "-DNDEBUG" "-c" "-o/Users/runner/work/rustup/rustup/target/aarch64-apple-darwin/release/build/ring-fcf530fe8716448c/out/aesv8-armx-linux64.o" "/Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S"
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:18:17: error: unexpected token in '.section' directive
  .section .rodata
                  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:28:1: error: unknown directive
  .hidden GFp_aes_hw_set_encrypt_key
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:29:1: error: unknown directive
  .type GFp_aes_hw_set_encrypt_key,%function
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:161:1: error: unknown directive
  .size GFp_aes_hw_set_encrypt_key,.-GFp_aes_hw_set_encrypt_key
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:163:1: error: unknown directive
  .hidden GFp_aes_hw_encrypt
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:164:1: error: unknown directive
  .type GFp_aes_hw_encrypt,%function
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:191:1: error: unknown directive
  .size GFp_aes_hw_encrypt,.-GFp_aes_hw_encrypt
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:193:1: error: unknown directive
  .hidden GFp_aes_hw_decrypt
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:194:1: error: unknown directive
  .type GFp_aes_hw_decrypt,%function
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:221:1: error: unknown directive
  .size GFp_aes_hw_decrypt,.-GFp_aes_hw_decrypt
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:223:1: error: unknown directive
  .hidden GFp_aes_hw_ctr32_encrypt_blocks
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:224:1: error: unknown directive
  .type GFp_aes_hw_ctr32_encrypt_blocks,%function
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:403:1: error: unknown directive
  .size GFp_aes_hw_ctr32_encrypt_blocks,.-GFp_aes_hw_ctr32_encrypt_blocks
  ^
  /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/pregenerated/aesv8-armx-linux64.S:407:19: error: unexpected token in '.section' directive
  .section .note.GNU-stack,"",%progbits
                    ^
  thread 'main' panicked at 'execution failed', /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-0.16.15/build.rs:664:9

cc @shepmaster

@est31
Copy link
Author

est31 commented Oct 17, 2020

Pr with a fix: #1055

@briansmith
Copy link
Owner

I'm working on merging recent BoringSSL changes that are relevant to this and then I'll deal with #1055.

@briansmith
Copy link
Owner

I see rust-lang/rust#73628 says that even rustc doesn't have support and the final target triple/triples isn't/aren't decide yet. I think it would be great if we could go over to rust-lang/rust#73628 and nail down at least what the final target triple names will be. (I think the default answer should be "whatever clang uses".)

In that issue, I saw an interesting comment:

arm64e is currently required when building kernel modules for macOS on Apple Silicon machines, but arm64e binaries will not run in userspace (currently arm64, or running x86_64 under Rosetta emulation is required).

I have a WIP to add arm64e support to ring based on merging BoringSSL's arm64e support. If arm64e support is mandatory for kernel space and forbidden from userspace then we'll almost definitely need to support both.

@briansmith
Copy link
Owner

@chadseld Any thoughts on ^?

@briansmith
Copy link
Owner

The PR in #1055 is based on the assumption that any assembly code compiled for Aarch64 iOS devices will work unmodified (modulo BTI and pointer authentication) on ARM64 macOS. Are there any (other) ABI differences at all?

@briansmith
Copy link
Owner

Well, I see the current rustc nightly uses the name aarch64-apple-darwin

@briansmith
Copy link
Owner

My understanding is that there is no emulator so one can only run a binary produced for this target on the actual hardware. Is that correct?

What is the status of cross-compiling to this target? Would it be a waste of time to try to get the CI/CD going to verify that things still build?

@est31
Copy link
Author

est31 commented Oct 20, 2020

I think it would be great if we could go over to rust-lang/rust#73628 and nail down at least what the final target triple names will be.

A better thread for the apple silicon target would be rust-lang/rust#73908 . Regarding the particular concern of ARM 8.3 version numbers, there is precedent with x86_64 where core2 is assumed for apple based targets, allowing for max_atomic_width to be set to 128 bits instead of 64. linux based targets on the other hand can only assume 64 bits. The names of the targets (x86_64-apple-darwin vs x86_64-unknown-linux-gnu) don't reflect this difference in minimum hardware version, it's solely implied by the fact that apple doesn't support pre-core2 CPUs.

@est31
Copy link
Author

est31 commented Oct 20, 2020

What is the status of cross-compiling to this target? Would it be a waste of time to try to get the CI/CD going to verify that things still build?

Officially, Xcode 12 supports cross compilation to apple silicon. Things seem to be a bit in flux still though. Rust CI uses a beta of Xcode 12.2 for example. Best ask @shepmaster for the details.

@chadseld
Copy link

I've been using aarch64-apple-darwin for building rust from source. That is now officially the rust tier 2 platform. Yes, you should be able to cross compile, the same as when building for iOS. But I have not attempted this. I know of no emulator appropriate for CI testing. As for this compilation failure, https://github.com/briansmith/ring/pull/1055/files fixes it. I'd prefer a better ASM_TARGETS triple. It feels dirty to use ios64 there since mac on arm is going to be a major platform going forward, but that's entirely internal to ring build scripts.

@briansmith
Copy link
Owner

I'd prefer a better ASM_TARGETS triple. It feels dirty to use ios64 there since mac on arm is going to be a major platform going forward, but that's entirely internal to ring build scripts.

I'm working on that. We need to get agreement with BoringSSL (and I hope OpenSSL upstream) to make it easy to maintain. The paperwork necessary for 1Password to contribute to BoringSSL is in progress.

@briansmith
Copy link
Owner

BoringSSL upstream has fixed the conditional compilation logic that was blocking the BoringSSL merge: https://boringssl.googlesource.com/boringssl/+/c46b1736a14135cc5b337aad3f359c819963ac68. Will finish the BoringSSL merge and then test and merge #1055.

@briansmith
Copy link
Owner

openssl/openssl#12254 is the upstream OpenSSL issue. I see in the PR they merged, https://github.com/openssl/openssl/pull/12591/files, that they are using ios64 as the perlasm flavour name. Unless/until BoringSSL upstream changes that, I'll continue using ios64 as the perlasm flavour name too.

@briansmith
Copy link
Owner

I had to redo the CI system to migrate off of Travis CI as adding jobs for aarch64-apple-darwin would have made an already-excruciating wait time even slower. That is now mostly done in PR #1083. Now I'm redoing the feature detection logic in cpu.rs to make it easier to understand and maintain, to add some smoke tests for it, to fix a bug in it, and to make it easier to add the aarch64-apple-darwin support correctly. As a prerequisite to that, I added some configurations to the automated testing in GitHub, wasm32-unknown-unknown in particular, to ensure that the cleanup and fixing in cpu.rs doesn't break anything that is currently supported.

I've also set up a x86-64 Mac to allow me to do the cross-compilation to verify that this builds. I've also ordered an ARM Mac that will allow me to do pre-release tests (at least) until GitHub Actions can add ARM Mac runners.

I expect all the prerequisite work mentioned above to be complete in the next day or two, and I expect to issue a new release with the aarch64-apple-darwin support by EoW. Note that I won't actually have an ARM Mac to run the tests on before that release so I would appreciate some help with the testing.

@chadseld
Copy link

You can ping me and I can test on the DTK, or provide remote access.

@digitalresistor
Copy link

I've got an Apple MacBook Pro with the M1 sitting in front of me. Let me know how I can help out.

@briansmith
Copy link
Owner

I also have an M1 now. Currently I am dealing with the code signing issue that I didn't expect. Then I need to verify that the fix to the CPU-feature-based dispatching logic is working and then we should be good to go.

@digitalresistor
Copy link

@briansmith https://github.com/shepmaster/rust/blob/silicon/silicon/README.md

RUSTC=$(rustup which rustc) $(rustup which cargo) <cargo command>

Worked for me in some other projects (and to install ripgrep). So long as you have installed the toolchain for aarch64. Maybe that will help?

@briansmith
Copy link
Owner

With PR #1100 (merged) and PR #1101 (pending) I am able to cargo test and pass all the tests on my M1 mac mini. PTAL at both of those as they are non-trivial.

Originally I was expecting to use @chadseld's PR #1055. I will comment in that PR about why I chose to do (a lot) more work to do things differently. However, I appreciate Chad's work on that PR; that was the key to me seeing that things needed to be improved.

@est31
Copy link
Author

est31 commented Nov 18, 2020

@briansmith thanks for your work on a fix. In order for me to be able to test it, it would be cool to have something available that I can point cargo at. As git is not supported by ring, could you maybe make a pre-release on crates.io? Like a 0.17.0-alpha.5 release?

@briansmith
Copy link
Owner

@briansmith thanks for your work on a fix. In order for me to be able to test it, it would be cool to have something available that I can point cargo at. As git is not supported by ring, could you maybe make a pre-release on crates.io? Like a 0.17.0-alpha.5 release?

I am working on rebasing 0.17-alpha on top of PR #1100 and PR #1101 and verifying that the important project that I know is using 0.17.0-alpha works. Once I've done that, I'll publish the 0.16 and 0.17-alpha releases on GitHub.

In the meantime, I recommend people look over the recent PRs, especailly #1097, #1099, #1100, and #1101.

@briansmith
Copy link
Owner

ring 0.16.16 is now published. Please try it out.

@briansmith
Copy link
Owner

Reopening this because I found some additional work to do.

PR #1105 bumps some of the minimum versions for dependencies of ring so that if somebody does cargo update -p ring but not a full cargo update, they will get versions of those dependencies that are (apparently, according to others) more likely to work.

FWIW, I also released 0.17.0-alpha.5 but I recommend using 0.16.16 over it. (I hope to have a final 0.17.0 at the end of the month.)

@est31
Copy link
Author

est31 commented Nov 19, 2020

Please try it out.

It works like a charm in the rustup CI, but note it only tests the build, nothing beyond that. rust-lang/rustup#2517

@citruz
Copy link

citruz commented Nov 19, 2020

Can confirm that 0.16.16 compiles correctly on my M1 machine. I am using the pbkdf2 functionality and it produces correct results.

@briansmith
Copy link
Owner

I am curious what version of Xcode people are using.

I initially was using XCode 12.3 beta, kind of by accident, and everything worked fine.
However, I tried to use XCode 12.2 final release and the build started to fail, starting with libc's build script, apparently because of the codesigning issue.

@briansmith
Copy link
Owner

After running https://gist.github.com/briansmith/dd06e8dfeac46d1032d974000dac1fa6 I am now able to get things working with Xcode 12.2, not just Xcode 12.3 beta.

@digitalresistor
Copy link

I have Xcode 12.2 installed from the App Store, nothing else. No betas. I was testing building zola (static site generator), which built successfully. I haven't had a chance to test if it builds my site correctly, but generally it building is a good sign.

@shepmaster
Copy link

shepmaster commented Nov 21, 2020

I am curious what version of Xcode people are using.

Rust uses Xcode 12.2

@briansmith
Copy link
Owner

Fixed in ring 0.16.16. You should use 0.16.17 though, because it has some compatibility fixes.

raboof added a commit to raboof/gbl that referenced this issue May 28, 2022
My main motivation was to fix the build on darwin-aarch64 (Mac M1,
briansmith/ring#1063)

Note that I don't really have any particular expertise here, I just
made the 'mechanical' change and made the changes needed to make
the compiler and the tests happy.
raboof added a commit to raboof/gbl that referenced this issue May 28, 2022
My main motivation was to fix the build on darwin-aarch64 (Mac M1,
briansmith/ring#1063)

Note that I don't really have any particular expertise here, I just
made the 'mechanical' change and made the changes needed to make
the compiler and the tests happy.
raboof added a commit to raboof/nixpkgs that referenced this issue Jan 4, 2023
Compilation failed due to an old transitive dependency on 'ring',
briansmith/ring#1063.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants