No, that file's supposed to be built in this configuration. That's just how OpenSSL names the files.

I'll take a look at it on Monday. Though for the trybot situation you'll need to talk to Cronet folks.

https://cs.chromium.org/chromium/src/third_party/boringssl/linux-arm/crypto/chacha/chacha-armv4.S?q=chacha-armv4.S+package:%5Echromium$&dr&l=12

#if defined(__thumb2__) || defined(__clang__)
#define ldrhsb	ldrbhs
#endif

So that file gets built with clang, with that define, but we then send it to the system assembler which doesn't understand this replacement.

clang + arm assembly is (or used to be, at least) a mess (which is why we pass -fno-integrated-as), but boringssl tries to make it work but mostly gets it wrong (by assuming that clang == ios, etc).

No, anything specific to iOS in BoringSSL assembly is not going to be done by preprocessor stuff like that. That's going to be handled by the horror that is perlasm. That code is, I believe, actually trying to detect clang's assembler. (Though remember this assembly is third-party code. It comes from upstream OpenSSL.)

We and upstream do need to support clang's integrated assembler for iOS, so just taking those out won't work. Does clang not have a way for the preprocessor to tell whether it's using the integrated assembler? That sounds like a problem clang will need to fix so long as its assembler is flaky enough to need a -fno-integrated-as flag.

Another possibility is to just switch to the integrated assembler. I tried that recently and it *almost* worked. We might be able to finish it up?
https://chromium-review.googlesource.com/c/529565/

But I'm confused why this is only starting to break now. This is not the first time BoringSSL on ARM is being built with clang and -fno-integrated-as.

I just enabled clang by default on Android. Most bots are happy, this one isn't. I think it's because that bot sets arm_version=6. Maybe I can stop passing -fno-integrated-as if arm_version==6 to unbreak the bot.

Looks like your thing only failed on cros, which doesn't use our hermetic clang.

…oh, also you deleted the -B bit, we do always need that.

I'm trying the arm_version=6 check at https://codereview.chromium.org/2932043004/ . Maybe it'll also need a !is_chromeos or something.

Err, if we tell clang to not shell out to gas, we do not need -B. Sorry. Also sorry about all the emails.

Also checked that https://bugs.llvm.org//show_bug.cgi?id=23000#c14 is still an issue for arm-v7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4fb1dcc42b4f733129e9c3dfba2add43c25d740

commit a4fb1dcc42b4f733129e9c3dfba2add43c25d740
Author: thakis <thakis@chromium.org>
Date: Sun Jun 11 00:01:35 2017

clang/arm: Use integrated assembler for boringssl for arm_version 6.

An arm_version 6 file assumes that __clang__ means that clang's assembler is
being used, while it only really identifies clang is used as preprocessor.

Luckily, it looks like only the armv7 assembly code in boringssl is incompatible
with clang's built-in assembler.

BUG= 732066 ,124610
TBR=davidben

Review-Url: https://codereview.chromium.org/2932043004
Cr-Commit-Position: refs/heads/master@{#478524}

[modify] https://crrev.com/a4fb1dcc42b4f733129e9c3dfba2add43c25d740/third_party/boringssl/BUILD.gn

Good enough for now (https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20ARMv6%20Builder/builds/5007).

I somewhat suspect the bsaes issue can be resolved by swapping all (some of?) those __APPLE__ checks to __clang__. There were other cases where upstream made that swap, but not those.

Though I guess this bug suggests that it may, in turn, trigger this bug where the preprocessor does not properly report if -fno-integerated-as is being used. Perhaps we need another BORINGSSL_CLANG_SUPPORTS_DOT_ARCH-like define.

I haven't gotten clang (Android NDK r15) to be happy with bsaes-armv7.pl (I think I now understand why upstream doesn't build all its files in Thumb mode...), but I believe this will fix the -fno-integrated-as incompatibility, and has some precedent so hopefully upstream will approve:

https://github.com/openssl/openssl/pull/3694

Restoring a -fno-integrated-as is, of course, not the direction we ultimately want to go, but this way we have more options if further problems crop up. We got kinda lucky with r478524 that the problem was isolated to ports where the integrated assembler mattered.

Do you know why they don't do .syntax unified unconditionally? I thought that has been working in gcc for a decade or so.

Beats me. They love to support truly ancient assemblers, but I'll ask.

The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl/+/3c9729212be5d30ea5ab1d4007842ec527125342

commit 3c9729212be5d30ea5ab1d4007842ec527125342
Author: David Benjamin <davidben@google.com>
Date: Wed Jun 28 13:35:29 2017

Fix chacha-armv4.pl with clang -fno-integrated-as.

The __clang__-guarded #defines cause gas to complain if clang is passed
-fno-integrated-as. Emitting .syntax unified when those are used fixes
this. This matches the change made to ghash-armv4.pl in upstream's
6cf412c473d8145562b76219ce3da73b201b3255.

See also https://github.com/openssl/openssl/pull/3694. This fixes the
build with the latest Android NDK (use the NDK-supplied toolchain file)
with the armeabi ABI.

Bug:  chromium:732066 
Change-Id: Ic6ca633a58edbe8ae8c7d501bd9515c2476fd7c2
Reviewed-on: https://boringssl-review.googlesource.com/17404
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

[modify] https://crrev.com/3c9729212be5d30ea5ab1d4007842ec527125342/crypto/chacha/asm/chacha-armv4.pl