Sparse knew about the __sync_*() builtin functions but without real type checking and nothing about the more commonly used __atomic_*(). This series fixes by adding the full type evaluation for both sets. Luc Van Oostenryck (12): builtin: add generic .args method builtin: add builtin type for volatile void * builtin: make eval_sync_compare_and_swap() more generic builtin: evaluate __sync_*_fetch*() builtin: fix evaluation of __sync_lock_release builtin: __sync_synchronize() too is variadic builtin: add predefines for __ATOMIC_RELAXED & friends builtin: add support for __atomic_add_fetch(), ... builtin: add support for others generic atomic builtins builtin: add builtin type: [volatile] pointer to bool builtin: add support for __atomic_clear() builtin: add support for remaining atomic builtins builtin.c | 112 +++++++++++++++++++++--------- predefine.c | 7 ++ symbol.c | 6 ++ symbol.h | 2 + validation/builtin-atomic-clear.c | 15 ++++ validation/builtin-sync-fetch.c | 24 +++++++ 6 files changed, 135 insertions(+), 31 deletions(-) create mode 100644 validation/builtin-atomic-clear.c create mode 100644 validation/builtin-sync-fetch.c base-commit: 5192dc1ff23dae8644480a89ada8ff420ebb674a -- 2.28.0
The arity of builtin functions can be retrieved from their prototype. So, create a generic .args method, doing the evaluation of all arguments present in the prototype. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/builtin.c b/builtin.c index 26b612dc401b..0d4cb12cca22 100644 --- a/builtin.c +++ b/builtin.c @@ -83,6 +83,13 @@ error: return 0; } +static int args_prototype(struct expression *expr) +{ + struct symbol *fntype = expr->fn->ctype->ctype.base_type; + int n = symbol_list_size(fntype->arguments); + return eval_args(expr, n); +} + static int args_triadic(struct expression *expr) { return eval_args(expr, 3); -- 2.28.0
This is the type of most __sync_* or __atomic_* builtins. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- symbol.c | 3 +++ symbol.h | 1 + 2 files changed, 4 insertions(+) diff --git a/symbol.c b/symbol.c index 5e7f07969f96..ec514eb45df4 100644 --- a/symbol.c +++ b/symbol.c @@ -805,6 +805,7 @@ struct symbol float128_ctype; struct symbol const_void_ctype, const_char_ctype; struct symbol const_ptr_ctype, const_string_ctype; struct symbol const_wchar_ctype, const_wstring_ctype; +struct symbol volatile_void_ctype, volatile_ptr_ctype; struct symbol zero_int; @@ -909,6 +910,8 @@ static const struct ctype_declare { { &const_void_ctype, T_CONST(&void_ctype, NULL, NULL) }, { &const_char_ctype, T_CONST(&char_ctype, &bits_in_char, &max_int_alignment)}, { &const_wchar_ctype, T_CONST(&int_ctype, NULL, NULL) }, + { &volatile_void_ctype,T_NODE(MOD_VOLATILE, &void_ctype, NULL, NULL) }, + { &volatile_ptr_ctype, T_PTR(&volatile_void_ctype) }, { NULL, } }; diff --git a/symbol.h b/symbol.h index e75ea3abfcd3..97c608e84704 100644 --- a/symbol.h +++ b/symbol.h @@ -310,6 +310,7 @@ extern struct symbol float128_ctype; extern struct symbol const_void_ctype, const_char_ctype; extern struct symbol const_ptr_ctype, const_string_ctype; extern struct symbol const_wchar_ctype, const_wstring_ctype; +extern struct symbol volatile_void_ctype, volatile_ptr_ctype; /* Special internal symbols */ extern struct symbol zero_int; -- 2.28.0
Most __sync_* or __atomic_* builtin functions have one or more arguments having its real type determined by the first argument: either the same type (a pointer to an integral type) or the type of the object it points to. Currently, only __sync_{bool,val}_compare_and_swap() are handled but lots of very similar variants would be needed for the others. So, make it a generic function, able to handle all these builtins. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/builtin.c b/builtin.c index 0d4cb12cca22..880dd54f647e 100644 --- a/builtin.c +++ b/builtin.c @@ -31,6 +31,14 @@ #include "compat/bswap.h" #include <stdarg.h> +#define dyntype incomplete_ctype +static bool is_dynamic_type(struct symbol *t) +{ + if (t->type == SYM_NODE) + t = t->ctype.base_type; + return t == &dyntype; +} + static int evaluate_to_int_const_expr(struct expression *expr) { expr->ctype = &int_ctype; @@ -362,29 +370,32 @@ static struct symbol_op overflow_p_op = { }; -static int eval_sync_compare_and_swap(struct expression *expr) +static int eval_atomic_common(struct expression *expr) { + struct symbol *fntype = expr->fn->ctype->ctype.base_type; struct symbol_list *types = NULL; struct symbol *ctype = NULL; + struct symbol *t; struct expression *arg; int n = 0; - /* the first arg is a pointer type; we'd already verified that */ + // The number of arguments have already be verified. + // The first arg must be a pointer type to an integral type. + PREPARE_PTR_LIST(fntype->arguments, t); FOR_EACH_PTR(expr->args, arg) { - struct symbol *t = arg->ctype; + struct symbol *ptrtype = NULL; - if (!t) - return 0; - - // 2nd & 3rd args must be a basic integer type or a pointer - // 1st arg must be a pointer to such a type. if (++n == 1) { + t = arg->ctype; + if (!t) + return 0; if (t->type == SYM_NODE) t = t->ctype.base_type; if (!t) return 0; if (t->type != SYM_PTR) goto err; + ptrtype = t; t = t->ctype.base_type; if (!t) return 0; @@ -395,11 +406,14 @@ static int eval_sync_compare_and_swap(struct expression *expr) if (t->type != SYM_PTR && t->ctype.base_type != &int_type) goto err; ctype = t; - add_ptr_list(&types, arg->ctype); - } else { - add_ptr_list(&types, ctype); + t = ptrtype; + } else if (is_dynamic_type(t)) { + t = ctype; } + add_ptr_list(&types, t); + NEXT_PTR_LIST(t); } END_FOR_EACH_PTR(arg); + FINISH_PTR_LIST(t); if (!expr->ctype) // __sync_val_compare_and_swap() expr->ctype = ctype; @@ -412,9 +426,9 @@ err: return 0; } -static struct symbol_op sync_compare_and_swap_op = { - .args = args_triadic, - .evaluate = eval_sync_compare_and_swap, +static struct symbol_op atomic_op = { + .args = args_prototype, + .evaluate = eval_atomic_common, }; @@ -464,6 +478,7 @@ static void declare_builtins(int stream, const struct builtin_fn tbl[]) static const struct builtin_fn builtins_common[] = { #define size_t_ctype &size_t_alias #define va_list_ctype &ptr_ctype +#define vol_ptr &volatile_ptr_ctype { "__builtin_choose_expr", NULL, 1, .op = &choose_op }, { "__builtin_constant_p", NULL, 1, .op = &constant_p_op }, { "__builtin_expect", &long_ctype, 0, { &long_ctype ,&long_ctype }, .op = &expect_op }, @@ -614,7 +629,7 @@ static const struct builtin_fn builtins_common[] = { { "__sync_add_and_fetch", &int_ctype, 1, { &ptr_ctype }}, { "__sync_and_and_fetch", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_bool_compare_and_swap", &bool_ctype, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_op}, + { "__sync_bool_compare_and_swap", &bool_ctype, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op}, { "__sync_fetch_and_add", &int_ctype, 1, { &ptr_ctype }}, { "__sync_fetch_and_and", &int_ctype, 1, { &ptr_ctype }}, { "__sync_fetch_and_nand", &int_ctype, 1, { &ptr_ctype }}, @@ -627,7 +642,7 @@ static const struct builtin_fn builtins_common[] = { { "__sync_or_and_fetch", &int_ctype, 1, { &ptr_ctype }}, { "__sync_sub_and_fetch", &int_ctype, 1, { &ptr_ctype }}, { "__sync_synchronize", &void_ctype, 0 }, - { "__sync_val_compare_and_swap", NULL, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_op }, + { "__sync_val_compare_and_swap", NULL, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op }, { "__sync_xor_and_fetch", &int_ctype, 1, { &ptr_ctype }}, { } -- 2.28.0
Reuse the generic method for all these builtins. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 26 +++++++++++++------------- validation/builtin-sync-fetch.c | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 validation/builtin-sync-fetch.c diff --git a/builtin.c b/builtin.c index 880dd54f647e..5e490830e147 100644 --- a/builtin.c +++ b/builtin.c @@ -627,23 +627,23 @@ static const struct builtin_fn builtins_common[] = { { "__builtin___vsnprintf_chk", &int_ctype, 0, { &string_ctype, size_t_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }}, { "__builtin___vsprintf_chk", &int_ctype, 0, { &string_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }}, - { "__sync_add_and_fetch", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_and_and_fetch", &int_ctype, 1, { &ptr_ctype }}, + { "__sync_add_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_and_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_bool_compare_and_swap", &bool_ctype, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op}, - { "__sync_fetch_and_add", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_fetch_and_and", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_fetch_and_nand", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_fetch_and_or", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_fetch_and_sub", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_fetch_and_xor", &int_ctype, 1, { &ptr_ctype }}, + { "__sync_fetch_and_add", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_fetch_and_and", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_fetch_and_nand", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_fetch_and_or", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_fetch_and_sub", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_fetch_and_xor", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_lock_release", &void_ctype, 1, { &ptr_ctype }}, - { "__sync_lock_test_and_set", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_nand_and_fetch", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_or_and_fetch", &int_ctype, 1, { &ptr_ctype }}, - { "__sync_sub_and_fetch", &int_ctype, 1, { &ptr_ctype }}, + { "__sync_lock_test_and_set", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_nand_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_or_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, + { "__sync_sub_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_synchronize", &void_ctype, 0 }, { "__sync_val_compare_and_swap", NULL, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op }, - { "__sync_xor_and_fetch", &int_ctype, 1, { &ptr_ctype }}, + { "__sync_xor_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { } }; diff --git a/validation/builtin-sync-fetch.c b/validation/builtin-sync-fetch.c new file mode 100644 index 000000000000..45139a3c8c3e --- /dev/null +++ b/validation/builtin-sync-fetch.c @@ -0,0 +1,24 @@ +static int ok_int(int *ptr, int val) +{ + return __sync_add_and_fetch(ptr, val); +} + +static long* ok_ptr(long **ptr, long *val) +{ + return __sync_add_and_fetch(ptr, val); +} + +static void chk_ret_ok(long *ptr, long val) +{ + _Static_assert([typeof(__sync_add_and_fetch(ptr, val))] == [long], ""); +} + +static int chk_val(int *ptr, long val) +{ + // OK: val is converted to an int + return __sync_add_and_fetch(ptr, val); +} + +/* + * check-name: builtin-sync-fetch + */ -- 2.28.0
It must use the generic method too. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin.c b/builtin.c index 5e490830e147..cb900599f112 100644 --- a/builtin.c +++ b/builtin.c @@ -636,7 +636,7 @@ static const struct builtin_fn builtins_common[] = { { "__sync_fetch_and_or", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_fetch_and_sub", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_fetch_and_xor", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, - { "__sync_lock_release", &void_ctype, 1, { &ptr_ctype }}, + { "__sync_lock_release", &void_ctype, 1, { vol_ptr }, .op = &atomic_op }, { "__sync_lock_test_and_set", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_nand_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_or_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, -- 2.28.0
This builtin was marked as taking no argument but is in fact variadic (like all the __sync_* builtins). Fix this by marking it as being variadic. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin.c b/builtin.c index cb900599f112..4921a2429f55 100644 --- a/builtin.c +++ b/builtin.c @@ -641,7 +641,7 @@ static const struct builtin_fn builtins_common[] = { { "__sync_nand_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_or_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, { "__sync_sub_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, - { "__sync_synchronize", &void_ctype, 0 }, + { "__sync_synchronize", &void_ctype, 1 }, { "__sync_val_compare_and_swap", NULL, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op }, { "__sync_xor_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, -- 2.28.0
The __atomic_*() builtins take an int argument to specify the desired memory ordering. The different admissible values are predefined by the compiler, so do that too for Sparse. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- predefine.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/predefine.c b/predefine.c index f898cdfa39b8..98e38a04579d 100644 --- a/predefine.c +++ b/predefine.c @@ -179,6 +179,13 @@ void predefined_macros(void) if (arch_target->has_int128) predefined_sizeof("INT128", "", 128); + predefine("__ATOMIC_RELAXED", 0, "0"); + predefine("__ATOMIC_CONSUME", 0, "1"); + predefine("__ATOMIC_ACQUIRE", 0, "3"); + predefine("__ATOMIC_RELEASE", 0, "4"); + predefine("__ATOMIC_ACQ_REL", 0, "7"); + predefine("__ATOMIC_SEQ_CST", 0, "8"); + predefine("__ORDER_LITTLE_ENDIAN__", 1, "1234"); predefine("__ORDER_BIG_ENDIAN__", 1, "4321"); predefine("__ORDER_PDP_ENDIAN__", 1, "3412"); -- 2.28.0
Reuse the generic method for all these builtins. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/builtin.c b/builtin.c index 4921a2429f55..a65cfd53ced1 100644 --- a/builtin.c +++ b/builtin.c @@ -479,6 +479,18 @@ static const struct builtin_fn builtins_common[] = { #define size_t_ctype &size_t_alias #define va_list_ctype &ptr_ctype #define vol_ptr &volatile_ptr_ctype + { "__atomic_add_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_and_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_add", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_and", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_nand",NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_or", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_sub", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_fetch_xor", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_nand_fetch",NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_or_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_sub_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_xor_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__builtin_choose_expr", NULL, 1, .op = &choose_op }, { "__builtin_constant_p", NULL, 1, .op = &constant_p_op }, { "__builtin_expect", &long_ctype, 0, { &long_ctype ,&long_ctype }, .op = &expect_op }, -- 2.28.0
Reuse the generic method for all these builtins. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/builtin.c b/builtin.c index a65cfd53ced1..94f86a87c3e2 100644 --- a/builtin.c +++ b/builtin.c @@ -409,6 +409,8 @@ static int eval_atomic_common(struct expression *expr) t = ptrtype; } else if (is_dynamic_type(t)) { t = ctype; + } else if (t == &ptr_ctype) { + t = ptrtype; } add_ptr_list(&types, t); NEXT_PTR_LIST(t); @@ -481,14 +483,22 @@ static const struct builtin_fn builtins_common[] = { #define vol_ptr &volatile_ptr_ctype { "__atomic_add_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_and_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_compare_exchange", &bool_ctype, 0, { vol_ptr, &ptr_ctype, &ptr_ctype, &bool_ctype, &int_ctype, &int_ctype }, .op = &atomic_op }, + { "__atomic_compare_exchange_n", &bool_ctype, 0, { vol_ptr, &ptr_ctype, &dyntype, &bool_ctype, &int_ctype, &int_ctype }, .op = &atomic_op }, + { "__atomic_exchange", &void_ctype, 0, { vol_ptr, &ptr_ctype, &ptr_ctype, &int_ctype }, .op = &atomic_op }, + { "__atomic_exchange_n", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_add", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_and", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_nand",NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_or", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_sub", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_xor", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_load", &void_ctype, 0, { vol_ptr, &ptr_ctype, &int_ctype }, .op = &atomic_op }, + { "__atomic_load_n", NULL, 0, { vol_ptr, &int_ctype }, .op = &atomic_op }, { "__atomic_nand_fetch",NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_or_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_store", &void_ctype, 0, { vol_ptr, &ptr_ctype, &int_ctype }, .op = &atomic_op }, + { "__atomic_store_n", &void_ctype, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_sub_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_xor_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__builtin_choose_expr", NULL, 1, .op = &choose_op }, -- 2.28.0
This builtin type is needed for __atomic_clear()'s prototype. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- symbol.c | 3 +++ symbol.h | 1 + 2 files changed, 4 insertions(+) diff --git a/symbol.c b/symbol.c index ec514eb45df4..5d4f078b3444 100644 --- a/symbol.c +++ b/symbol.c @@ -806,6 +806,7 @@ struct symbol const_void_ctype, const_char_ctype; struct symbol const_ptr_ctype, const_string_ctype; struct symbol const_wchar_ctype, const_wstring_ctype; struct symbol volatile_void_ctype, volatile_ptr_ctype; +struct symbol volatile_bool_ctype, volatile_bool_ptr_ctype; struct symbol zero_int; @@ -912,6 +913,8 @@ static const struct ctype_declare { { &const_wchar_ctype, T_CONST(&int_ctype, NULL, NULL) }, { &volatile_void_ctype,T_NODE(MOD_VOLATILE, &void_ctype, NULL, NULL) }, { &volatile_ptr_ctype, T_PTR(&volatile_void_ctype) }, + { &volatile_bool_ctype,T_NODE(MOD_VOLATILE, &bool_ctype, NULL, NULL) }, + { &volatile_bool_ptr_ctype, T_PTR(&volatile_bool_ctype) }, { NULL, } }; diff --git a/symbol.h b/symbol.h index 97c608e84704..5c5a7f12affa 100644 --- a/symbol.h +++ b/symbol.h @@ -311,6 +311,7 @@ extern struct symbol const_void_ctype, const_char_ctype; extern struct symbol const_ptr_ctype, const_string_ctype; extern struct symbol const_wchar_ctype, const_wstring_ctype; extern struct symbol volatile_void_ctype, volatile_ptr_ctype; +extern struct symbol volatile_bool_ctype, volatile_bool_ptr_ctype; /* Special internal symbols */ extern struct symbol zero_int; -- 2.28.0
The first argument is supposed to be a pointer to a bool, but of course, a volatile qualified pointer should be accepted too. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 1 + validation/builtin-atomic-clear.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 validation/builtin-atomic-clear.c diff --git a/builtin.c b/builtin.c index 94f86a87c3e2..1c502597315f 100644 --- a/builtin.c +++ b/builtin.c @@ -483,6 +483,7 @@ static const struct builtin_fn builtins_common[] = { #define vol_ptr &volatile_ptr_ctype { "__atomic_add_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_and_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_clear", &void_ctype, 0, { &volatile_bool_ptr_ctype, &int_ctype }}, { "__atomic_compare_exchange", &bool_ctype, 0, { vol_ptr, &ptr_ctype, &ptr_ctype, &bool_ctype, &int_ctype, &int_ctype }, .op = &atomic_op }, { "__atomic_compare_exchange_n", &bool_ctype, 0, { vol_ptr, &ptr_ctype, &dyntype, &bool_ctype, &int_ctype, &int_ctype }, .op = &atomic_op }, { "__atomic_exchange", &void_ctype, 0, { vol_ptr, &ptr_ctype, &ptr_ctype, &int_ctype }, .op = &atomic_op }, diff --git a/validation/builtin-atomic-clear.c b/validation/builtin-atomic-clear.c new file mode 100644 index 000000000000..ef430c64c244 --- /dev/null +++ b/validation/builtin-atomic-clear.c @@ -0,0 +1,15 @@ +void foo(void *ptr, _Bool *bptr, volatile void *vptr, volatile _Bool *vbptr, int mo) +{ + __atomic_clear(ptr, mo); + __atomic_clear(bptr, mo); + __atomic_clear(vptr, mo); + __atomic_clear(vbptr, mo); +} + +/* + * check-name: builtin-atomic-clear + * + * check-error-start +builtin-atomic-clear.c:1:6: warning: symbol 'foo' was not declared. Should it be static? + * check-error-end + */ -- 2.28.0
Nothing special for these ones, just plain functions with fixed types and arity. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- builtin.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin.c b/builtin.c index 1c502597315f..bb0f11b9428b 100644 --- a/builtin.c +++ b/builtin.c @@ -482,6 +482,7 @@ static const struct builtin_fn builtins_common[] = { #define va_list_ctype &ptr_ctype #define vol_ptr &volatile_ptr_ctype { "__atomic_add_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_always_lock_free", &bool_ctype, 0, { size_t_ctype, vol_ptr }}, { "__atomic_and_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_clear", &void_ctype, 0, { &volatile_bool_ptr_ctype, &int_ctype }}, { "__atomic_compare_exchange", &bool_ctype, 0, { vol_ptr, &ptr_ctype, &ptr_ctype, &bool_ctype, &int_ctype, &int_ctype }, .op = &atomic_op }, @@ -494,13 +495,17 @@ static const struct builtin_fn builtins_common[] = { { "__atomic_fetch_or", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_sub", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_fetch_xor", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_is_lock_free", &bool_ctype, 0, { size_t_ctype, vol_ptr }}, { "__atomic_load", &void_ctype, 0, { vol_ptr, &ptr_ctype, &int_ctype }, .op = &atomic_op }, { "__atomic_load_n", NULL, 0, { vol_ptr, &int_ctype }, .op = &atomic_op }, { "__atomic_nand_fetch",NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_or_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_signal_fence", &void_ctype, 0, { &int_ctype }}, { "__atomic_store", &void_ctype, 0, { vol_ptr, &ptr_ctype, &int_ctype }, .op = &atomic_op }, { "__atomic_store_n", &void_ctype, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__atomic_sub_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, + { "__atomic_test_and_set", &bool_ctype, 0, { vol_ptr, &int_ctype }}, + { "__atomic_thread_fence", &void_ctype, 0, { &int_ctype }}, { "__atomic_xor_fetch", NULL, 0, { vol_ptr, &dyntype, &int_ctype }, .op = &atomic_op }, { "__builtin_choose_expr", NULL, 1, .op = &choose_op }, { "__builtin_constant_p", NULL, 1, .op = &constant_p_op }, -- 2.28.0
On 17/10/2020 23:56, Luc Van Oostenryck wrote: > Most __sync_* or __atomic_* builtin functions have one or > more arguments having its real type determined by the first > argument: either the same type (a pointer to an integral type) > or the type of the object it points to. > > Currently, only __sync_{bool,val}_compare_and_swap() are handled > but lots of very similar variants would be needed for the others. > So, make it a generic function, able to handle all these builtins. > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > builtin.c | 47 +++++++++++++++++++++++++++++++---------------- > 1 file changed, 31 insertions(+), 16 deletions(-) > > diff --git a/builtin.c b/builtin.c > index 0d4cb12cca22..880dd54f647e 100644 > --- a/builtin.c > +++ b/builtin.c > @@ -31,6 +31,14 @@ > #include "compat/bswap.h" > #include <stdarg.h> > > +#define dyntype incomplete_ctype > +static bool is_dynamic_type(struct symbol *t) > +{ > + if (t->type == SYM_NODE) > + t = t->ctype.base_type; > + return t == &dyntype; > +} > + > static int evaluate_to_int_const_expr(struct expression *expr) > { > expr->ctype = &int_ctype; > @@ -362,29 +370,32 @@ static struct symbol_op overflow_p_op = { > }; > > > -static int eval_sync_compare_and_swap(struct expression *expr) > +static int eval_atomic_common(struct expression *expr) > { > + struct symbol *fntype = expr->fn->ctype->ctype.base_type; > struct symbol_list *types = NULL; > struct symbol *ctype = NULL; > + struct symbol *t; > struct expression *arg; > int n = 0; > > - /* the first arg is a pointer type; we'd already verified that */ > + // The number of arguments have already be verified. > + // The first arg must be a pointer type to an integral type. s/pointer type to/pointer to/ (it just sounds odd, otherwise) or maybe ... must be a 'pointer to an integral' type. ? ATB, Ramsay Jones > + PREPARE_PTR_LIST(fntype->arguments, t); > FOR_EACH_PTR(expr->args, arg) { > - struct symbol *t = arg->ctype; > + struct symbol *ptrtype = NULL; > > - if (!t) > - return 0; > - > - // 2nd & 3rd args must be a basic integer type or a pointer > - // 1st arg must be a pointer to such a type. > if (++n == 1) { > + t = arg->ctype; > + if (!t) > + return 0; > if (t->type == SYM_NODE) > t = t->ctype.base_type; > if (!t) > return 0; > if (t->type != SYM_PTR) > goto err; > + ptrtype = t; > t = t->ctype.base_type; > if (!t) > return 0; > @@ -395,11 +406,14 @@ static int eval_sync_compare_and_swap(struct expression *expr) > if (t->type != SYM_PTR && t->ctype.base_type != &int_type) > goto err; > ctype = t; > - add_ptr_list(&types, arg->ctype); > - } else { > - add_ptr_list(&types, ctype); > + t = ptrtype; > + } else if (is_dynamic_type(t)) { > + t = ctype; > } > + add_ptr_list(&types, t); > + NEXT_PTR_LIST(t); > } END_FOR_EACH_PTR(arg); > + FINISH_PTR_LIST(t); > > if (!expr->ctype) // __sync_val_compare_and_swap() > expr->ctype = ctype; > @@ -412,9 +426,9 @@ err: > return 0; > } > > -static struct symbol_op sync_compare_and_swap_op = { > - .args = args_triadic, > - .evaluate = eval_sync_compare_and_swap, > +static struct symbol_op atomic_op = { > + .args = args_prototype, > + .evaluate = eval_atomic_common, > }; > > > @@ -464,6 +478,7 @@ static void declare_builtins(int stream, const struct builtin_fn tbl[]) > static const struct builtin_fn builtins_common[] = { > #define size_t_ctype &size_t_alias > #define va_list_ctype &ptr_ctype > +#define vol_ptr &volatile_ptr_ctype > { "__builtin_choose_expr", NULL, 1, .op = &choose_op }, > { "__builtin_constant_p", NULL, 1, .op = &constant_p_op }, > { "__builtin_expect", &long_ctype, 0, { &long_ctype ,&long_ctype }, .op = &expect_op }, > @@ -614,7 +629,7 @@ static const struct builtin_fn builtins_common[] = { > > { "__sync_add_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > { "__sync_and_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_bool_compare_and_swap", &bool_ctype, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_op}, > + { "__sync_bool_compare_and_swap", &bool_ctype, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op}, > { "__sync_fetch_and_add", &int_ctype, 1, { &ptr_ctype }}, > { "__sync_fetch_and_and", &int_ctype, 1, { &ptr_ctype }}, > { "__sync_fetch_and_nand", &int_ctype, 1, { &ptr_ctype }}, > @@ -627,7 +642,7 @@ static const struct builtin_fn builtins_common[] = { > { "__sync_or_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > { "__sync_sub_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > { "__sync_synchronize", &void_ctype, 0 }, > - { "__sync_val_compare_and_swap", NULL, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_op }, > + { "__sync_val_compare_and_swap", NULL, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op }, > { "__sync_xor_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > > { } >
On 17/10/2020 23:56, Luc Van Oostenryck wrote: > Reuse the generic method for all these builtins. > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > builtin.c | 26 +++++++++++++------------- > validation/builtin-sync-fetch.c | 24 ++++++++++++++++++++++++ > 2 files changed, 37 insertions(+), 13 deletions(-) > create mode 100644 validation/builtin-sync-fetch.c > > diff --git a/builtin.c b/builtin.c > index 880dd54f647e..5e490830e147 100644 > --- a/builtin.c > +++ b/builtin.c > @@ -627,23 +627,23 @@ static const struct builtin_fn builtins_common[] = { > { "__builtin___vsnprintf_chk", &int_ctype, 0, { &string_ctype, size_t_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }}, > { "__builtin___vsprintf_chk", &int_ctype, 0, { &string_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }}, > > - { "__sync_add_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_and_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > + { "__sync_add_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_and_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, Hmm, I get that the return type is derived from the type of the first argument, but I don't see where that return type is checked/assigned to the function return type. :( So, why are these set to NULL, but ... > { "__sync_bool_compare_and_swap", &bool_ctype, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op}, ... this one isn't? ATB, Ramsay Jones > - { "__sync_fetch_and_add", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_fetch_and_and", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_fetch_and_nand", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_fetch_and_or", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_fetch_and_sub", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_fetch_and_xor", &int_ctype, 1, { &ptr_ctype }}, > + { "__sync_fetch_and_add", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_fetch_and_and", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_fetch_and_nand", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_fetch_and_or", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_fetch_and_sub", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_fetch_and_xor", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > { "__sync_lock_release", &void_ctype, 1, { &ptr_ctype }}, > - { "__sync_lock_test_and_set", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_nand_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_or_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > - { "__sync_sub_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > + { "__sync_lock_test_and_set", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_nand_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_or_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > + { "__sync_sub_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > { "__sync_synchronize", &void_ctype, 0 }, > { "__sync_val_compare_and_swap", NULL, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op }, > - { "__sync_xor_and_fetch", &int_ctype, 1, { &ptr_ctype }}, > + { "__sync_xor_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op }, > > { } > }; > diff --git a/validation/builtin-sync-fetch.c b/validation/builtin-sync-fetch.c > new file mode 100644 > index 000000000000..45139a3c8c3e > --- /dev/null > +++ b/validation/builtin-sync-fetch.c > @@ -0,0 +1,24 @@ > +static int ok_int(int *ptr, int val) > +{ > + return __sync_add_and_fetch(ptr, val); > +} > + > +static long* ok_ptr(long **ptr, long *val) > +{ > + return __sync_add_and_fetch(ptr, val); > +} > + > +static void chk_ret_ok(long *ptr, long val) > +{ > + _Static_assert([typeof(__sync_add_and_fetch(ptr, val))] == [long], ""); > +} > + > +static int chk_val(int *ptr, long val) > +{ > + // OK: val is converted to an int > + return __sync_add_and_fetch(ptr, val); > +} > + > +/* > + * check-name: builtin-sync-fetch > + */ >
On Mon, Oct 19, 2020 at 12:59:01AM +0100, Ramsay Jones wrote:
> On 17/10/2020 23:56, Luc Van Oostenryck wrote:
> > diff --git a/builtin.c b/builtin.c
> > index 880dd54f647e..5e490830e147 100644
> > --- a/builtin.c
> > +++ b/builtin.c
> > @@ -627,23 +627,23 @@ static const struct builtin_fn builtins_common[] = {
> > { "__builtin___vsnprintf_chk", &int_ctype, 0, { &string_ctype, size_t_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }},
> > { "__builtin___vsprintf_chk", &int_ctype, 0, { &string_ctype, &int_ctype, size_t_ctype, &const_string_ctype, va_list_ctype }},
> >
> > - { "__sync_add_and_fetch", &int_ctype, 1, { &ptr_ctype }},
> > - { "__sync_and_and_fetch", &int_ctype, 1, { &ptr_ctype }},
> > + { "__sync_add_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op },
> > + { "__sync_and_and_fetch", NULL, 1, { vol_ptr, &dyntype }, .op = &atomic_op },
>
> Hmm, I get that the return type is derived from the type of the first
> argument, but I don't see where that return type is checked/assigned
> to the function return type. :( So, why are these set to NULL, but ...
>
> > { "__sync_bool_compare_and_swap", &bool_ctype, 1, { vol_ptr, &dyntype, &dyntype }, .op = &atomic_op},
>
> ... this one isn't?
Well, yes, __sync_bool_compare_and_swap() is defined as returning a bool,
so it can directly specified here. For the other functions, for example
__sync_add_and_fetch(), the return type must indeed be deduced form the
type of the first argument. It is set at the end of eval_atomic_common()
if (!expr->ctype) // __sync_val_compare_and_swap()
expr->ctype = ctype;
The comment should be removed or replaced by a real one.
I'll clarify that. Thanks for noticng this.
-- Luc
On Sun, Oct 18, 2020 at 10:31:55PM +0100, Ramsay Jones wrote:
> On 17/10/2020 23:56, Luc Van Oostenryck wrote:
> > Most __sync_* or __atomic_* builtin functions have one or
> > more arguments having its real type determined by the first
> > argument: either the same type (a pointer to an integral type)
> > or the type of the object it points to.
> >
> > Currently, only __sync_{bool,val}_compare_and_swap() are handled
> > but lots of very similar variants would be needed for the others.
> > So, make it a generic function, able to handle all these builtins.
> >
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > ---
> > builtin.c | 47 +++++++++++++++++++++++++++++++----------------
> > 1 file changed, 31 insertions(+), 16 deletions(-)
> >
> > diff --git a/builtin.c b/builtin.c
> > index 0d4cb12cca22..880dd54f647e 100644
> > --- a/builtin.c
> > +++ b/builtin.c
> > @@ -31,6 +31,14 @@
> > #include "compat/bswap.h"
> > #include <stdarg.h>
> >
> > +#define dyntype incomplete_ctype
> > +static bool is_dynamic_type(struct symbol *t)
> > +{
> > + if (t->type == SYM_NODE)
> > + t = t->ctype.base_type;
> > + return t == &dyntype;
> > +}
> > +
> > static int evaluate_to_int_const_expr(struct expression *expr)
> > {
> > expr->ctype = &int_ctype;
> > @@ -362,29 +370,32 @@ static struct symbol_op overflow_p_op = {
> > };
> >
> >
> > -static int eval_sync_compare_and_swap(struct expression *expr)
> > +static int eval_atomic_common(struct expression *expr)
> > {
> > + struct symbol *fntype = expr->fn->ctype->ctype.base_type;
> > struct symbol_list *types = NULL;
> > struct symbol *ctype = NULL;
> > + struct symbol *t;
> > struct expression *arg;
> > int n = 0;
> >
> > - /* the first arg is a pointer type; we'd already verified that */
> > + // The number of arguments have already be verified.
> > + // The first arg must be a pointer type to an integral type.
>
> s/pointer type to/pointer to/
> (it just sounds odd, otherwise)
> or maybe ... must be a 'pointer to an integral' type. ?
Well, yes, it's a bit hard to express clearly. The real infos are:
* it must be a 'struct symbol *', the are used for any type, pointer or not
* the type represented by this 'struct symbol *' must indeed be a pointer :
(first_args->type == SYM_PTR) to an integral type:
(first_args->ctype.base_type = &int_ctype).
So yes, "must be a 'pointer to an integral' type. " seems to do the job.
Thanks
-- Luc