[CDRIVER-5996] Prevent multi-evaluation of arithmetic macro-function operands

src/common/src/mlib/ckdint.h

@@ -5,9 +5,6 @@
  *
  * This file implements the C23 checked-integer-arithmetic functions as macros.
  *
- * The implementation is nearly perfect: The macros necessarily evaluate the
- * operand expressions more than once, so callers should be aware of this caveat.
- *
  * The function-like macros are defined:
  *
  * - `mlib_add(Dst, L, R)` / `mlib_add(Dst, A)`
@@ -159,7 +156,7 @@ mlib_extern_c_begin ();
  * @param T A type specifier for a target integral type for the cast.
  * @param Operand The integral value to be converted.
  *
- * If the cast would result in the operand value chaning, the program will be
+ * If the cast would result in the operand value changing, the program will be
  * terminated with a diagnostic.
  */
 #define mlib_assert_narrow(T, Operand) \

src/common/src/mlib/cmp.h

@@ -46,9 +46,6 @@ enum mlib_cmp_result {
 /**
  * @brief Compare two integral values safely.
  *
- * NOTE: This macro may evaluate the operand expressions more than once! Do not
- * use expressions that are expensive or have side effects!
- *
  * This function can be called with two arguments or with three:
  *
  * - `mlib_cmp(a, b)` Returns a value of type `mlib_cmp_result`

src/common/src/mlib/intutil.h

@@ -78,16 +78,21 @@ typedef struct mlib_upsized_integer {
  * an i64 losslessly.
  *
  * If the integer to upcast is the same size as `intmax_t`, we need to decide whether to store
- * it as unsigned. The expression `(0 & Value) - 1 < 0` will be `true` iff the operand is signed,
+ * it as unsigned. The expression `(_mlibGetZero(Values)) - 1 < 0` will be `true` iff the operand is signed,
  * otherwise false. If the operand is signed, we can safely cast to `intmax_t` (it probably already
  * is of that type), otherwise, we can to `uintmax_t` and the returned `mlib_upsized_integer` will
  * indicate that the stored value is unsigned.
  */
 #define mlib_upsize_integer(Value) \
    /* NOLINTNEXTLINE(bugprone-sizeof-expression) */ \
-   ((sizeof ((Value)) < sizeof (intmax_t) || ((0 & (Value)) - 1) < 0) \
+   ((sizeof ((Value)) < sizeof (intmax_t) || (_mlibGetZero(Value) - 1) < _mlibGetZero(Value)) \
       ? mlib_init(mlib_upsized_integer) {{(intmax_t) (Value)}, true} \
       : mlib_init(mlib_upsized_integer) {{(intmax_t) (uintmax_t) (Value)}})
+// Yield a zero value of similar-ish type to the given expression. The ternary
+// forces an integer promotion of literal zero match the type of `V`, while leaving
+// `V` unevaluated. Note that this will also promote `V` to be at least `(unsigned) int`,
+// so the zero value is only "similar" to `V`, and may be of a larger type
+#define _mlibGetZero(V) (1 ? 0 : (V))
 // clang-format on
 
 #endif // MLIB_INTUTIL_H_INCLUDED

src/common/tests/test-mlib.c

@@ -313,6 +313,14 @@ _test_cmp (void)
    mlib_diagnostic_pop ();
    // mlib_cmp produces the correct answer:
    ASSERT (mlib_cmp (-27, <, 20u));
+
+   {
+      // Check that we do not double-evaluate the operand expression.
+      intmax_t a = 4;
+      mlib_check (mlib_cmp (++a, ==, 5));
+      // We only increment once:
+      mlib_check (a, eq, 5);
+   }
 }
 
 static void