Disable the __SHA__ static check for now
https://boringssl-review.googlesource.com/c/boringssl/+/64208 caused us
to skip the runtime check for SHA extensions when SHA is statically
available. This change was correct (though we don't get the size wins
without also emulating -ffunction-sections in the asm file), but we've
run into a few places where projects build with -march=goldmont, but
need a build that does not require SHA extensions:
- Some CrOS toolchain definitions are incorrect and build with
-march=goldmont when targetting boards that are not Goldmont. b/320482539
tracks fixing this.
- Sometimes projects build with -march=goldmont as a rough optimized
baseline. However, Intel CPU capabilities are not strictly linear, so
this does not quite work. Some combination of -mtune and
-march=x86-64-v{1,2,3,4} would be a better strategy here.
- QEMU versions before 8.2 do not support SHA extensions and disable it
with a warning. Projects that target Goldmont and test on QEMU will
break. The long-term fix is to update to 8.2. A principled short-term fix
would be -march=goldmont -mno-sha, to reflect that the binary needs to
run on both QEMU-8.1-Goldmont and actual-Goldmont.
Other than the CrOS issue, the others have been resolved or at least
worked around, so once the CrOS issue is fixed, and the changes made
their way through the pipeline, we can try this again. Though with the
QEMU issue, the longer we wait, the less likely we'll trip people
running old QEMUs.
Bug: b:320482539
Change-Id: Iab30a441fce313a1413a465f88d7d52cde179292
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65407
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h
index 1d9bcd0..25cb106 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -1453,11 +1453,27 @@
// SHA-1 and SHA-256 are defined as a single extension.
OPENSSL_INLINE int CRYPTO_is_x86_SHA_capable(void) {
-#if defined(__SHA__)
- return 1;
-#else
+ // We should check __SHA__ here, but for now we ignore it. We've run into a
+ // few places where projects build with -march=goldmont, but need a build that
+ // does not require SHA extensions:
+ //
+ // - Some CrOS toolchain definitions are incorrect and build with
+ // -march=goldmont when targetting boards that are not Goldmont. b/320482539
+ // tracks fixing this.
+ //
+ // - Sometimes projects build with -march=goldmont as a rough optimized
+ // baseline. However, Intel CPU capabilities are not strictly linear, so
+ // this does not quite work. Some combination of -mtune and
+ // -march=x86-64-v{1,2,3,4} would be a better strategy here.
+ //
+ // - QEMU versions before 8.2 do not support SHA extensions and disable it
+ // with a warning. Projects that target Goldmont and test on QEMU will
+ // break. The long-term fix is to update to 8.2. A principled short-term fix
+ // would be -march=goldmont -mno-sha, to reflect that the binary needs to
+ // run on both QEMU-8.1-Goldmont and actual-Goldmont.
+ //
+ // TODO(b/320482539): Once the CrOS toolchain is fixed, try this again.
return (OPENSSL_get_ia32cap(2) & (1u << 29)) != 0;
-#endif
}
// CRYPTO_cpu_perf_is_like_silvermont returns one if, based on a heuristic, the
