From d6aa1fb24aeda8418ccb56a00bd1f6ee7c729ccd Mon Sep 17 00:00:00 2001 From: TudorFanaru18 Date: Wed, 29 Jul 2026 15:12:16 -0700 Subject: [PATCH] [math] Fix crc8_ccitt discarding the MSB of every input byte The non-AVR path transliterates the avr-libc inline assembly literally: 1: lsl %0 ; shift left, MSB -> CARRY brcc 2f ; branch on CARRY, i.e. the *pre-shift* MSB eor %0, %2 `lsl` preserves the departing bit in the carry flag, so `brcc` tests bit 7 of the value before the shift. C has no carry flag: `data <<= 1` on a uint8_t discards that bit, so the following `if (data & 0x80)` tests the pre-shift bit 6 instead and bit 7 is never examined. Over all 32768 (crc, data) pairs this made crc8_ccitt_update(crc, data) == crc8_ccitt_update(crc, data ^ 0x80) hold universally, left only 128 of 256 output values reachable, and made single-bit errors in bit 7 of any byte undetectable. Adds crc_test, which the module picks up by glob. There were no tests for this header, which is why the fault went unnoticed; the new suite pins the published check values for all three functions and asserts that flipping the MSB of an input byte changes the CRC-8. --- src/modm/math/utils/crc.hpp | 3 +- test/modm/math/utils/crc_test.cpp | 61 +++++++++++++++++++++++++++++++ test/modm/math/utils/crc_test.hpp | 29 +++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 test/modm/math/utils/crc_test.cpp create mode 100644 test/modm/math/utils/crc_test.hpp diff --git a/src/modm/math/utils/crc.hpp b/src/modm/math/utils/crc.hpp index 655f9754c2..faceab3269 100644 --- a/src/modm/math/utils/crc.hpp +++ b/src/modm/math/utils/crc.hpp @@ -31,8 +31,9 @@ crc8_ccitt_update(uint8_t crc, uint8_t data) data ^= crc; for (uint8_t ii = 0; ii < 8; ii++) { + const bool msb = (data & 0x80) != 0; data <<= 1; - if (data & 0x80) data ^= 0x07; + if (msb) data ^= 0x07; } return data; #endif diff --git a/test/modm/math/utils/crc_test.cpp b/test/modm/math/utils/crc_test.cpp new file mode 100644 index 0000000000..d93330fd86 --- /dev/null +++ b/test/modm/math/utils/crc_test.cpp @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2026, Tudor Fanaru + * + * This file is part of the modm project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +// ---------------------------------------------------------------------------- + +#include + +#include "crc_test.hpp" + +namespace +{ + // "123456789" is the input the CRC catalogue publishes check values against, + // so the expectations below are external references. + const uint8_t check_input[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'}; +} + +void +CrcTest::testCrc8CcittCheckValue() +{ + // CRC-8 with polynomial 0x07 and init 0xFF. + TEST_ASSERT_EQUALS(modm::math::crc8_ccitt(check_input, sizeof(check_input)), 0xFBU); +} + +void +CrcTest::testCrc8CcittUsesEveryInputBit() +{ + // Regression: shifting before testing bit 7 discards it, leaving the MSB of + // every input byte unexamined. That made crc8_ccitt_update(crc, d) equal + // crc8_ccitt_update(crc, d ^ 0x80) for all 32768 (crc, d) pairs, put only + // 128 of 256 outputs in reach, and left single-bit errors in that position + // undetectable. + for (uint16_t crc = 0; crc < 256; ++crc) + { + for (uint16_t data = 0; data < 128; ++data) + { + const uint8_t a = modm::math::crc8_ccitt_update(uint8_t(crc), uint8_t(data)); + const uint8_t b = modm::math::crc8_ccitt_update(uint8_t(crc), uint8_t(data ^ 0x80)); + TEST_ASSERT_TRUE(a != b); + } + } +} + +void +CrcTest::testCrc16CcittCheckValue() +{ + // CRC-16/MCRF4XX: reflected polynomial 0x8408, init 0xFFFF, no final xor. + TEST_ASSERT_EQUALS(modm::math::crc16_ccitt(check_input, sizeof(check_input)), 0x6F91U); +} + +void +CrcTest::testCrc32CheckValue() +{ + // CRC-32/ISO-HDLC, as used by zlib and PNG. + TEST_ASSERT_EQUALS(modm::math::crc32(check_input, sizeof(check_input)), 0xCBF43926UL); +} diff --git a/test/modm/math/utils/crc_test.hpp b/test/modm/math/utils/crc_test.hpp new file mode 100644 index 0000000000..8ce117087c --- /dev/null +++ b/test/modm/math/utils/crc_test.hpp @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2026, Tudor Fanaru + * + * This file is part of the modm project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +// ---------------------------------------------------------------------------- + +#include + +/// @ingroup modm_test_test_math +class CrcTest : public unittest::TestSuite +{ +public: + void + testCrc8CcittCheckValue(); + + void + testCrc8CcittUsesEveryInputBit(); + + void + testCrc16CcittCheckValue(); + + void + testCrc32CheckValue(); +};