From 6ffb79f7a0d4a95e475178cfe4e04be57a8a0a0d Mon Sep 17 00:00:00 2001 From: jun <83899451+zeichensystem@users.noreply.github.com> Date: Wed, 14 May 2025 08:51:26 +0200 Subject: [PATCH] Fix implementation defined behaviour in guf_wrapping_mul --- src/guf_alloc_tracker.c | 1 + src/guf_math_ckdint.h | 104 +++++++++++++++++++++++++++++++-------- src/test/test_ckdint.cpp | 40 +++++++++++++++ tools/ckdint-gen.py | 13 ++++- 4 files changed, 135 insertions(+), 23 deletions(-) diff --git a/src/guf_alloc_tracker.c b/src/guf_alloc_tracker.c index 86db727..c69c747 100644 --- a/src/guf_alloc_tracker.c +++ b/src/guf_alloc_tracker.c @@ -19,6 +19,7 @@ static bool guf_track_alloc(guf_alloc_tracker *t, ptrdiff_t size) fprintf(t->err_log, "guf_alloc_track (id %" PRIu32 ") WARNING: alloc_count overflow\n", t->id); //success = false; } + t->alloc_count = guf_add_saturated_size_t(t->alloc_count, 1); if (guf_add_is_overflow_ptrdiff(t->allocated_bytes, size)) { diff --git a/src/guf_math_ckdint.h b/src/guf_math_ckdint.h index 5f0fbc2..233fc5c 100644 --- a/src/guf_math_ckdint.h +++ b/src/guf_math_ckdint.h @@ -7,12 +7,6 @@ #define GUF_MATH_CKDINT_KWRDS #endif -#ifndef GUF_MATH_CKDINT_H -#define GUF_MATH_CKDINT_H -#include "guf_common.h" - -typedef enum guf_math_ckd_result {GUF_MATH_CKD_SUCCESS = 0, GUF_MATH_CKD_OVERFLOW, GUF_MATH_CKD_UNDERFLOW} guf_math_ckd_result; - /* // Functions for safely checking for over- and underflow of arithmetic operations @@ -78,6 +72,19 @@ typedef enum guf_math_ckd_result {GUF_MATH_CKD_SUCCESS = 0, GUF_MATH_CKD_OVERFLO cf. https://stackoverflow.com/questions/29808397/how-to-portably-find-out-minint-max-absint-min (last-retrieved 2025-05-11) */ + +#if defined(GUF_MATH_CKDINT_IMPL_STATIC) + #define GUF_MATH_CKDINT_KWRDS static inline +#else + #define GUF_MATH_CKDINT_KWRDS +#endif + +#ifndef GUF_MATH_CKDINT_H +#define GUF_MATH_CKDINT_H +#include "guf_common.h" + +typedef enum guf_math_ckd_result {GUF_MATH_CKD_SUCCESS = 0, GUF_MATH_CKD_OVERFLOW, GUF_MATH_CKD_UNDERFLOW} guf_math_ckd_result; + // 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); @@ -190,6 +197,7 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_add_ptrdiff_t(ptrdiff_t a GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_sub_ptrdiff_t(ptrdiff_t a, ptrdiff_t b, ptrdiff_t *result); GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_ptrdiff_t(ptrdiff_t a, ptrdiff_t b, ptrdiff_t *result); + // Unsigned saturating/wrapping arithmetic (generated with libguf/tools/ckdint-gen.py) GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_saturating_add_uchar(unsigned char a, unsigned char b, unsigned char *result); @@ -257,7 +265,6 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_size_t(size_t a, size #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) GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_ckd_add_int(int a, int b) @@ -801,9 +808,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_int(int a, int b, int *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (int)((unsigned)a * (unsigned)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + unsigned res = (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); + GUF_ASSERT(mod > 0 && mod > INT_MAX); + res = res % mod; + *result = INT_MIN + (int)res; + } else { + *result = (int)res; + } break; + } default: GUF_ASSERT(false); } @@ -924,9 +940,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i8(int8_t a, int8_t b *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (int8_t)((uint8_t)a * (uint8_t)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + uint8_t res = (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); + GUF_ASSERT(mod > 0 && mod > INT8_MAX); + res = res % mod; + *result = INT8_MIN + (int8_t)res; + } else { + *result = (int8_t)res; + } break; + } default: GUF_ASSERT(false); } @@ -1047,9 +1072,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i16(int16_t a, int16_ *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (int16_t)((uint16_t)a * (uint16_t)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + uint16_t res = (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); + GUF_ASSERT(mod > 0 && mod > INT16_MAX); + res = res % mod; + *result = INT16_MIN + (int16_t)res; + } else { + *result = (int16_t)res; + } break; + } default: GUF_ASSERT(false); } @@ -1170,9 +1204,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i32(int32_t a, int32_ *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (int32_t)((uint32_t)a * (uint32_t)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + uint32_t res = (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); + GUF_ASSERT(mod > 0 && mod > INT32_MAX); + res = res % mod; + *result = INT32_MIN + (int32_t)res; + } else { + *result = (int32_t)res; + } break; + } default: GUF_ASSERT(false); } @@ -1293,9 +1336,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_i64(int64_t a, int64_ *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (int64_t)((uint64_t)a * (uint64_t)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + uint64_t res = (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); + GUF_ASSERT(mod > 0 && mod > INT64_MAX); + res = res % mod; + *result = INT64_MIN + (int64_t)res; + } else { + *result = (int64_t)res; + } break; + } default: GUF_ASSERT(false); } @@ -1416,9 +1468,18 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_ptrdiff_t(ptrdiff_t a *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = (ptrdiff_t)((size_t)a * (size_t)b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: { + size_t res = (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); + GUF_ASSERT(mod > 0 && mod > PTRDIFF_MAX); + res = res % mod; + *result = PTRDIFF_MIN + (ptrdiff_t)res; + } else { + *result = (ptrdiff_t)res; + } break; + } default: GUF_ASSERT(false); } @@ -1995,9 +2056,10 @@ GUF_MATH_CKDINT_KWRDS guf_math_ckd_result guf_wrapping_mul_size_t(size_t a, size return check; } -#endif -#endif +#endif /* End impl */ + +#endif /* End header-guard */ #undef GUF_MATH_CKDINT_IMPL #undef GUF_MATH_CKDINT_IMPL_STATIC diff --git a/src/test/test_ckdint.cpp b/src/test/test_ckdint.cpp index adbb423..1eee600 100644 --- a/src/test/test_ckdint.cpp +++ b/src/test/test_ckdint.cpp @@ -154,6 +154,18 @@ void CkdIntTest::test_ckd() TEST_CHECK(guf_wrapping_mul_i8(5, 42, &mul_i8_res) == GUF_MATH_CKD_OVERFLOW); TEST_CHECK(mul_i8_res == -46); + /* + // https://play.rust-lang.org/?version=stable&mode=debug&edition=2024 + + use std::num::Wrapping; + + fn main() { + let a = Wrapping(-314159265_i32); + let b = Wrapping(4096_i32); + println!("{}", a * b); + } + */ + int32_t mul_i32_res = -1; TEST_CHECK(guf_wrapping_mul_i32(42002718, 314159265, &mul_i32_res) == GUF_MATH_CKD_OVERFLOW); TEST_CHECK(mul_i32_res == -972735522); @@ -161,6 +173,34 @@ void CkdIntTest::test_ckd() TEST_CHECK(guf_wrapping_mul_i32(314159265, 42002718, &mul_i32_res) == GUF_MATH_CKD_OVERFLOW); TEST_CHECK(mul_i32_res == -972735522); + mul_i32_res = 12345; + guf_wrapping_mul_i32(42002718, 314159265, &mul_i32_res); + TEST_CHECK(mul_i32_res == -972735522); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(-42002718, 314159265, &mul_i32_res); + TEST_CHECK(mul_i32_res == 972735522); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(-88888888, 99999999, &mul_i32_res); + TEST_CHECK(mul_i32_res == 1374494264); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(INT32_MIN, -1, &mul_i32_res); + TEST_CHECK(mul_i32_res == INT32_MIN); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(-2147483648, 2147483640, &mul_i32_res); + TEST_CHECK(mul_i32_res == 0); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(-2048, -314159265, &mul_i32_res); + TEST_CHECK(mul_i32_res == -846919680); + + mul_i32_res = 12345; + guf_wrapping_mul_i32(4096, -314159265, &mul_i32_res); + TEST_CHECK(mul_i32_res == 1693839360); + ptrdiff_t ptrdiff_res = -1234; TEST_CHECK(guf_saturating_add_ptrdiff_t(PTRDIFF_MAX, 1, &ptrdiff_res) == GUF_MATH_CKD_OVERFLOW); TEST_CHECK(ptrdiff_res == PTRDIFF_MAX); diff --git a/tools/ckdint-gen.py b/tools/ckdint-gen.py index 72ddb9a..e97913a 100644 --- a/tools/ckdint-gen.py +++ b/tools/ckdint-gen.py @@ -229,9 +229,18 @@ def generate_ckdint_functions(int_types: list, uint_types: list) -> Tuple[str, s *result = a * b; break; case GUF_MATH_CKD_OVERFLOW: - case GUF_MATH_CKD_UNDERFLOW: - *result = ({type})(({uint_type})a * ({uint_type})b); // TODO: Implementation defined... + case GUF_MATH_CKD_UNDERFLOW: {{ + {uint_type} res = ({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); + GUF_ASSERT(mod > 0 && mod > {int_max}); + res = res % mod; + *result = {int_min} + ({type})res; + }} else {{ + *result = ({type})res; + }} break; + }} default: GUF_ASSERT(false); }}