After I backed out mock yesterday we started getting messages like this on the Linux 64 builds: intel-gcm.s: Assembler messages: intel-gcm.s:39: Error: no such instruction: `vmovdqu (Tp),T' intel-gcm.s:40: Error: no such instruction: `vpshufb .Lbswap_mask(%rip),T,T' intel-gcm.s:41: Error: no such instruction: `vpxor TMP0,TMP0,TMP0' intel-gcm.s:44: Error: no such instruction: `vpinsrq $0,Mlen,TMP0,TMP0' intel-gcm.s:45: Error: no such instruction: `vpinsrq $1,Alen,TMP0,TMP0' I'm pretty sure the NSS upgrade we took is what tripped this: https://hg.mozilla.org/releases/mozilla-esr17/rev/747c40b60d2b I also found bug 838401, which has SeaMonkey builders hitting the exact same errors. Their fix was to take a binutils upgrade. Not sure how risky that is though.

The other obvious option is to revert the NSS upgrade. Is that an option, Alex?

(In reply to Ben Hearsum [:bhearsum] from comment #0) > After I backed out mock yesterday we started getting messages like this on > the Linux 64 builds: This was bug 813613.

Including those who will be able to help here, and following up in email.

For the record, we're using binutils 2.17.50.0.6 for these builds. (32-bit Linux uses the exact same version, too, which is curious.)

Another option is to patch nss to remove hardware acceleration

(In reply to Ben Hearsum [:bhearsum] from comment #4) > For the record, we're using binutils 2.17.50.0.6 for these builds. (32-bit > Linux uses the exact same version, too, which is curious.) It's not curious, the code that fails to build is x86-64-only.

(In reply to Mike Hommey [:glandium] from comment #5) > Another option is to patch nss to remove hardware acceleration I remember when we made these changes that we had to upgrade binutils (?) on the NSS tinderbox builders too. If the Intel AES-GCM code is the only code that is causing a problem, and it is too difficult to upgrade binutils on the builders for this machine, then we should patch NSS to revert the patches that add the AES-GCM acceleration. If that is the course of action that we'd like to take, then let's file a bug for that in the NSS:build component.

I think it would be more reasonable to revert AES-GCM acceleration than to change the toolchain on ESR.

(In reply to Mike Hommey [:glandium] from comment #8) > I think it would be more reasonable to revert AES-GCM acceleration than to > change the toolchain on ESR. That being said, it would be better if NSS allowed to disable it with a compile flag.

I know too little about this/asm to identify what is and isn't a related problem here. But other bugs relating to this same "stuff" that were filed: Bug 835293 - Build failures: intel-gcm.s Error: `foo` is not supported on `opteron' Bug 835176 - mozilla-central ASan builds broken in nss/lib/freebl/intel-gcm.o by NSS_3_14_2_BETA2 Bug 835050 - intel-gcm.s fails to compile with clang 3.2 Bug 805604 - Efficient AES-GCM implementation that uses Intel's AES and PCLMULQDQ instructions (AES-NI) and the Advanced Vector Extension (AVX) architecture.

(In reply to Mike Hommey [:glandium] from comment #9) > (In reply to Mike Hommey [:glandium] from comment #8) > > I think it would be more reasonable to revert AES-GCM acceleration than to > > change the toolchain on ESR. > > That being said, it would be better if NSS allowed to disable it with a > compile flag. The NSS team already decided explicitly not to add the flag. I am also against adding the flag. The code already does runtime detection. AFAICT, binutils 2.17.50.0.6 is six years old. It is not reasonable to create new, never-tested, configurations of NSS to support such old tools. So, it is OK to do a one-off revert of the AES-GCM code for now, but we should make effort to upgrade to a compatible version of binutils on the ESR branch before we add TLS 1.2 support to ESR. Otherwise the ESR branch would be using a completely untested configuration of the AES-GCM code that provides the very security fix that would warrant backporting TLS 1.2 to ESR in the first place.

(In reply to Brian Smith (:bsmith) from comment #11) > we > should make effort to upgrade to a compatible version of binutils on the ESR > branch before we add TLS 1.2 support to ESR. Otherwise the ESR branch would > be using a completely untested configuration of the AES-GCM code that > provides the very security fix that would warrant backporting TLS 1.2 to ESR > in the first place. We were working towards upgrading to newer tools on ESR. I just WONTFIX'ed that bug though, because we continually have regression that cause compatibility changes (bug 813613), which we're not allowed to have on minor ESR versions.

(In reply to Ben Hearsum [:bhearsum] from comment #12) > We were working towards upgrading to newer tools on ESR. I just WONTFIX'ed > that bug though, because we continually have regression that cause > compatibility changes (bug 813613), which we're not allowed to have on minor > ESR versions. Lots of sites (including all Mozilla sites, and I think Google and other big websites) are going to be working on enabling TLS 1.2 and AES-GCM cipher suites and making them the preferred ones. Unfortunately, without the AES-GCM assembler code, the AES-GCM cipher suites are much slower than the RC4 cipher suites that these sites will have previously been using. So, our choices long-term are: 1. Don't offer TLS 1.2 support in ESR, increasing exposure to RC4 risks, risking compatibility issues. 2. Backport TLS 1.2 support, but disable the AES-GCM code, and accept a performance regression on ESR for what will soon be many websites. 3. Figure out a way to upgrade the assembler in an acceptable way. 4. Figure out a way to change the AES-GCM assembler code so that it builds with the old assembler. For the current release, and any release prior to us backporting TLS 1.2 to ESR, we can just revert the AES-GCM patches. CC me on the NSS bug if/when it gets filed. I do not know/understand what compatibility issues were encountered with the upgraded binutils. I don't think an upgraded binutils should cause ABI issues. Are the compatibility issues purely related to build-time compatibility issues? If so, we should just not promise build-time compatibility for ESR. It is reasonable to ask Linux distros to use a non-default set of tools to build Firefox, just like we do on Windows.

(In reply to Mike Hommey [:glandium] from comment #8) > I think it would be more reasonable to revert AES-GCM acceleration than to > change the toolchain on ESR. Agreed, especially since changing build configurations is what caused us to realize this later than we would have liked.

(In reply to Brian Smith (:bsmith) from comment #13) > 1. Don't offer TLS 1.2 support in ESR, increasing exposure to RC4 risks, > risking compatibility issues. That's certainly not appealing. > 2. Backport TLS 1.2 support, but disable the AES-GCM code, and accept a > performance regression on ESR for what will soon be many websites. Define "soon". For some value of soon, ESR17 is going to be unsupported "soon". Also, that code is only for 64-bits processors with AVX extensions that are only available in < 2 years old CPUs. I doubt the performance regression on these CPUs would matter more than the fact that older (and thus slower CPUs) are *not* using that code anyways. > 3. Figure out a way to upgrade the assembler in an acceptable way. > 4. Figure out a way to change the AES-GCM assembler code so that it builds > with the old assembler. Apart from including direct machine code, that's not possible.

Comment on attachment 730278 [details] [diff] [review] backout of aes-gcm Please document this patch in security/patches/README and add a copy of the patch to security/patches/

(In reply to Mike Hommey [:glandium] from comment #16) > (In reply to Brian Smith (:bsmith) from comment #13) > > 1. Don't offer TLS 1.2 support in ESR, increasing exposure to RC4 risks, > > risking compatibility issues. > > That's certainly not appealing. Thinking about it more, TLS 1.2 support will not be a security enhancement for quite some time (because we will let the attacker drop us down to TLS 1.0 or SSL 3.0 if he wants to). So, I don't think we'd backport TLS 1.2 support, as it is just a compatibility fix, which is our of scope for ESR. > Also, that code is only for 64-bits processors with AVX extensions > that are only available in < 2 years old CPUs. I doubt the performance > regression on these CPUs would matter more than the fact that older (and > thus slower CPUs) are *not* using that code anyways. Right. This is the issue that we'll need to resolve in general, not just for ESR.

Thanks for the quick turnaround all!

Bob: This is the bug I mentioned a couple of weeks ago, where we backed out the new Intel AES-GCM assembler optimization patch on the Firefox 17 branch because older versions of gas could not compile it.