Skip to content

Commit d6aa1fb

Browse files
committed
[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.
1 parent 2390135 commit d6aa1fb

3 files changed

Lines changed: 92 additions & 1 deletion

File tree

src/modm/math/utils/crc.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ crc8_ccitt_update(uint8_t crc, uint8_t data)
3131
data ^= crc;
3232
for (uint8_t ii = 0; ii < 8; ii++)
3333
{
34+
const bool msb = (data & 0x80) != 0;
3435
data <<= 1;
35-
if (data & 0x80) data ^= 0x07;
36+
if (msb) data ^= 0x07;
3637
}
3738
return data;
3839
#endif

test/modm/math/utils/crc_test.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright (c) 2026, Tudor Fanaru
3+
*
4+
* This file is part of the modm project.
5+
*
6+
* This Source Code Form is subject to the terms of the Mozilla Public
7+
* License, v. 2.0. If a copy of the MPL was not distributed with this
8+
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
9+
*/
10+
// ----------------------------------------------------------------------------
11+
12+
#include <modm/math/utils/crc.hpp>
13+
14+
#include "crc_test.hpp"
15+
16+
namespace
17+
{
18+
// "123456789" is the input the CRC catalogue publishes check values against,
19+
// so the expectations below are external references.
20+
const uint8_t check_input[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
21+
}
22+
23+
void
24+
CrcTest::testCrc8CcittCheckValue()
25+
{
26+
// CRC-8 with polynomial 0x07 and init 0xFF.
27+
TEST_ASSERT_EQUALS(modm::math::crc8_ccitt(check_input, sizeof(check_input)), 0xFBU);
28+
}
29+
30+
void
31+
CrcTest::testCrc8CcittUsesEveryInputBit()
32+
{
33+
// Regression: shifting before testing bit 7 discards it, leaving the MSB of
34+
// every input byte unexamined. That made crc8_ccitt_update(crc, d) equal
35+
// crc8_ccitt_update(crc, d ^ 0x80) for all 32768 (crc, d) pairs, put only
36+
// 128 of 256 outputs in reach, and left single-bit errors in that position
37+
// undetectable.
38+
for (uint16_t crc = 0; crc < 256; ++crc)
39+
{
40+
for (uint16_t data = 0; data < 128; ++data)
41+
{
42+
const uint8_t a = modm::math::crc8_ccitt_update(uint8_t(crc), uint8_t(data));
43+
const uint8_t b = modm::math::crc8_ccitt_update(uint8_t(crc), uint8_t(data ^ 0x80));
44+
TEST_ASSERT_TRUE(a != b);
45+
}
46+
}
47+
}
48+
49+
void
50+
CrcTest::testCrc16CcittCheckValue()
51+
{
52+
// CRC-16/MCRF4XX: reflected polynomial 0x8408, init 0xFFFF, no final xor.
53+
TEST_ASSERT_EQUALS(modm::math::crc16_ccitt(check_input, sizeof(check_input)), 0x6F91U);
54+
}
55+
56+
void
57+
CrcTest::testCrc32CheckValue()
58+
{
59+
// CRC-32/ISO-HDLC, as used by zlib and PNG.
60+
TEST_ASSERT_EQUALS(modm::math::crc32(check_input, sizeof(check_input)), 0xCBF43926UL);
61+
}

test/modm/math/utils/crc_test.hpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright (c) 2026, Tudor Fanaru
3+
*
4+
* This file is part of the modm project.
5+
*
6+
* This Source Code Form is subject to the terms of the Mozilla Public
7+
* License, v. 2.0. If a copy of the MPL was not distributed with this
8+
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
9+
*/
10+
// ----------------------------------------------------------------------------
11+
12+
#include <unittest/testsuite.hpp>
13+
14+
/// @ingroup modm_test_test_math
15+
class CrcTest : public unittest::TestSuite
16+
{
17+
public:
18+
void
19+
testCrc8CcittCheckValue();
20+
21+
void
22+
testCrc8CcittUsesEveryInputBit();
23+
24+
void
25+
testCrc16CcittCheckValue();
26+
27+
void
28+
testCrc32CheckValue();
29+
};

0 commit comments

Comments
 (0)