aboutsummaryrefslogtreecommitdiff
path: root/crypto/crypto_test.cc
diff options
context:
space:
mode:
authorDavid Benjamin <davidben@google.com>2024-05-24 15:31:28 -0400
committerBoringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>2024-05-24 21:44:57 +0000
commitafd52e91dfed27ab7193be040f067900947b14ac (patch)
treed299c683299da0063c5d73af2c6b2a20c4c6cdd4 /crypto/crypto_test.cc
parente95b0cad901abd49755d2a2a2f1f6c3e87d12b94 (diff)
downloadboringssl-afd52e91dfed27ab7193be040f067900947b14ac.zip
boringssl-afd52e91dfed27ab7193be040f067900947b14ac.tar.gz
boringssl-afd52e91dfed27ab7193be040f067900947b14ac.tar.bz2
Fix some enum issues in the test-only BORINGSSL_FIPS_COUNTERS build
This fixes b/342634459. enums are weird. They're neither tight sum types, nor ints with designated values but something in between. By default (i.e. if you do not say `enum Foo : int`, which isn't in C until very new versions), an enum's range of valid values is the smallest hypothetical-bit-width signed or unsigned integer that can fit all the values. So, the following enum can only hold 0, 1, 2, or 3, and all values outside that range are UB. enum E { A = 0, B = 2, } Meanwhile, enum is signed and can hold values -2, -1, 0, 1: enum E { A = -1, B = 1, } This is incredibly bizarre. This has two consequences. First, clang emits a warning like the following: .../boringssl/crypto/fipsmodule/self_check/fips.c:75:15: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero -compare] 75 | if (counter < 0 || counter > fips_counter_max) { | ~~~~~~~ ^ ~ Second, this loop over enum values in the unit test trips UBSan. [----------] 1 test from CryptoTest [ RUN ] CryptoTest.FIPSCountersEVP_AEAD .../boringssl/crypto/crypto_test.cc:51:8: runtime error: load of value 4, which is not a valid value for type 'fips_counter_t' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/davidben/boringssl/crypto/crypto_test.cc:51:8 To avoid this, just do our arithemetic in integer space and only move to enums at the edges. Change-Id: Icbd0e41604ce2aa67be8d394b03c7de50062199c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68768 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com>
Diffstat (limited to 'crypto/crypto_test.cc')
-rw-r--r--crypto/crypto_test.cc10
1 files changed, 3 insertions, 7 deletions
diff --git a/crypto/crypto_test.cc b/crypto/crypto_test.cc
index 4543d5c..7be370b 100644
--- a/crypto/crypto_test.cc
+++ b/crypto/crypto_test.cc
@@ -47,18 +47,14 @@ TEST(CryptoTest, Strndup) {
using CounterArray = size_t[fips_counter_max + 1];
static void read_all_counters(CounterArray counters) {
- for (fips_counter_t counter = static_cast<fips_counter_t>(0);
- counter <= fips_counter_max;
- counter = static_cast<fips_counter_t>(counter + 1)) {
- counters[counter] = FIPS_read_counter(counter);
+ for (int counter = 0; counter <= fips_counter_max; counter++) {
+ counters[counter] = FIPS_read_counter(static_cast<fips_counter_t>(counter));
}
}
static void expect_counter_delta_is_zero_except_for_a_one_at(
CounterArray before, CounterArray after, fips_counter_t position) {
- for (fips_counter_t counter = static_cast<fips_counter_t>(0);
- counter <= fips_counter_max;
- counter = static_cast<fips_counter_t>(counter + 1)) {
+ for (int counter = 0; counter <= fips_counter_max; counter++) {
const size_t expected_delta = counter == position ? 1 : 0;
EXPECT_EQ(after[counter], before[counter] + expected_delta) << counter;
}