Skip to content

Android arm64 build is broken on boringssl #24321

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

Closed
zanderso opened this issue Sep 9, 2015 · 12 comments
Closed

Android arm64 build is broken on boringssl #24321

zanderso opened this issue Sep 9, 2015 · 12 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@zanderso
Copy link
Member

zanderso commented Sep 9, 2015

$ ./tools/build.py -m release -a arm64 --os=android runtime
...
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S: Assembler messages:
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:28: Error: unknown pseudo-op: `.syntax'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:29: Error: unknown cpu `cortex-a8'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:30: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:40: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:43: Error: unknown pseudo-op: `.fpu'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:44: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:45: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:46: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:47: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:48: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:49: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:50: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:51: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:52: Error: unknown pseudo-op: `.eabi_attribute'
third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S:53: Error: unknown pseudo-op: `.thumb'
chacha_vec.c:59: Error: unknown pseudo-op: `.thumb'
chacha_vec.c:60: Error: unknown pseudo-op: `.thumb_func'
chacha_vec.c:63: Error: junk at end of line, first unrecognized character is `@'
chacha_vec.c:64: Error: junk at end of line, first unrecognized character is `@'
chacha_vec.c:65: Error: unknown mnemonic `push' -- `push {r4,r5,r6,r7,r8,r9,r10,fp,lr}'
chacha_vec.c:66: Error: operand 1 should be an integer register -- `mov r8,r3'

Etc.

@zanderso
Copy link
Member Author

zanderso commented Sep 9, 2015

I have tried updating our android_tools, but the same error is produced by the newest version.

@mit-mit mit-mit added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. Type-Defect labels Sep 11, 2015
@mit-mit mit-mit added this to the 1.13 milestone Sep 11, 2015
@sgjesse
Copy link
Contributor

sgjesse commented Sep 11, 2015

This is most likely caused by the BoringSSL ARM32 assembly code being used on ARM64 as well.

If BoringSSL does not have ARM64 assembly implementation we should use the generic C implementation.

@whesse
Copy link
Contributor

whesse commented Sep 11, 2015

I have filed an issue with BoringSSL to ask if that is the case. It seems likely, but they actually specifically enable this file for aarch64 in the guards. But this might have been an oversight. There are also errors in the c file, though, so it is not just a matter of changing this guard.

Here is the BoringSSL issue (which is public) that I filed. I'll try and get the exact compiler lines to them as well. https://code.google.com/p/chromium/issues/detail?id=530543

@whesse
Copy link
Contributor

whesse commented Oct 5, 2015

@zanderso : Could you try removing the "|| defined(aarch64)" from the guard in line 26 of chacha_vec_arm.S, and see if the compilation works? I think this might be sufficient to make it work.

The file chacha_vec_arm.S is not included in Chromium builds on aarch64, because Chromium specifies different lists of files to include for each architecture, and doesn't include it for arm64. We include all files for all architectures, and rely on guards in the files to disable them for architectures they don't apply to. This may just be a mistake in the guard.

@whesse whesse removed this from the 1.13 milestone Oct 5, 2015
@zanderso
Copy link
Member Author

zanderso commented Oct 5, 2015

The build succeeds after removing that.

@whesse
Copy link
Contributor

whesse commented Oct 5, 2015

Great - I have submitted the change upstream. We can't change it locally, since the file with the problem is in our checkout of the boringssl repo. We could work around it, but I think it will be sufficient to get this once it is upstreamed.

On Mon, Oct 5, 2015 at 5:17 PM, Zachary Anderson [email protected]
wrote:

The build succeeds after removing that.


Reply to this email directly or view it on GitHub
#24321 (comment).

William Hesse

@rmacnak-google
Copy link
Contributor

It looks like this change was landed upstream, but we're still on an old version of boringssl. In fact, it looks like we have never update boringssl since it landed. How often do we expect to update it?

@whesse
Copy link
Contributor

whesse commented Jan 5, 2016

Our update schedule will be limited by Chrome's update schedule, since the
gyp build of BoringSSL that we use is coming from them. But they have
updated recently, to a Dec 18 2015 version of BoringSSL, so we can update
to that one.

I expect we will update it somewhere between twice a year and monthly. We
don't have a formal plan or schedule for doing it - perhaps we should
establish one.

On Tue, Jan 5, 2016 at 12:49 AM, Ryan Macnak [email protected]
wrote:

It looks like this change was landed upstream, but we're still on an old
version of boringssl. In fact, it looks like we have never update boringssl
since it landed. How often do we expect to update it?


Reply to this email directly or view it on GitHub
#24321 (comment).

William Hesse

@iposva-google
Copy link
Contributor

While you are figuring out the ongoing schedule I suggest that you roll to the latest from Chrome now. This will likely solve at least some of the issues we are seeing with the switch to boringssl. Thanks!

@whesse
Copy link
Contributor

whesse commented Feb 2, 2016

I have a CL that rolls to the lastest BoringSSL from Chrome, which is Dec
18. It compiles and runs (with patches), without error on linux x64 and
ia32.
I will do more testing, but the patches need to be upstreamed, and I will
roll to tip-of-tree, to make it compile without patches.

I think we could do a patch in Dart, by copying the file to our local
assembly directories in third_party/boring.ssl, removing the original from
the sources in boringssl.gypi,
modifying the copy, and adding it to boringssl.gypi, but it seems better to
upstream it.

The Dart CL is https://codereview.chromium.org/1653973006/

The patch is https://boringssl-review.googlesource.com/#/c/7030/

On Mon, Jan 25, 2016 at 8:20 PM, Ivan Posva [email protected]
wrote:

While you are figuring out the ongoing schedule I suggest that you roll to
the latest from Chrome now. This will likely solve at least some of the
issues we are seeing with the switch to boringssl. Thanks!


Reply to this email directly or view it on GitHub
#24321 (comment).

William Hesse

@kevmoo kevmoo added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed Type-Defect labels Mar 1, 2016
@whesse
Copy link
Contributor

whesse commented Jun 14, 2016

The fix is still in the pending BoringSSL roll, which is delayed on C11 support. The issue tracking this is
#26367 . We need to update from the compilers in ubuntu precise to those in ubuntu trusty, but this also rolls our glibc requirements.

@zanderso
Copy link
Member Author

We rolled BoringSSL, so I think we can close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

8 participants