[PATCH 0/3] fix evaluation of __sync_{bool,val}_compare_and_swap()

[PATCH 0/3] fix evaluation of __sync_{bool,val}_compare_and_swap()

[Luc Van Oostenryck]

The builtins __sync_{bool,val}_compare_and_swap() were known
to sparse but the type of the arguments were not checked, and
more importantly, the return type was always 'int'.

This series adds real support for these polymorphic builtins.


Luc Van Oostenryck (3):
  add testcases for __sync_{bool,val}_compare_and_swap()
  export evaluate_arguments()
  add builtin support for __sync_{bool,val}_compare_and_swap()

 builtin.c                     | 60 +++++++++++++++++++++++++++++++++--
 evaluate.c                    |  7 ++--
 evaluate.h                    |  7 ++++
 validation/builtin-sync-cas.c | 25 +++++++++++++++
 4 files changed, 93 insertions(+), 6 deletions(-)
 create mode 100644 validation/builtin-sync-cas.c


base-commit: e1578773182e8f69c3a0cd8add8dfbe7561a8240
-- 
2.28.0

[PATCH 1/3] add testcases for __sync_{bool,val}_compare_and_swap()

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/builtin-sync-cas.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/validation/builtin-sync-cas.c b/validation/builtin-sync-cas.c
new file mode 100644
index 000000000000..e289eba2949b
--- /dev/null
+++ b/validation/builtin-sync-cas.c
@@ -0,0 +1,26 @@
+static int *foo(int *ptr)
+{
+	__sync_val_compare_and_swap(ptr, 123, 0L);
+	return __sync_val_compare_and_swap(&ptr, ptr, ptr);
+}
+
+static long bar(long *ptr)
+{
+	return __sync_val_compare_and_swap(ptr, ptr, 1);
+}
+
+static _Bool boz(_Bool *ptr)
+{
+	return __sync_bool_compare_and_swap(ptr, 0, ptr);
+}
+
+/*
+ * check-name: builtin-sync-cas
+ * check-known-to-fail
+ *
+ * check-error-start
+builtin-sync-cas.c:9:49: warning: incorrect type in argument 2 (different base types)
+builtin-sync-cas.c:9:49:    expected long
+builtin-sync-cas.c:9:49:    got long *ptr
+ * check-error-end
+ */
-- 
2.28.0

[PATCH 2/3] export evaluate_arguments()

[Luc Van Oostenryck]

evaluate_arguments() is local to evaluate.c but the same functionality
is needed for builtins.

So, in order to not duplicate this code:
*) change slightly its interface to accept the expected types as a list
*) make this function extern.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 7 +++----
 evaluate.h | 7 +++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index dddea76182ad..7c63cc60e91b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2333,14 +2333,13 @@ static struct symbol *evaluate_alignof(struct expression *expr)
 	return size_t_ctype;
 }
 
-static int evaluate_arguments(struct symbol *fn, struct expression_list *head)
+int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *head)
 {
 	struct expression *expr;
-	struct symbol_list *argument_types = fn->arguments;
 	struct symbol *argtype;
 	int i = 1;
 
-	PREPARE_PTR_LIST(argument_types, argtype);
+	PREPARE_PTR_LIST(argtypes, argtype);
 	FOR_EACH_PTR (head, expr) {
 		struct expression **p = THIS_ADDRESS(expr);
 		struct symbol *ctype, *target;
@@ -3149,7 +3148,7 @@ static struct symbol *evaluate_call(struct expression *expr)
 		if (!sym->op->args(expr))
 			return NULL;
 	} else {
-		if (!evaluate_arguments(ctype, arglist))
+		if (!evaluate_arguments(ctype->arguments, arglist))
 			return NULL;
 		args = expression_list_size(expr->args);
 		fnargs = symbol_list_size(ctype->arguments);
diff --git a/evaluate.h b/evaluate.h
index f68f7fb7c367..a16e97036b2a 100644
--- a/evaluate.h
+++ b/evaluate.h
@@ -2,6 +2,7 @@
 #define EVALUATE_H
 
 struct expression;
+struct expression_list;
 struct statement;
 struct symbol;
 struct symbol_list;
@@ -25,4 +26,10 @@ struct symbol *evaluate_statement(struct statement *stmt);
 // @list: the list of the symbol to be evaluated
 void evaluate_symbol_list(struct symbol_list *list);
 
+///
+// evaluate the arguments of a function
+// @argtypes: the list of the types in the prototype
+// @args: the list of the effective arguments
+int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *args);
+
 #endif
-- 
2.28.0

[PATCH 3/3] add builtin support for __sync_{bool,val}_compare_and_swap()

[Luc Van Oostenryck]

In the kernel, the architecture s390 uses these builtins to
implement __atomic_cmpxchg() and friends. These builtins
are polymorphic, so they need some special evaluation.
These builtins are known to sparse but with a return type
of 'int' and the argument's types being ignored.

A problem occurs when used on a pointer type: the expected
type doesn't match 'int' and it can give warnings like:
	warning: non size-preserving integer to pointer cast

So, improve the support for these builtins by:
*) checking the number of arguments
*) extract the type from the 1st argument
*) set the returned type to this type if needed
*) finally, do the typechecking by calling evaluate_arguments()

Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/202008072005.Myrby1lg%25lkp@intel.com/
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 builtin.c                     | 60 +++++++++++++++++++++++++++++++++--
 validation/builtin-sync-cas.c |  1 -
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/builtin.c b/builtin.c
index f29b4a8d518c..2e9be8be8adb 100644
--- a/builtin.c
+++ b/builtin.c
@@ -355,6 +355,62 @@ static struct symbol_op overflow_p_op = {
 };
 
 
+static int eval_sync_compare_and_swap(struct expression *expr)
+{
+	struct symbol_list *types = NULL;
+	struct symbol *ctype = NULL;
+	struct expression *arg;
+	int n = 0;
+
+	/* the first arg is a pointer type; we'd already verified that */
+	FOR_EACH_PTR(expr->args, arg) {
+		struct symbol *t = arg->ctype;
+
+		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) {
+			if (t->type == SYM_NODE)
+				t = t->ctype.base_type;
+			if (!t)
+				return 0;
+			if (t->type != SYM_PTR)
+				goto err;
+			t = t->ctype.base_type;
+			if (!t)
+				return 0;
+			if (t->type == SYM_NODE)
+				t = t->ctype.base_type;
+			if (!t)
+				return 0;
+			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);
+		}
+	} END_FOR_EACH_PTR(arg);
+
+	if (!expr->ctype)	// __sync_val_compare_and_swap()
+		expr->ctype = ctype;
+	return evaluate_arguments(types, expr->args);
+
+err:
+	sparse_error(arg->pos, "invalid type for argument %d:", n);
+	info(arg->pos, "        %s", show_typename(arg->ctype));
+	expr->ctype = &bad_ctype;
+	return 0;
+}
+
+static struct symbol_op sync_compare_and_swap_op = {
+	.args = args_triadic,
+	.evaluate = eval_sync_compare_and_swap,
+};
+
+
 /*
  * Builtin functions
  */
@@ -548,7 +604,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", &int_ctype, 1, { &ptr_ctype }},
+	{ "__sync_bool_compare_and_swap", &bool_ctype, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_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 }},
@@ -561,7 +617,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", &int_ctype, 1, { &ptr_ctype }},
+	{ "__sync_val_compare_and_swap", NULL, 1, { &ptr_ctype }, .op = &sync_compare_and_swap_op },
 	{ "__sync_xor_and_fetch", &int_ctype, 1, { &ptr_ctype }},
 
 	{ }
diff --git a/validation/builtin-sync-cas.c b/validation/builtin-sync-cas.c
index e289eba2949b..846e21bb2fbb 100644
--- a/validation/builtin-sync-cas.c
+++ b/validation/builtin-sync-cas.c
@@ -16,7 +16,6 @@ static _Bool boz(_Bool *ptr)
 
 /*
  * check-name: builtin-sync-cas
- * check-known-to-fail
  *
  * check-error-start
 builtin-sync-cas.c:9:49: warning: incorrect type in argument 2 (different base types)
-- 
2.28.0