From ec074d5753d589a3a4e061b80f1dd487778fb1b6 Mon Sep 17 00:00:00 2001 From: jun <83899451+zeichensystem@users.noreply.github.com> Date: Thu, 15 May 2025 08:46:54 +0200 Subject: [PATCH] Fix potential int-promotion bugs in ckdint Expressions like (uin16_t)a * (uint16_t)b * (uint16_t)c might be promoted to (signed) int (in that example, on platforms where sizeof(int) > sizeof(uint16_t)), and therefore lead to undefined behaviour on overflow. The above expression can be fixed as 1u * (uint16_t)a * (uint16_t)b * (uint16_t)c (The 1u makes sure a, b, and c would be promoted to unsigned int (instead of int) on platforms where sizeof(int) > sizeof(uint16_t)) cf. https://stackoverflow.com/questions/27001604/32-bit-unsigned-multiply-on-64-bit-causing-undefined-behavior --- src/guf_math_ckdint.h | 119 +++++++++++++++++++++--------------------- todo.txt | 10 ++-- tools/ckdint-gen.py | 16 +++--- 3 files changed, 72 insertions(+), 73 deletions(-) diff --git a/src/guf_math_ckdint.h b/src/guf_math_ckdint.h index 0fa5172..a3849be 100644 --- a/src/guf_math_ckdint.h +++ b/src/guf_math_ckdint.h @@ -68,6 +68,8 @@ */ + + #if defined(GUF_MATH_CKDINT_IMPL_STATIC) #define GUF_MATH_CKDINT_KWRDS static inline #else @@ -80,8 +82,8 @@ typedef enum guf_math_ckd_result {GUF_MATH_CKD_SUCCESS = 0, GUF_MATH_CKD_OVERFLOW, GUF_MATH_CKD_UNDERFLOW} guf_math_ckd_result; -#if !defined(GUF_MATH_CKDINT_IMPL_STATIC) && !defined(GUF_MATH_CKDINT_IMPL) +#if !defined(GUF_MATH_CKDINT_IMPL_STATIC) && !defined(GUF_MATH_CKDINT_IMPL) // Signed integer arithmetic checks (generated with libguf/tools/ckdint-gen.py) GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_int(int a, int b); @@ -259,8 +261,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_size_t(size_t a, size GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_size_t(size_t a, size_t b, size_t *result); GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_size_t(size_t a, size_t b, size_t *result); -#endif +#endif #if defined(GUF_MATH_CKDINT_IMPL) || defined(GUF_MATH_CKDINT_IMPL_STATIC) #include "guf_assert.h" // Signed integer arithmetic checks (generated with libguf/tools/ckdint-gen.py) @@ -555,8 +557,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_uchar(unsigned char a, uns } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_uchar(unsigned char a, unsigned char b) { - const unsigned char c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const unsigned char c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_unsigned(unsigned a, unsigned b) @@ -577,8 +579,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_unsigned(unsigned a, unsig } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_unsigned(unsigned a, unsigned b) { - const unsigned c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const unsigned c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_u8(uint8_t a, uint8_t b) @@ -599,8 +601,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_u8(uint8_t a, uint8_t b) } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_u8(uint8_t a, uint8_t b) { - const uint8_t c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const uint8_t c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_u16(uint16_t a, uint16_t b) @@ -621,8 +623,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_u16(uint16_t a, uint16_t b } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_u16(uint16_t a, uint16_t b) { - const uint16_t c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const uint16_t c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_u32(uint32_t a, uint32_t b) @@ -643,8 +645,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_u32(uint32_t a, uint32_t b } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_u32(uint32_t a, uint32_t b) { - const uint32_t c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const uint32_t c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_u64(uint64_t a, uint64_t b) @@ -665,8 +667,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_u64(uint64_t a, uint64_t b } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_u64(uint64_t a, uint64_t b) { - const uint64_t c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const uint64_t c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_size_t(size_t a, size_t b) @@ -687,8 +689,8 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_sub_size_t(size_t a, size_t b) } GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_size_t(size_t a, size_t b) { - const size_t c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const size_t c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; } @@ -807,11 +809,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_int(int a, int b, int break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - unsigned res = (unsigned)a * (unsigned)b; + unsigned res = 1u * (unsigned)a * (unsigned)b; if (res > INT_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const unsigned mod = ((unsigned)INT_MAX + 1u); + const unsigned mod = (1u * (unsigned)INT_MAX + 1u); GUF_ASSERT(mod > 0 && mod > INT_MAX); - res = res % mod; + res = 1u * res % mod; *result = INT_MIN + (int)res; } else { *result = (int)res; @@ -939,11 +941,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i8(int8_t a, int8_t b break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - uint8_t res = (uint8_t)a * (uint8_t)b; + uint8_t res = 1u * (uint8_t)a * (uint8_t)b; if (res > INT8_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const uint8_t mod = ((uint8_t)INT8_MAX + 1u); + const uint8_t mod = (1u * (uint8_t)INT8_MAX + 1u); GUF_ASSERT(mod > 0 && mod > INT8_MAX); - res = res % mod; + res = 1u * res % mod; *result = INT8_MIN + (int8_t)res; } else { *result = (int8_t)res; @@ -1071,11 +1073,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i16(int16_t a, int16_ break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - uint16_t res = (uint16_t)a * (uint16_t)b; + uint16_t res = 1u * (uint16_t)a * (uint16_t)b; if (res > INT16_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const uint16_t mod = ((uint16_t)INT16_MAX + 1u); + const uint16_t mod = (1u * (uint16_t)INT16_MAX + 1u); GUF_ASSERT(mod > 0 && mod > INT16_MAX); - res = res % mod; + res = 1u * res % mod; *result = INT16_MIN + (int16_t)res; } else { *result = (int16_t)res; @@ -1203,11 +1205,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i32(int32_t a, int32_ break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - uint32_t res = (uint32_t)a * (uint32_t)b; + uint32_t res = 1u * (uint32_t)a * (uint32_t)b; if (res > INT32_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const uint32_t mod = ((uint32_t)INT32_MAX + 1u); + const uint32_t mod = (1u * (uint32_t)INT32_MAX + 1u); GUF_ASSERT(mod > 0 && mod > INT32_MAX); - res = res % mod; + res = 1u * res % mod; *result = INT32_MIN + (int32_t)res; } else { *result = (int32_t)res; @@ -1335,11 +1337,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i64(int64_t a, int64_ break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - uint64_t res = (uint64_t)a * (uint64_t)b; + uint64_t res = 1u * (uint64_t)a * (uint64_t)b; if (res > INT64_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const uint64_t mod = ((uint64_t)INT64_MAX + 1u); + const uint64_t mod = (1u * (uint64_t)INT64_MAX + 1u); GUF_ASSERT(mod > 0 && mod > INT64_MAX); - res = res % mod; + res = 1u * res % mod; *result = INT64_MIN + (int64_t)res; } else { *result = (int64_t)res; @@ -1467,11 +1469,11 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_ptrdiff_t(ptrdiff_t a break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: { - size_t res = (size_t)a * (size_t)b; + size_t res = 1u * (size_t)a * (size_t)b; if (res > PTRDIFF_MAX) { // This is the fix for implementation defined conversion from unsigned to signed. - const size_t mod = ((size_t)PTRDIFF_MAX + 1u); + const size_t mod = (1u * (size_t)PTRDIFF_MAX + 1u); GUF_ASSERT(mod > 0 && mod > PTRDIFF_MAX); - res = res % mod; + res = 1u * res % mod; *result = PTRDIFF_MIN + (ptrdiff_t)res; } else { *result = (ptrdiff_t)res; @@ -1547,7 +1549,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_uchar(unsigned char a { const guf_math_ckd_result check = guf_ckd_add_uchar(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1555,7 +1557,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_uchar(unsigned char a { const guf_math_ckd_result check = guf_ckd_sub_uchar(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1563,7 +1565,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_uchar(unsigned char a { const guf_math_ckd_result check = guf_ckd_mul_uchar(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -1628,7 +1630,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_unsigned(unsigned a, { const guf_math_ckd_result check = guf_ckd_add_unsigned(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1636,7 +1638,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_unsigned(unsigned a, { const guf_math_ckd_result check = guf_ckd_sub_unsigned(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1644,7 +1646,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_unsigned(unsigned a, { const guf_math_ckd_result check = guf_ckd_mul_unsigned(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -1709,7 +1711,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_u8(uint8_t a, uint8_t { const guf_math_ckd_result check = guf_ckd_add_u8(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1717,7 +1719,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_u8(uint8_t a, uint8_t { const guf_math_ckd_result check = guf_ckd_sub_u8(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1725,7 +1727,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_u8(uint8_t a, uint8_t { const guf_math_ckd_result check = guf_ckd_mul_u8(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -1790,7 +1792,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_u16(uint16_t a, uint1 { const guf_math_ckd_result check = guf_ckd_add_u16(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1798,7 +1800,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_u16(uint16_t a, uint1 { const guf_math_ckd_result check = guf_ckd_sub_u16(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1806,7 +1808,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_u16(uint16_t a, uint1 { const guf_math_ckd_result check = guf_ckd_mul_u16(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -1871,7 +1873,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_u32(uint32_t a, uint3 { const guf_math_ckd_result check = guf_ckd_add_u32(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1879,7 +1881,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_u32(uint32_t a, uint3 { const guf_math_ckd_result check = guf_ckd_sub_u32(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1887,7 +1889,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_u32(uint32_t a, uint3 { const guf_math_ckd_result check = guf_ckd_mul_u32(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -1952,7 +1954,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_u64(uint64_t a, uint6 { const guf_math_ckd_result check = guf_ckd_add_u64(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -1960,7 +1962,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_u64(uint64_t a, uint6 { const guf_math_ckd_result check = guf_ckd_sub_u64(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -1968,7 +1970,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_u64(uint64_t a, uint6 { const guf_math_ckd_result check = guf_ckd_mul_u64(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } @@ -2033,7 +2035,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_size_t(size_t a, size { const guf_math_ckd_result check = guf_ckd_add_size_t(a, b); if (result) { - *result = a + b; + *result = 1u * a + b; } return check; } @@ -2041,7 +2043,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_size_t(size_t a, size { const guf_math_ckd_result check = guf_ckd_sub_size_t(a, b); if (result) { - *result = a - b; + *result = 1u * a - b; } return check; } @@ -2049,16 +2051,15 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_size_t(size_t a, size { const guf_math_ckd_result check = guf_ckd_mul_size_t(a, b); if (result) { - *result = a * b; + *result = 1u * a * b; } return check; } -#endif /* End impl */ - -#endif /* End header-guard */ +#endif +#endif +#undef GUF_MATH_CKDINT_KWRDS #undef GUF_MATH_CKDINT_IMPL #undef GUF_MATH_CKDINT_IMPL_STATIC -#undef GUF_MATH_CKDINT_KWRDS diff --git a/todo.txt b/todo.txt index fcf7efc..635e3ff 100644 --- a/todo.txt +++ b/todo.txt @@ -1,16 +1,14 @@ -- guf_wrapping_mul_TYPE: Not 100 % sure if it does not depend on implementation defined behaviour, but it shouldn't - +- guf_stack, guf_queue, guf_dqueue, guf_prio_queue (using a heap), guf_ringbuf - sort: add cpp #ifdef to remove restrict from declaration +- guf_wrapping_mul_TYPE: Not 100 % sure if it does not depend on implementation defined behaviour, but it shouldn't + - tests for guf_dict with GUF_DICT_64_BIT_IDX (and also hash32/hash64); maybe pass kv_type to insert to avoid copy - dict elems shrink to fit; allow to pass GUF_DBUF_USE_GROWTH_FAC_ONE_POINT_FIVE; start capacity (for elems and kv_indices?) - dict: if load factor is high due to mostly tombstones, just try rehashing without resizing first? + - bench -- example directory - -- guf_stack, guf_queue, guf_dqueue, guf_prio_queue (using a heap), guf_ringbuf - - no guf_init.h - unicode normalisation diff --git a/tools/ckdint-gen.py b/tools/ckdint-gen.py index bdd049e..de200d7 100644 --- a/tools/ckdint-gen.py +++ b/tools/ckdint-gen.py @@ -47,8 +47,8 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s }} GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_mul_{type_abbr}({type} a, {type} b) {{ - const {type} c = a * b; - return a != 0 && ((c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; + const {type} c = 1u * a * b; + return a != 0 && ((1u * c / a) != b) ? GUF_MATH_CKD_OVERFLOW : GUF_MATH_CKD_SUCCESS; }} """) @@ -230,11 +230,11 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s break; case GUF_MATH_CKD_OVERFLOW: case GUF_MATH_CKD_UNDERFLOW: {{ - {uint_type} res = ({uint_type})a * ({uint_type})b; + {uint_type} res = 1u * ({uint_type})a * ({uint_type})b; if (res > {int_max}) {{ // This is the fix for implementation defined conversion from unsigned to signed. - const {uint_type} mod = (({uint_type}){int_max} + 1u); + const {uint_type} mod = (1u * ({uint_type}){int_max} + 1u); GUF_ASSERT(mod > 0 && mod > {int_max}); - res = res % mod; + res = 1u * res % mod; *result = {int_min} + ({type})res; }} else {{ *result = ({type})res; @@ -319,7 +319,7 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s {{ const guf_math_ckd_result check = guf_ckd_add_{type_abbr}(a, b); if (result) {{ - *result = a + b; + *result = 1u * a + b; }} return check; }} @@ -327,7 +327,7 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s {{ const guf_math_ckd_result check = guf_ckd_sub_{type_abbr}(a, b); if (result) {{ - *result = a - b; + *result = 1u * a - b; }} return check; }} @@ -335,7 +335,7 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s {{ const guf_math_ckd_result check = guf_ckd_mul_{type_abbr}(a, b); if (result) {{ - *result = a * b; + *result = 1u * a * b; }} return check; }}