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
This commit is contained in:
jun 2025-05-15 08:46:54 +02:00
parent b99434a611
commit ec074d5753
3 changed files with 72 additions and 73 deletions

View File

@ -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

View File

@ -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

View File

@ -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;
}}