From 5df5469cc23fae59cbee358f4d21fcbefdde457f Mon Sep 17 00:00:00 2001 From: Robin Getz Date: Sat, 1 Aug 2026 14:49:19 -0400 Subject: [PATCH] leansdr : Fix undefined shift behavior in GF(2^N) arithmetic The gf2n implementation used a left shift on a potentially negative value when masking field elements: (~(Te)0) << N This can result in undefined behavior for signed integer types and was reported by static analysis (cppcheck). Require the field element type to be unsigned and replace the mask generation with an explicit N-bit mask constructed from the element type. Also make the overflow check use the element type to avoid implicit signed integer operations. Add comments documenting the packed polynomial representation and the GF(2^N) reduction steps to clarify the intent of the bit operations. Fixes static analysis warning about shifting negative values. Signed-off-by: Robin Getz --- plugins/channelrx/demoddatv/leansdr/discrmath.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/channelrx/demoddatv/leansdr/discrmath.h b/plugins/channelrx/demoddatv/leansdr/discrmath.h index de3835713..ff276c296 100644 --- a/plugins/channelrx/demoddatv/leansdr/discrmath.h +++ b/plugins/channelrx/demoddatv/leansdr/discrmath.h @@ -24,6 +24,7 @@ #pragma GCC diagnostic ignored "-Wshift-negative-value" #include +#include namespace leansdr { @@ -223,6 +224,11 @@ bitvect operator*(bitvect a, const bitvect &b) template struct gf2n { + // Field elements are represented as packed polynomial coefficients: + // bit i corresponds to the coefficient of X^i. Unsigned storage is + // required so bit operations have well-defined behavior. + static_assert(std::is_unsigned_v, "Te must be unsigned"); + typedef Te element; static const Te alpha = ALPHA; gf2n() @@ -236,9 +242,14 @@ struct gf2n lut_exp[i] = alpha_i; // ALPHA^i lut_exp[((1 << N) - 1) + i] = alpha_i; // Wrap to avoid modulo 2^N-1 lut_log[alpha_i] = i; - bool overflow = alpha_i & (1 << (N - 1)); + // Multiplication by ALPHA=[X] shifts the polynomial left by one. + // If the X^(N-1) coefficient was set, the shift will overflow and + // the generator polynomial must be applied modulo P(X). + bool overflow = alpha_i & (static_cast(1) << (N - 1)); alpha_i *= 2; // Multiply by alpha=[X] i.e. increase degrees - alpha_i &= ~((~(Te)0) << N); // In case Te is wider than N bits + // Keep only the lowest N bits. This removes the X^N term and + // higher bits before applying the generator polynomial reduction. + alpha_i &= static_cast((static_cast(1) << N) - 1); if (overflow) alpha_i ^= TRUNCP; // Modulo P iteratively }