On Fri, Sep 23, 2022 at 11:26:22AM +0300, Gwan-gyeong Mun wrote:
> Adds assert_same_type and assert_same_typable macros to catch type
> mis-match while compiling. The existing typecheck() macro outputs build
> warnings, but the newly added assert_same_type() macro uses the
> static_assert macro (which uses _Static_assert keyword and it introduced
> in C11) to generate a build break when the types are different and can be
> used to detect explicit build errors. Unlike the assert_same_type() macro,
> assert_same_typable() macro allows a constant value as the second argument.
> Since static_assert is used at compile time and it requires
> constant-expression as an argument [1][2], overflows_type_ret_const_expr()
> is newly added. There is overflows_type() that has the same behavior, but
> the macro uses __builtin_add_overflow() internally, and
> __builtin_add_overflows returns a bool type [3], so it is difficult to use
> as an argument of _Static_assert. The assert_same_type and
> assert_same_typable macros have been added to compiler_types.h, but the
> overflows_type_ret_const_expr macro has been added to overflow.h
> So, overflow.h has to be included to use assert_same_typable which
> internally uses overflows_type_ret_const_expr.
> And it adds unit tests for overflows_type, overflows_type_ret_const_expr,
> assert_same_type and assert_same_typable. The overflows_type has been added
> as well to compare whether the overflows_type_ret_const_expr unit test has
> the same as the result.
I spent some time rewriting the code in this patch. I think it's really
close, but I wanted to tweak how things were being defined, naming, etc.
Notes below, and I'll send my proposed patch separately...
> [...]
> +#define overflows_type_ret_const_expr(x,T) ( \
For the "overflows_type" defines, I think this reads a bit better:
#define __overflows_type_constexpr(x, T) ( \
is_unsigned_type(typeof(x)) ? \
(x) > type_max(typeof(T)) ? 1 : 0 \
: is_unsigned_type(typeof(T)) ? \
(x) < 0 || (x) > type_max(typeof(T)) ? 1 : 0 \
: (x) < type_min(typeof(T)) || \
(x) > type_max(typeof(T)) ? 1 : 0 )
#define __overflows_type(x, T) ({ \
typeof(T) v = 0; \
check_add_overflow((x), v, &v); \
})
#define overflows_type(n, T) \
__builtin_choose_expr(__is_constexpr(n), \
__overflows_type_constexpr(n, T), \
__overflows_type(n, T))
> [...]
> +/**
> + * assert_same_type - abort compilation if the first argument's data type and
> + * the second argument's data type are not the same
> + * @t1: data type or variable
> + * @t2: data type or variable
> + *
> + * The first and second arguments can be data types or variables or mixed
> (the
> + * first argument is the data type and the second argument is variable or
> vice
> + * versa). It determines whether the first argument's data type and the
> second
> + * argument's data type are the same while compiling, and it aborts
> compilation
> + * if the two types are not the same.
> + * See also assert_same_typable().
> + */
> +#define assert_same_type(t1, t2) static_assert(__same_type(t1, t2))
I still think I'd rather avoid a define for this. It doesn't seem worth
4 characters of savings to just have to type it out:
static_assert(__same_type(a, b))
> [...]
> +#define assert_same_typable(t, n) static_assert( \
> + __builtin_choose_expr(__builtin_constant_p(n), \
> + overflows_type_ret_const_expr(n,t) == 0, \
> + __same_type(t, n)))
This one I'd like to convert into something closer in naming convention to
"__same_type". Also note that "__builtin_constant_p()" doesn't actually
work here: it needs to be __is_constexpr(). So, I propose:
#define __castable_to_type(n, T) \
__builtin_choose_expr(__is_constexpr(n), \
__overflows_type_constexpr(n, T), \
__same_type(n, T))
Then we can do:
static_assert(__castable_to_type(INT_MAX, size_t));
> [...[
> +static void overflows_type_test(struct kunit *test)
> +{
> +/* Args are: first type, secound type, value, overflow expected */
> +#define TEST_OVERFLOWS_TYPE(t1, t2, v, of) do {
> \
> + t1 __t1 = v; \
> + t2 __t2; \
> + bool __of; \
> + __of = overflows_type(v, t2); \
> + if (__of != of) { \
> + KUNIT_EXPECT_EQ_MSG(test, __of, of, \
> + "expected overflows_type(%s, %s) to%s overflow\n", \
> + #v, #t2, of ? "" : " not"); \
> + } \
> [...]
> + __of = overflows_type_ret_const_expr(__t1, __t2) ? true : false;\
> + if (__of != of) { \
> + KUNIT_EXPECT_EQ_MSG(test, __of, of, \
> + "expected overflows_type_ret_const_expr(%s, %s) to%s
> overflow\n", \
> + #t1" __t1 = "#v, #t2" __t2", of ? "" : " not"); \
> + } \
These tests are excellent! I've adapted them a little bit to avoid some
of their internal redundancy. (i.e. the above blocks are basically
almost entire the same, etc).
-Kees
--
Kees Cook
