[PATCH 0/3] builtins expansion

[PATCH 0/3] builtins expansion

[Luc Van Oostenryck]

This serie propose a clean solution to the expansion of some builtins
like __builtin_bswap16() which gcc consider as an integer constant
expression when the arg is itself an integer constant and is used as
such in the kernel in code like:
	#define htons(x) __builtin_bswap16(x)
	...
	switch (protocol) {
	case htons(ETH_P_IPV6):
		...

This serie needs to be applied on top of Johannes Berg's patch
concerning the same problem and the tests depends on the testsuite
extensions posted previously.

Alternatively, this serie can also be found as:
  git://github.com/lucvoo/sparse.git sent/builtin-bswap
      

Luc Van Oostenryck (3):
  move evaluation & expansion of builtins in a separate file
  allow builtins to have prototype and evaluate/expand methods
  expand __builtin_bswap*() with constant args

 Makefile                            |   1 +
 builtin.c                           | 241 ++++++++++++++++++++++++++++++++++++
 evaluate.c                          |   6 +
 expand.c                            |  24 +---
 expand.h                            |  34 +++++
 lib.c                               |  35 +-----
 lib.h                               |   5 +
 symbol.c                            | 160 +-----------------------
 symbol.h                            |   3 +-
 token.h                             |   1 +
 validation/builtin-bswap-constant.c |  48 +++++++
 validation/builtin-bswap.c          |  52 ++++++++
 12 files changed, 394 insertions(+), 216 deletions(-)
 create mode 100644 builtin.c
 create mode 100644 expand.h
 create mode 100644 validation/builtin-bswap-constant.c
 create mode 100644 validation/builtin-bswap.c

[PATCH 1/3] move evaluation & expansion of builtins in a separate file

[Luc Van Oostenryck]

No functional changes, just move some code around, and rename
'eval_init_table[]' to 'builtins_table[]'.
---
 Makefile  |   1 +
 builtin.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 expand.c  |  24 +------
 expand.h  |  34 ++++++++++
 symbol.c  | 160 +----------------------------------------------
 symbol.h  |   3 +-
 6 files changed, 248 insertions(+), 184 deletions(-)
 create mode 100644 builtin.c
 create mode 100644 expand.h

diff --git a/Makefile b/Makefile
index f59993f7c..76902b75e 100644
--- a/Makefile
+++ b/Makefile
@@ -105,6 +105,7 @@ LIB_H=    token.h parse.h lib.h symbol.h scope.h expression.h target.h \
 LIB_OBJS= target.o parse.o tokenize.o pre-process.o symbol.o lib.o scope.o \
 	  expression.o show-parse.o evaluate.o expand.o inline.o linearize.o \
 	  char.o sort.o allocate.o compat-$(OS).o ptrlist.o \
+	  builtin.o \
 	  flow.o cse.o simplify.o memops.o liveness.o storage.o unssa.o dissect.o
 
 LIB_FILE= libsparse.a
diff --git a/builtin.c b/builtin.c
new file mode 100644
index 000000000..c6c97ed85
--- /dev/null
+++ b/builtin.c
@@ -0,0 +1,210 @@
+/*
+ * builtin evaluation & expansion.
+ *
+ * Copyright (C) 2003 Transmeta Corp.
+ *               2003-2004 Linus Torvalds
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "expression.h"
+#include "expand.h"
+#include "symbol.h"
+
+static int evaluate_to_integer(struct expression *expr)
+{
+	expr->ctype = &int_ctype;
+	return 1;
+}
+
+static int evaluate_expect(struct expression *expr)
+{
+	/* Should we evaluate it to return the type of the first argument? */
+	expr->ctype = &int_ctype;
+	return 1;
+}
+
+static int arguments_choose(struct expression *expr)
+{
+	struct expression_list *arglist = expr->args;
+	struct expression *arg;
+	int i = 0;
+
+	FOR_EACH_PTR (arglist, arg) {
+		if (!evaluate_expression(arg))
+			return 0;
+		i++;
+	} END_FOR_EACH_PTR(arg);
+	if (i < 3) {
+		sparse_error(expr->pos,
+			     "not enough arguments for __builtin_choose_expr");
+		return 0;
+	} if (i > 3) {
+		sparse_error(expr->pos,
+			     "too many arguments for __builtin_choose_expr");
+		return 0;
+	}
+	return 1;
+}
+
+static int evaluate_choose(struct expression *expr)
+{
+	struct expression_list *list = expr->args;
+	struct expression *arg, *args[3];
+	int n = 0;
+
+	/* there will be exactly 3; we'd already verified that */
+	FOR_EACH_PTR(list, arg) {
+		args[n++] = arg;
+	} END_FOR_EACH_PTR(arg);
+
+	*expr = get_expression_value(args[0]) ? *args[1] : *args[2];
+
+	return 1;
+}
+
+static int expand_expect(struct expression *expr, int cost)
+{
+	struct expression *arg = first_ptr_list((struct ptr_list *) expr->args);
+
+	if (arg)
+		*expr = *arg;
+	return 0;
+}
+
+/*
+ * __builtin_warning() has type "int" and always returns 1,
+ * so that you can use it in conditionals or whatever
+ */
+static int expand_warning(struct expression *expr, int cost)
+{
+	struct expression *arg;
+	struct expression_list *arglist = expr->args;
+
+	FOR_EACH_PTR (arglist, arg) {
+		/*
+		 * Constant strings get printed out as a warning. By the
+		 * time we get here, the EXPR_STRING has been fully 
+		 * evaluated, so by now it's an anonymous symbol with a
+		 * string initializer.
+		 *
+		 * Just for the heck of it, allow any constant string
+		 * symbol.
+		 */
+		if (arg->type == EXPR_SYMBOL) {
+			struct symbol *sym = arg->symbol;
+			if (sym->initializer && sym->initializer->type == EXPR_STRING) {
+				struct string *string = sym->initializer->string;
+				warning(expr->pos, "%*s", string->length-1, string->data);
+			}
+			continue;
+		}
+
+		/*
+		 * Any other argument is a conditional. If it's
+		 * non-constant, or it is false, we exit and do
+		 * not print any warning.
+		 */
+		if (arg->type != EXPR_VALUE)
+			goto out;
+		if (!arg->value)
+			goto out;
+	} END_FOR_EACH_PTR(arg);
+out:
+	expr->type = EXPR_VALUE;
+	expr->value = 1;
+	expr->taint = 0;
+	return 0;
+}
+
+/* The arguments are constant if the cost of all of them is zero */
+static int expand_constant_p(struct expression *expr, int cost)
+{
+	expr->type = EXPR_VALUE;
+	expr->value = !cost;
+	expr->taint = 0;
+	return 0;
+}
+
+/* The arguments are safe, if their cost is less than SIDE_EFFECTS */
+static int expand_safe_p(struct expression *expr, int cost)
+{
+	expr->type = EXPR_VALUE;
+	expr->value = (cost < SIDE_EFFECTS);
+	expr->taint = 0;
+	return 0;
+}
+
+static struct symbol_op constant_p_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_constant_p
+};
+
+static struct symbol_op safe_p_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_safe_p
+};
+
+static struct symbol_op warning_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_warning
+};
+
+static struct symbol_op expect_op = {
+	.evaluate = evaluate_expect,
+	.expand = expand_expect
+};
+
+static struct symbol_op choose_op = {
+	.evaluate = evaluate_choose,
+	.args = arguments_choose,
+};
+
+
+/*
+ * Builtin functions
+ */
+static struct symbol builtin_fn_type = { .type = SYM_FN /* , .variadic =1 */ };
+static struct sym_init {
+	const char *name;
+	struct symbol *base_type;
+	unsigned int modifiers;
+	struct symbol_op *op;
+} builtins_table[] = {
+	{ "__builtin_constant_p", &builtin_fn_type, MOD_TOPLEVEL, &constant_p_op },
+	{ "__builtin_safe_p", &builtin_fn_type, MOD_TOPLEVEL, &safe_p_op },
+	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
+	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
+	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
+	{ NULL,		NULL,		0 }
+};
+
+void init_builtins(int stream)
+{
+	struct sym_init *ptr;
+
+	builtin_fn_type.variadic = 1;
+	for (ptr = builtins_table; ptr->name; ptr++) {
+		struct symbol *sym;
+		sym = create_symbol(stream, ptr->name, SYM_NODE, NS_SYMBOL);
+		sym->ctype.base_type = ptr->base_type;
+		sym->ctype.modifiers = ptr->modifiers;
+		sym->op = ptr->op;
+	}
+}
diff --git a/expand.c b/expand.c
index 0f6720c44..7af12707e 100644
--- a/expand.c
+++ b/expand.c
@@ -41,12 +41,8 @@
 #include "symbol.h"
 #include "target.h"
 #include "expression.h"
+#include "expand.h"
 
-/* Random cost numbers */
-#define SIDE_EFFECTS 10000	/* The expression has side effects */
-#define UNSAFE 100		/* The expression may be "infinitely costly" due to exceptions */
-#define SELECT_COST 20		/* Cut-off for turning a conditional into a select */
-#define BRANCH_COST 10		/* Cost of a conditional branch */
 
 static int expand_expression(struct expression *);
 static int expand_statement(struct statement *);
@@ -784,24 +780,6 @@ static int expand_cast(struct expression *expr)
 	return cost + 1;
 }
 
-/* The arguments are constant if the cost of all of them is zero */
-int expand_constant_p(struct expression *expr, int cost)
-{
-	expr->type = EXPR_VALUE;
-	expr->value = !cost;
-	expr->taint = 0;
-	return 0;
-}
-
-/* The arguments are safe, if their cost is less than SIDE_EFFECTS */
-int expand_safe_p(struct expression *expr, int cost)
-{
-	expr->type = EXPR_VALUE;
-	expr->value = (cost < SIDE_EFFECTS);
-	expr->taint = 0;
-	return 0;
-}
-
 /*
  * expand a call expression with a symbol. This
  * should expand builtins.
diff --git a/expand.h b/expand.h
new file mode 100644
index 000000000..27e10c0a8
--- /dev/null
+++ b/expand.h
@@ -0,0 +1,34 @@
+#ifndef EXPAND_H
+#define EXPAND_H
+/*
+ * sparse/expand.h
+ *
+ * Copyright (C) 2003 Transmeta Corp.
+ *               2003 Linus Torvalds
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/* Random cost numbers */
+#define SIDE_EFFECTS 10000	/* The expression has side effects */
+#define UNSAFE 100		/* The expression may be "infinitely costly" due to exceptions */
+#define SELECT_COST 20		/* Cut-off for turning a conditional into a select */
+#define BRANCH_COST 10		/* Cost of a conditional branch */
+
+#endif
diff --git a/symbol.c b/symbol.c
index 92a7a6253..5b6cb7fde 100644
--- a/symbol.c
+++ b/symbol.c
@@ -642,155 +642,6 @@ struct symbol *create_symbol(int stream, const char *name, int type, int namespa
 	return sym;
 }
 
-static int evaluate_to_integer(struct expression *expr)
-{
-	expr->ctype = &int_ctype;
-	return 1;
-}
-
-static int evaluate_expect(struct expression *expr)
-{
-	/* Should we evaluate it to return the type of the first argument? */
-	expr->ctype = &int_ctype;
-	return 1;
-}
-
-static int arguments_choose(struct expression *expr)
-{
-	struct expression_list *arglist = expr->args;
-	struct expression *arg;
-	int i = 0;
-
-	FOR_EACH_PTR (arglist, arg) {
-		if (!evaluate_expression(arg))
-			return 0;
-		i++;
-	} END_FOR_EACH_PTR(arg);
-	if (i < 3) {
-		sparse_error(expr->pos,
-			     "not enough arguments for __builtin_choose_expr");
-		return 0;
-	} if (i > 3) {
-		sparse_error(expr->pos,
-			     "too many arguments for __builtin_choose_expr");
-		return 0;
-	}
-	return 1;
-}
-
-static int evaluate_choose(struct expression *expr)
-{
-	struct expression_list *list = expr->args;
-	struct expression *arg, *args[3];
-	int n = 0;
-
-	/* there will be exactly 3; we'd already verified that */
-	FOR_EACH_PTR(list, arg) {
-		args[n++] = arg;
-	} END_FOR_EACH_PTR(arg);
-
-	*expr = get_expression_value(args[0]) ? *args[1] : *args[2];
-
-	return 1;
-}
-
-static int expand_expect(struct expression *expr, int cost)
-{
-	struct expression *arg = first_ptr_list((struct ptr_list *) expr->args);
-
-	if (arg)
-		*expr = *arg;
-	return 0;
-}
-
-/*
- * __builtin_warning() has type "int" and always returns 1,
- * so that you can use it in conditionals or whatever
- */
-static int expand_warning(struct expression *expr, int cost)
-{
-	struct expression *arg;
-	struct expression_list *arglist = expr->args;
-
-	FOR_EACH_PTR (arglist, arg) {
-		/*
-		 * Constant strings get printed out as a warning. By the
-		 * time we get here, the EXPR_STRING has been fully 
-		 * evaluated, so by now it's an anonymous symbol with a
-		 * string initializer.
-		 *
-		 * Just for the heck of it, allow any constant string
-		 * symbol.
-		 */
-		if (arg->type == EXPR_SYMBOL) {
-			struct symbol *sym = arg->symbol;
-			if (sym->initializer && sym->initializer->type == EXPR_STRING) {
-				struct string *string = sym->initializer->string;
-				warning(expr->pos, "%*s", string->length-1, string->data);
-			}
-			continue;
-		}
-
-		/*
-		 * Any other argument is a conditional. If it's
-		 * non-constant, or it is false, we exit and do
-		 * not print any warning.
-		 */
-		if (arg->type != EXPR_VALUE)
-			goto out;
-		if (!arg->value)
-			goto out;
-	} END_FOR_EACH_PTR(arg);
-out:
-	expr->type = EXPR_VALUE;
-	expr->value = 1;
-	expr->taint = 0;
-	return 0;
-}
-
-static struct symbol_op constant_p_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_constant_p
-};
-
-static struct symbol_op safe_p_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_safe_p
-};
-
-static struct symbol_op warning_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_warning
-};
-
-static struct symbol_op expect_op = {
-	.evaluate = evaluate_expect,
-	.expand = expand_expect
-};
-
-static struct symbol_op choose_op = {
-	.evaluate = evaluate_choose,
-	.args = arguments_choose,
-};
-
-/*
- * Builtin functions
- */
-static struct symbol builtin_fn_type = { .type = SYM_FN /* , .variadic =1 */ };
-static struct sym_init {
-	const char *name;
-	struct symbol *base_type;
-	unsigned int modifiers;
-	struct symbol_op *op;
-} eval_init_table[] = {
-	{ "__builtin_constant_p", &builtin_fn_type, MOD_TOPLEVEL, &constant_p_op },
-	{ "__builtin_safe_p", &builtin_fn_type, MOD_TOPLEVEL, &safe_p_op },
-	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
-	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
-	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
-	{ NULL,		NULL,		0 }
-};
-
 
 /*
  * Abstract types
@@ -825,22 +676,13 @@ struct symbol	zero_int;
 void init_symbols(void)
 {
 	int stream = init_stream("builtin", -1, includepath);
-	struct sym_init *ptr;
 
 #define __IDENT(n,str,res) \
 	hash_ident(&n)
 #include "ident-list.h"
 
 	init_parser(stream);
-
-	builtin_fn_type.variadic = 1;
-	for (ptr = eval_init_table; ptr->name; ptr++) {
-		struct symbol *sym;
-		sym = create_symbol(stream, ptr->name, SYM_NODE, NS_SYMBOL);
-		sym->ctype.base_type = ptr->base_type;
-		sym->ctype.modifiers = ptr->modifiers;
-		sym->op = ptr->op;
-	}
+	init_builtins(stream);
 }
 
 #define MOD_ESIGNED (MOD_SIGNED | MOD_EXPLICITLY_SIGNED)
diff --git a/symbol.h b/symbol.h
index 9b3f1604e..f18306094 100644
--- a/symbol.h
+++ b/symbol.h
@@ -128,8 +128,6 @@ struct symbol_op {
 	int test, set, class;
 };
 
-extern int expand_safe_p(struct expression *expr, int cost);
-extern int expand_constant_p(struct expression *expr, int cost);
 
 #define SYM_ATTR_WEAK		0
 #define SYM_ATTR_NORMAL		1
@@ -289,6 +287,7 @@ extern const char * type_difference(struct ctype *c1, struct ctype *c2,
 extern struct symbol *lookup_symbol(struct ident *, enum namespace);
 extern struct symbol *create_symbol(int stream, const char *name, int type, int namespace);
 extern void init_symbols(void);
+extern void init_builtins(int stream);
 extern void init_ctype(void);
 extern struct symbol *alloc_symbol(struct position, int type);
 extern void show_type(struct symbol *);
-- 
2.11.0

[PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Luc Van Oostenryck]

So far, builtin functions which had some evaluate/expand method
couldn't also have a prototype because each would have its own symbol
and only the one for the prototype will be seen.
This also meant that the evaluate/expand functions had to take care
to set the correct types for they argumenst & results, which is fine
for some generic builtins like __builtin_constant_p() it's much less
practical for the ones like __builtin_bswap{16,32,64}().

Fix this by marking the idents for the builtins we declare some
evaluate/expand methods has being the ident of a builtin function
and later at evaluation time, to share the methods between all the symbols
corresponding to an identifier so marked.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 builtin.c  | 1 +
 evaluate.c | 6 ++++++
 token.h    | 1 +
 3 files changed, 8 insertions(+)

diff --git a/builtin.c b/builtin.c
index c6c97ed85..ddc71f785 100644
--- a/builtin.c
+++ b/builtin.c
@@ -205,6 +205,7 @@ void init_builtins(int stream)
 		sym = create_symbol(stream, ptr->name, SYM_NODE, NS_SYMBOL);
 		sym->ctype.base_type = ptr->base_type;
 		sym->ctype.modifiers = ptr->modifiers;
+		sym->ident->builtin = 1;
 		sym->op = ptr->op;
 	}
 }
diff --git a/evaluate.c b/evaluate.c
index 6d44e0d9c..f89e44b75 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2829,6 +2829,12 @@ static int evaluate_symbol_call(struct expression *expr)
 	if (fn->type != EXPR_PREOP)
 		return 0;
 
+	if (ctype->ident->builtin) {
+		struct symbol *next = ctype->next_id;
+		if (!ctype->op && next)
+			ctype->op = next->op;
+	}
+
 	if (ctype->op && ctype->op->evaluate)
 		return ctype->op->evaluate(expr);
 
diff --git a/token.h b/token.h
index f7d88eb45..7b9c7ab74 100644
--- a/token.h
+++ b/token.h
@@ -73,6 +73,7 @@ struct ident {
 	unsigned char len;	/* Length of identifier name */
 	unsigned char tainted:1,
 	              reserved:1,
+	              builtin:1,
 		      keyword:1;
 	char name[];		/* Actual identifier */
 };
-- 
2.11.0

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Chris Li]

[-- Attachment #1: Type: text/plain, Size: 2075 bytes --]

On Tue, Jan 24, 2017 at 5:37 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> So far, builtin functions which had some evaluate/expand method
> couldn't also have a prototype because each would have its own symbol
> and only the one for the prototype will be seen.
> This also meant that the evaluate/expand functions had to take care
> to set the correct types for they argumenst & results, which is fine
> for some generic builtins like __builtin_constant_p() it's much less
> practical for the ones like __builtin_bswap{16,32,64}().
>
> Fix this by marking the idents for the builtins we declare some
> evaluate/expand methods has being the ident of a builtin function
> and later at evaluation time, to share the methods between all the symbols
> corresponding to an identifier so marked.

This certainly fix the problem for builtin functions. However, I think there
is more general bug in sparse not limit to builtin functions. When the symbol
is declare twice, the later one did not migrate all the declare information from
the previous one.

In function external_declaration()

        check_declaration(decl);
        if (decl->same_symbol)
                decl->definition = decl->same_symbol->definition;

Here it only migrates the definition. I think if you add symbol->op in
the migration
as well, it *should* work for your case. I haven't test this myself.
If that works for you, it is a more general fix. Want to give it a try?

We might want to extract the migration code into a new function and
later adding new code it. e.g. function attributes.

A more complicate example will involve same symbol in different scope.

void __attr_x foo(void);

void a(void)
{
        void __attr_y foo(void); // foo should have both __attr_{x,y}
        {
                void _attr_z foo(void); // foo should have all __attr_{x,y,z}
        }
        // now foo should only have __attr_{x,y}
}

update:

I attach a quick and dirty patch file for you to try. It will replace
this 2/3 patch.
I just try the test suite and nothing seems complain about it.

Chris

[-- Attachment #2: migrate-same-symbol --]
[-- Type: application/octet-stream, Size: 355 bytes --]


--- sparse.chrisl.orig/parse.c
+++ sparse.chrisl/parse.c
@@ -2879,8 +2879,10 @@ struct token *external_declaration(struc
 			}
 		}
 		check_declaration(decl);
-		if (decl->same_symbol)
+		if (decl->same_symbol) {
 			decl->definition = decl->same_symbol->definition;
+			decl->op = decl->same_symbol->op;
+		}
 
 		if (!match_op(token, ','))
 			break;

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Luc Van Oostenryck]

On Wed, Feb 08, 2017 at 03:49:54AM +0800, Chris Li wrote:
> On Tue, Jan 24, 2017 at 5:37 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > So far, builtin functions which had some evaluate/expand method
> > couldn't also have a prototype because each would have its own symbol
> > and only the one for the prototype will be seen.
> > This also meant that the evaluate/expand functions had to take care
> > to set the correct types for they argumenst & results, which is fine
> > for some generic builtins like __builtin_constant_p() it's much less
> > practical for the ones like __builtin_bswap{16,32,64}().
> >
> > Fix this by marking the idents for the builtins we declare some
> > evaluate/expand methods has being the ident of a builtin function
> > and later at evaluation time, to share the methods between all the symbols
> > corresponding to an identifier so marked.
> 
> This certainly fix the problem for builtin functions. However, I think there
> is more general bug in sparse not limit to builtin functions. When the symbol
> is declare twice, the later one did not migrate all the declare information from
> the previous one.

I'm very well aware of this bug/problem, it creates all sort of complications
but I vaguely understood it was a design choice. To be 100%, you're talking
the fact that each declaration create a new symbol only related by their
identifier chain, right?

> In function external_declaration()
> 
>         check_declaration(decl);
>         if (decl->same_symbol)
>                 decl->definition = decl->same_symbol->definition;
> 
> Here it only migrates the definition. I think if you add symbol->op in
> the migration
> as well, it *should* work for your case. I haven't test this myself.
> If that works for you, it is a more general fix. Want to give it a try?
> 
> We might want to extract the migration code into a new function and
> later adding new code it. e.g. function attributes.

Yes and diagnose any compatibility.
I'll give it a try but I won't be able to do that before the weekend.

Luc

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Chris Li]

On Wed, Feb 8, 2017 at 4:12 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Wed, Feb 08, 2017 at 03:49:54AM +0800, Chris Li wrote:
>> On Tue, Jan 24, 2017 at 5:37 AM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>> > So far, builtin functions which had some evaluate/expand method
>> > couldn't also have a prototype because each would have its own symbol
>> > and only the one for the prototype will be seen.
>> > This also meant that the evaluate/expand functions had to take care
>> > to set the correct types for they argumenst & results, which is fine
>> > for some generic builtins like __builtin_constant_p() it's much less
>> > practical for the ones like __builtin_bswap{16,32,64}().
>> >
>> > Fix this by marking the idents for the builtins we declare some
>> > evaluate/expand methods has being the ident of a builtin function
>> > and later at evaluation time, to share the methods between all the symbols
>> > corresponding to an identifier so marked.
>>
>> This certainly fix the problem for builtin functions. However, I think there
>> is more general bug in sparse not limit to builtin functions. When the symbol
>> is declare twice, the later one did not migrate all the declare information from
>> the previous one.
>
> I'm very well aware of this bug/problem, it creates all sort of complications
> but I vaguely understood it was a design choice. To be 100%, you're talking
> the fact that each declaration create a new symbol only related by their
> identifier chain, right?
>
>> In function external_declaration()
>>
>>         check_declaration(decl);
>>         if (decl->same_symbol)
>>                 decl->definition = decl->same_symbol->definition;
>>
>> Here it only migrates the definition. I think if you add symbol->op in
>> the migration
>> as well, it *should* work for your case. I haven't test this myself.
>> If that works for you, it is a more general fix. Want to give it a try?
>>
>> We might want to extract the migration code into a new function and
>> later adding new code it. e.g. function attributes.
>
> Yes and diagnose any compatibility.
> I'll give it a try but I won't be able to do that before the weekend.
>
> Luc

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Christopher Li]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

On Wed, Feb 8, 2017 at 4:12 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> I'm very well aware of this bug/problem, it creates all sort of complications
> but I vaguely understood it was a design choice. To be 100%, you're talking
> the fact that each declaration create a new symbol only related by their
> identifier chain, right?

Yes. I don't see a way not creating the symbol for each declaration.
However, we can make a better job at migrating the bits between each
declaration.

>
> Yes and diagnose any compatibility.
> I'll give it a try but I won't be able to do that before the weekend.

No problem. Just in case you miss it. I attach a one line patch in my
previous email. I attach it here again.

Chris

[-- Attachment #2: migrate-same-symbol.patch --]
[-- Type: text/x-patch, Size: 355 bytes --]


--- sparse.chrisl.orig/parse.c
+++ sparse.chrisl/parse.c
@@ -2879,8 +2879,10 @@ struct token *external_declaration(struc
 			}
 		}
 		check_declaration(decl);
-		if (decl->same_symbol)
+		if (decl->same_symbol) {
 			decl->definition = decl->same_symbol->definition;
+			decl->op = decl->same_symbol->op;
+		}
 
 		if (!match_op(token, ','))
 			break;

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Luc Van Oostenryck]

On Wed, Feb 08, 2017 at 04:32:08AM +0800, Christopher Li wrote:
> On Wed, Feb 8, 2017 at 4:12 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > I'm very well aware of this bug/problem, it creates all sort of complications
> > but I vaguely understood it was a design choice. To be 100%, you're talking
> > the fact that each declaration create a new symbol only related by their
> > identifier chain, right?
> 
> Yes. I don't see a way not creating the symbol for each declaration.
> However, we can make a better job at migrating the bits between each
> declaration.

I see roughly a way how we could do that but I'm far from being
convinced it would be a good idea.
The problem is not that we have one symbol per declaration, it's
even not that we have several symbols for the same object, it's
that we don't have a central place holding the information about
the object.  Like, for example, we can't answer something essential
like "what is the type of this function" because we have as much
types as we have declarations. So yes, verification of the
comptability between the declarations and consolidate them is what
is missing.


Luc

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Christopher Li]

On Wed, Feb 8, 2017 at 5:34 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> I see roughly a way how we could do that but I'm far from being
> convinced it would be a good idea.
> The problem is not that we have one symbol per declaration, it's
> even not that we have several symbols for the same object, it's
> that we don't have a central place holding the information about
> the object.  Like, for example, we can't answer something essential
> like "what is the type of this function" because we have as much
> types as we have declarations.

As far as I can tell. You can't have one single place to hold type for
every declaration. If the declaration is exactly the same, yes.
But if the declaration was done in difference scope with different
incremental declare, you need to have different version of the same
symbol due to the C specification of scoping. See my previous example.
The field "same_symbol" is for that purpose tracking the same symbol
in different declares.

> So yes, verification of the
> comptability between the declarations and consolidate them is what
> is missing.

I think in evaluate.c check_duplicates() does the verification of incompatible
declares already. I agree consolidating is missing right now.

I am open to suggestions how to fix it.

Chris

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Luc Van Oostenryck]

On Wed, Feb 08, 2017 at 03:49:54AM +0800, Chris Li wrote:
> On Tue, Jan 24, 2017 at 5:37 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > So far, builtin functions which had some evaluate/expand method
> > couldn't also have a prototype because each would have its own symbol
> > and only the one for the prototype will be seen.
> > This also meant that the evaluate/expand functions had to take care
> > to set the correct types for they argumenst & results, which is fine
> > for some generic builtins like __builtin_constant_p() it's much less
> > practical for the ones like __builtin_bswap{16,32,64}().
> >
> > Fix this by marking the idents for the builtins we declare some
> > evaluate/expand methods has being the ident of a builtin function
> > and later at evaluation time, to share the methods between all the symbols
> > corresponding to an identifier so marked.
> 
> This certainly fix the problem for builtin functions. However, I think there
> is more general bug in sparse not limit to builtin functions. When the symbol
> is declare twice, the later one did not migrate all the declare information from
> the previous one.
> 
> In function external_declaration()
> 
>         check_declaration(decl);
>         if (decl->same_symbol)
>                 decl->definition = decl->same_symbol->definition;
> 
> Here it only migrates the definition. I think if you add symbol->op in
> the migration
> as well, it *should* work for your case. I haven't test this myself.
> If that works for you, it is a more general fix. Want to give it a try?

OK, I've look at this.
The difference between your patch and mine is that yours share/
migrate the methods for all symbols while mine purposely did that
for builtins only. The reasons I did this way was to avoid to share
methods between things that don't need to, for example if someone
define a function that have the same name as one of the numerous
attributes (which are not reserved keywords and aren't necessarily
protected by leading double underscore). But having look at this now,
I see that it should never be a problem when this happen (but is it
a good idea?).

Anyway, I've retest with your patch and everything is fine, as
expected, and I'm sending a new version of the serie.

Luc

[PATCH v2 0/3] builtins expansion

[Luc Van Oostenryck]

This serie solves the problem of the expansion of some builtins
like __builtin_bswap16() which gcc consider as an integer constant
expression when the arg is itself an integer constant.
Such builtins are used as such in the kernel and their non-expansion
create undesirable warnings from sparse.

This serie needs to be applied on top of Johannes Berg's patch
concerning the same problem and the tests depend on the testsuite
extensions posted previously.

Change since v1:
- simpler and more generic way to share the eval/expand ops
  thanks to Christopher Li
- small changes in the log messages
- change the variable name in the bswap expansion method.
- small reorganization of the test files

Luc Van Oostenryck (3):
  move evaluation & expansion of builtins in a separate file
  let identical symbols share their evaluate/expand methods
  expand __builtin_bswap*() with constant args

 Makefile                                           |   1 +
 builtin.c                                          | 240 +++++++++++++++++++++
 expand.c                                           |  24 +--
 expand.h                                           |  34 +++
 lib.c                                              |  35 +--
 lib.h                                              |   5 +
 parse.c                                            |   4 +-
 symbol.c                                           | 160 +-------------
 symbol.h                                           |   3 +-
 validation/builtin-args-checking.c                 |  45 ++++
 ...in-constant-eval.c => builtin-bswap-constant.c} |   2 +-
 validation/builtin-bswap-variable.c                |  32 +++
 12 files changed, 367 insertions(+), 218 deletions(-)
 create mode 100644 builtin.c
 create mode 100644 expand.h
 create mode 100644 validation/builtin-args-checking.c
 rename validation/{builtin-constant-eval.c => builtin-bswap-constant.c} (93%)
 create mode 100644 validation/builtin-bswap-variable.c

-- 
2.11.0

[PATCH v2 1/3] move evaluation & expansion of builtins in a separate file

[Luc Van Oostenryck]

No functional changes, just move some code around and rename
'eval_init_table[]' to 'builtins_table[]'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Makefile  |   1 +
 builtin.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 expand.c  |  24 +------
 expand.h  |  34 ++++++++++
 symbol.c  | 160 +----------------------------------------------
 symbol.h  |   3 +-
 6 files changed, 248 insertions(+), 184 deletions(-)
 create mode 100644 builtin.c
 create mode 100644 expand.h

diff --git a/Makefile b/Makefile
index f59993f7c..76902b75e 100644
--- a/Makefile
+++ b/Makefile
@@ -105,6 +105,7 @@ LIB_H=    token.h parse.h lib.h symbol.h scope.h expression.h target.h \
 LIB_OBJS= target.o parse.o tokenize.o pre-process.o symbol.o lib.o scope.o \
 	  expression.o show-parse.o evaluate.o expand.o inline.o linearize.o \
 	  char.o sort.o allocate.o compat-$(OS).o ptrlist.o \
+	  builtin.o \
 	  flow.o cse.o simplify.o memops.o liveness.o storage.o unssa.o dissect.o
 
 LIB_FILE= libsparse.a
diff --git a/builtin.c b/builtin.c
new file mode 100644
index 000000000..c6c97ed85
--- /dev/null
+++ b/builtin.c
@@ -0,0 +1,210 @@
+/*
+ * builtin evaluation & expansion.
+ *
+ * Copyright (C) 2003 Transmeta Corp.
+ *               2003-2004 Linus Torvalds
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "expression.h"
+#include "expand.h"
+#include "symbol.h"
+
+static int evaluate_to_integer(struct expression *expr)
+{
+	expr->ctype = &int_ctype;
+	return 1;
+}
+
+static int evaluate_expect(struct expression *expr)
+{
+	/* Should we evaluate it to return the type of the first argument? */
+	expr->ctype = &int_ctype;
+	return 1;
+}
+
+static int arguments_choose(struct expression *expr)
+{
+	struct expression_list *arglist = expr->args;
+	struct expression *arg;
+	int i = 0;
+
+	FOR_EACH_PTR (arglist, arg) {
+		if (!evaluate_expression(arg))
+			return 0;
+		i++;
+	} END_FOR_EACH_PTR(arg);
+	if (i < 3) {
+		sparse_error(expr->pos,
+			     "not enough arguments for __builtin_choose_expr");
+		return 0;
+	} if (i > 3) {
+		sparse_error(expr->pos,
+			     "too many arguments for __builtin_choose_expr");
+		return 0;
+	}
+	return 1;
+}
+
+static int evaluate_choose(struct expression *expr)
+{
+	struct expression_list *list = expr->args;
+	struct expression *arg, *args[3];
+	int n = 0;
+
+	/* there will be exactly 3; we'd already verified that */
+	FOR_EACH_PTR(list, arg) {
+		args[n++] = arg;
+	} END_FOR_EACH_PTR(arg);
+
+	*expr = get_expression_value(args[0]) ? *args[1] : *args[2];
+
+	return 1;
+}
+
+static int expand_expect(struct expression *expr, int cost)
+{
+	struct expression *arg = first_ptr_list((struct ptr_list *) expr->args);
+
+	if (arg)
+		*expr = *arg;
+	return 0;
+}
+
+/*
+ * __builtin_warning() has type "int" and always returns 1,
+ * so that you can use it in conditionals or whatever
+ */
+static int expand_warning(struct expression *expr, int cost)
+{
+	struct expression *arg;
+	struct expression_list *arglist = expr->args;
+
+	FOR_EACH_PTR (arglist, arg) {
+		/*
+		 * Constant strings get printed out as a warning. By the
+		 * time we get here, the EXPR_STRING has been fully 
+		 * evaluated, so by now it's an anonymous symbol with a
+		 * string initializer.
+		 *
+		 * Just for the heck of it, allow any constant string
+		 * symbol.
+		 */
+		if (arg->type == EXPR_SYMBOL) {
+			struct symbol *sym = arg->symbol;
+			if (sym->initializer && sym->initializer->type == EXPR_STRING) {
+				struct string *string = sym->initializer->string;
+				warning(expr->pos, "%*s", string->length-1, string->data);
+			}
+			continue;
+		}
+
+		/*
+		 * Any other argument is a conditional. If it's
+		 * non-constant, or it is false, we exit and do
+		 * not print any warning.
+		 */
+		if (arg->type != EXPR_VALUE)
+			goto out;
+		if (!arg->value)
+			goto out;
+	} END_FOR_EACH_PTR(arg);
+out:
+	expr->type = EXPR_VALUE;
+	expr->value = 1;
+	expr->taint = 0;
+	return 0;
+}
+
+/* The arguments are constant if the cost of all of them is zero */
+static int expand_constant_p(struct expression *expr, int cost)
+{
+	expr->type = EXPR_VALUE;
+	expr->value = !cost;
+	expr->taint = 0;
+	return 0;
+}
+
+/* The arguments are safe, if their cost is less than SIDE_EFFECTS */
+static int expand_safe_p(struct expression *expr, int cost)
+{
+	expr->type = EXPR_VALUE;
+	expr->value = (cost < SIDE_EFFECTS);
+	expr->taint = 0;
+	return 0;
+}
+
+static struct symbol_op constant_p_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_constant_p
+};
+
+static struct symbol_op safe_p_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_safe_p
+};
+
+static struct symbol_op warning_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_warning
+};
+
+static struct symbol_op expect_op = {
+	.evaluate = evaluate_expect,
+	.expand = expand_expect
+};
+
+static struct symbol_op choose_op = {
+	.evaluate = evaluate_choose,
+	.args = arguments_choose,
+};
+
+
+/*
+ * Builtin functions
+ */
+static struct symbol builtin_fn_type = { .type = SYM_FN /* , .variadic =1 */ };
+static struct sym_init {
+	const char *name;
+	struct symbol *base_type;
+	unsigned int modifiers;
+	struct symbol_op *op;
+} builtins_table[] = {
+	{ "__builtin_constant_p", &builtin_fn_type, MOD_TOPLEVEL, &constant_p_op },
+	{ "__builtin_safe_p", &builtin_fn_type, MOD_TOPLEVEL, &safe_p_op },
+	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
+	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
+	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
+	{ NULL,		NULL,		0 }
+};
+
+void init_builtins(int stream)
+{
+	struct sym_init *ptr;
+
+	builtin_fn_type.variadic = 1;
+	for (ptr = builtins_table; ptr->name; ptr++) {
+		struct symbol *sym;
+		sym = create_symbol(stream, ptr->name, SYM_NODE, NS_SYMBOL);
+		sym->ctype.base_type = ptr->base_type;
+		sym->ctype.modifiers = ptr->modifiers;
+		sym->op = ptr->op;
+	}
+}
diff --git a/expand.c b/expand.c
index 0f6720c44..7af12707e 100644
--- a/expand.c
+++ b/expand.c
@@ -41,12 +41,8 @@
 #include "symbol.h"
 #include "target.h"
 #include "expression.h"
+#include "expand.h"
 
-/* Random cost numbers */
-#define SIDE_EFFECTS 10000	/* The expression has side effects */
-#define UNSAFE 100		/* The expression may be "infinitely costly" due to exceptions */
-#define SELECT_COST 20		/* Cut-off for turning a conditional into a select */
-#define BRANCH_COST 10		/* Cost of a conditional branch */
 
 static int expand_expression(struct expression *);
 static int expand_statement(struct statement *);
@@ -784,24 +780,6 @@ static int expand_cast(struct expression *expr)
 	return cost + 1;
 }
 
-/* The arguments are constant if the cost of all of them is zero */
-int expand_constant_p(struct expression *expr, int cost)
-{
-	expr->type = EXPR_VALUE;
-	expr->value = !cost;
-	expr->taint = 0;
-	return 0;
-}
-
-/* The arguments are safe, if their cost is less than SIDE_EFFECTS */
-int expand_safe_p(struct expression *expr, int cost)
-{
-	expr->type = EXPR_VALUE;
-	expr->value = (cost < SIDE_EFFECTS);
-	expr->taint = 0;
-	return 0;
-}
-
 /*
  * expand a call expression with a symbol. This
  * should expand builtins.
diff --git a/expand.h b/expand.h
new file mode 100644
index 000000000..27e10c0a8
--- /dev/null
+++ b/expand.h
@@ -0,0 +1,34 @@
+#ifndef EXPAND_H
+#define EXPAND_H
+/*
+ * sparse/expand.h
+ *
+ * Copyright (C) 2003 Transmeta Corp.
+ *               2003 Linus Torvalds
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/* Random cost numbers */
+#define SIDE_EFFECTS 10000	/* The expression has side effects */
+#define UNSAFE 100		/* The expression may be "infinitely costly" due to exceptions */
+#define SELECT_COST 20		/* Cut-off for turning a conditional into a select */
+#define BRANCH_COST 10		/* Cost of a conditional branch */
+
+#endif
diff --git a/symbol.c b/symbol.c
index a65ad17b6..08c85f40e 100644
--- a/symbol.c
+++ b/symbol.c
@@ -646,155 +646,6 @@ struct symbol *create_symbol(int stream, const char *name, int type, int namespa
 	return sym;
 }
 
-static int evaluate_to_integer(struct expression *expr)
-{
-	expr->ctype = &int_ctype;
-	return 1;
-}
-
-static int evaluate_expect(struct expression *expr)
-{
-	/* Should we evaluate it to return the type of the first argument? */
-	expr->ctype = &int_ctype;
-	return 1;
-}
-
-static int arguments_choose(struct expression *expr)
-{
-	struct expression_list *arglist = expr->args;
-	struct expression *arg;
-	int i = 0;
-
-	FOR_EACH_PTR (arglist, arg) {
-		if (!evaluate_expression(arg))
-			return 0;
-		i++;
-	} END_FOR_EACH_PTR(arg);
-	if (i < 3) {
-		sparse_error(expr->pos,
-			     "not enough arguments for __builtin_choose_expr");
-		return 0;
-	} if (i > 3) {
-		sparse_error(expr->pos,
-			     "too many arguments for __builtin_choose_expr");
-		return 0;
-	}
-	return 1;
-}
-
-static int evaluate_choose(struct expression *expr)
-{
-	struct expression_list *list = expr->args;
-	struct expression *arg, *args[3];
-	int n = 0;
-
-	/* there will be exactly 3; we'd already verified that */
-	FOR_EACH_PTR(list, arg) {
-		args[n++] = arg;
-	} END_FOR_EACH_PTR(arg);
-
-	*expr = get_expression_value(args[0]) ? *args[1] : *args[2];
-
-	return 1;
-}
-
-static int expand_expect(struct expression *expr, int cost)
-{
-	struct expression *arg = first_ptr_list((struct ptr_list *) expr->args);
-
-	if (arg)
-		*expr = *arg;
-	return 0;
-}
-
-/*
- * __builtin_warning() has type "int" and always returns 1,
- * so that you can use it in conditionals or whatever
- */
-static int expand_warning(struct expression *expr, int cost)
-{
-	struct expression *arg;
-	struct expression_list *arglist = expr->args;
-
-	FOR_EACH_PTR (arglist, arg) {
-		/*
-		 * Constant strings get printed out as a warning. By the
-		 * time we get here, the EXPR_STRING has been fully 
-		 * evaluated, so by now it's an anonymous symbol with a
-		 * string initializer.
-		 *
-		 * Just for the heck of it, allow any constant string
-		 * symbol.
-		 */
-		if (arg->type == EXPR_SYMBOL) {
-			struct symbol *sym = arg->symbol;
-			if (sym->initializer && sym->initializer->type == EXPR_STRING) {
-				struct string *string = sym->initializer->string;
-				warning(expr->pos, "%*s", string->length-1, string->data);
-			}
-			continue;
-		}
-
-		/*
-		 * Any other argument is a conditional. If it's
-		 * non-constant, or it is false, we exit and do
-		 * not print any warning.
-		 */
-		if (arg->type != EXPR_VALUE)
-			goto out;
-		if (!arg->value)
-			goto out;
-	} END_FOR_EACH_PTR(arg);
-out:
-	expr->type = EXPR_VALUE;
-	expr->value = 1;
-	expr->taint = 0;
-	return 0;
-}
-
-static struct symbol_op constant_p_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_constant_p
-};
-
-static struct symbol_op safe_p_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_safe_p
-};
-
-static struct symbol_op warning_op = {
-	.evaluate = evaluate_to_integer,
-	.expand = expand_warning
-};
-
-static struct symbol_op expect_op = {
-	.evaluate = evaluate_expect,
-	.expand = expand_expect
-};
-
-static struct symbol_op choose_op = {
-	.evaluate = evaluate_choose,
-	.args = arguments_choose,
-};
-
-/*
- * Builtin functions
- */
-static struct symbol builtin_fn_type = { .type = SYM_FN /* , .variadic =1 */ };
-static struct sym_init {
-	const char *name;
-	struct symbol *base_type;
-	unsigned int modifiers;
-	struct symbol_op *op;
-} eval_init_table[] = {
-	{ "__builtin_constant_p", &builtin_fn_type, MOD_TOPLEVEL, &constant_p_op },
-	{ "__builtin_safe_p", &builtin_fn_type, MOD_TOPLEVEL, &safe_p_op },
-	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
-	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
-	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
-	{ NULL,		NULL,		0 }
-};
-
 
 /*
  * Abstract types
@@ -829,22 +680,13 @@ struct symbol	zero_int;
 void init_symbols(void)
 {
 	int stream = init_stream("builtin", -1, includepath);
-	struct sym_init *ptr;
 
 #define __IDENT(n,str,res) \
 	hash_ident(&n)
 #include "ident-list.h"
 
 	init_parser(stream);
-
-	builtin_fn_type.variadic = 1;
-	for (ptr = eval_init_table; ptr->name; ptr++) {
-		struct symbol *sym;
-		sym = create_symbol(stream, ptr->name, SYM_NODE, NS_SYMBOL);
-		sym->ctype.base_type = ptr->base_type;
-		sym->ctype.modifiers = ptr->modifiers;
-		sym->op = ptr->op;
-	}
+	init_builtins(stream);
 }
 
 #define MOD_ESIGNED (MOD_SIGNED | MOD_EXPLICITLY_SIGNED)
diff --git a/symbol.h b/symbol.h
index 947a66d2b..36f8345b5 100644
--- a/symbol.h
+++ b/symbol.h
@@ -128,8 +128,6 @@ struct symbol_op {
 	int test, set, class;
 };
 
-extern int expand_safe_p(struct expression *expr, int cost);
-extern int expand_constant_p(struct expression *expr, int cost);
 
 #define SYM_ATTR_WEAK		0
 #define SYM_ATTR_NORMAL		1
@@ -291,6 +289,7 @@ extern const char * type_difference(struct ctype *c1, struct ctype *c2,
 extern struct symbol *lookup_symbol(struct ident *, enum namespace);
 extern struct symbol *create_symbol(int stream, const char *name, int type, int namespace);
 extern void init_symbols(void);
+extern void init_builtins(int stream);
 extern void init_ctype(void);
 extern struct symbol *alloc_symbol(struct position, int type);
 extern void show_type(struct symbol *);
-- 
2.11.0

[PATCH v2 2/3] let identical symbols share their evaluate/expand methods

[Luc Van Oostenryck]

So far, builtin functions which had some evaluate/expand method
couldn't also have a prototype because the declaration of the
prototype and the definition of the builtin with its method would
each have their own symbol and only one of them will be seen, the
last one, the one for the prototype.
This also meant that the evaluate/expand methods had to take care
to set the correct types for they argumenst & results, which is fine
for some generic builtins like __builtin_constant_p() but much less
practical for the ones like __builtin_bswap{16,32,64}().

Fix this by letting symbols with same name share their methods.

Originally-by: Christopher Li <sparse@chrisli.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index b65a6ebce..a4a126720 100644
--- a/parse.c
+++ b/parse.c
@@ -2879,8 +2879,10 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 			}
 		}
 		check_declaration(decl);
-		if (decl->same_symbol)
+		if (decl->same_symbol) {
 			decl->definition = decl->same_symbol->definition;
+			decl->op = decl->same_symbol->op;
+		}
 
 		if (!match_op(token, ','))
 			break;
-- 
2.11.0

[PATCH v2 3/3] expand __builtin_bswap*() with constant args

[Luc Van Oostenryck]

Things are greatly simplified now that such builtins can have a
prototype: the args and result are already evaluated, the
arguments number and their type are already checked, etc.

Based-on-patch-by: Christopher Li <sparse@chrisli.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 builtin.c                                          | 30 +++++++++++++++
 lib.c                                              | 35 ++---------------
 lib.h                                              |  5 +++
 validation/builtin-args-checking.c                 | 45 ++++++++++++++++++++++
 ...in-constant-eval.c => builtin-bswap-constant.c} |  2 +-
 validation/builtin-bswap-variable.c                | 32 +++++++++++++++
 6 files changed, 116 insertions(+), 33 deletions(-)
 create mode 100644 validation/builtin-args-checking.c
 rename validation/{builtin-constant-eval.c => builtin-bswap-constant.c} (93%)
 create mode 100644 validation/builtin-bswap-variable.c

diff --git a/builtin.c b/builtin.c
index c6c97ed85..dbb81032b 100644
--- a/builtin.c
+++ b/builtin.c
@@ -176,6 +176,33 @@ static struct symbol_op choose_op = {
 	.args = arguments_choose,
 };
 
+/* The argument is constant and valid if the cost is zero */
+static int expand_bswap(struct expression *expr, int cost)
+{
+	long long val;
+
+	if (cost)
+		return cost;
+
+	/* the arguments number & type have already been checked */
+	val = const_expression_value(first_expression(expr->args));
+	switch (expr->ctype->bit_size) {
+	case 16: expr->value = __builtin_bswap16(val); break;
+	case 32: expr->value = __builtin_bswap32(val); break;
+	case 64: expr->value = __builtin_bswap64(val); break;
+	default: /* impossible error */
+		return SIDE_EFFECTS;
+	}
+
+	expr->type = EXPR_VALUE;
+	expr->taint = 0;
+	return 0;
+}
+
+static struct symbol_op bswap_op = {
+	.expand = expand_bswap,
+};
+
 
 /*
  * Builtin functions
@@ -192,6 +219,9 @@ static struct sym_init {
 	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
 	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
 	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
+	{ "__builtin_bswap16", NULL, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap32", NULL, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap64", NULL, MOD_TOPLEVEL, &bswap_op },
 	{ NULL,		NULL,		0 }
 };
 
diff --git a/lib.c b/lib.c
index 467e040be..d47325243 100644
--- a/lib.c
+++ b/lib.c
@@ -867,38 +867,9 @@ void declare_builtin_functions(void)
 	add_pre_buffer("extern int __builtin_popcountll(unsigned long long);\n");
 
 	/* And byte swaps.. */
-	add_pre_buffer("extern unsigned short ____builtin_bswap16(unsigned short);\n");
-	add_pre_buffer("extern unsigned int ____builtin_bswap32(unsigned int);\n");
-	add_pre_buffer("extern unsigned long long ____builtin_bswap64(unsigned long long);\n");
-	add_pre_buffer("#define __sparse_constant_swab16(x) ((unsigned short)("
-		       "	(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) |"
-		       "	(((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))\n");
-	add_pre_buffer("#define __sparse_constant_swab32(x) ((unsigned int)("
-		       "	(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))\n");
-	add_pre_buffer("#define __sparse_constant_swab64(x) ((unsigned long long)("
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000000000ffULL) << 56) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000000000ff00ULL) << 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000000000ff0000ULL) << 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000ff000000ULL) <<  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000ff00000000ULL) >>  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000ff0000000000ULL) >> 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00ff000000000000ULL) >> 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0xff00000000000000ULL) >> 56)))\n");
-	add_pre_buffer("#define __builtin_bswap16(x)"
-		       "	(__builtin_constant_p((unsigned short)(x)) ?"
-		       "	__sparse_constant_swab16(x) :"
-		       "	____builtin_bswap16(x))\n");
-	add_pre_buffer("#define __builtin_bswap32(x)"
-		       "	(__builtin_constant_p((unsigned int)(x)) ?"
-		       "	__sparse_constant_swab32(x) :"
-		       "	____builtin_bswap32(x))\n");
-	add_pre_buffer("#define __builtin_bswap64(x)"
-		       "	(__builtin_constant_p((unsigned long long)(x)) ?"
-		       "	__sparse_constant_swab64(x) :"
-		       "	____builtin_bswap64(x))\n");
+	add_pre_buffer("extern unsigned short __builtin_bswap16(unsigned short);\n");
+	add_pre_buffer("extern unsigned int __builtin_bswap32(unsigned int);\n");
+	add_pre_buffer("extern unsigned long long __builtin_bswap64(unsigned long long);\n");
 
 	/* And atomic memory access functions.. */
 	add_pre_buffer("extern int __sync_fetch_and_add(void *, ...);\n");
diff --git a/lib.h b/lib.h
index 57aea9e46..095b1f517 100644
--- a/lib.h
+++ b/lib.h
@@ -201,6 +201,11 @@ static inline struct instruction *first_instruction(struct instruction_list *hea
 	return first_ptr_list((struct ptr_list *)head);
 }
 
+static inline struct expression *first_expression(struct expression_list *head)
+{
+	return first_ptr_list((struct ptr_list *)head);
+}
+
 static inline pseudo_t first_pseudo(struct pseudo_list *head)
 {
 	return first_ptr_list((struct ptr_list *)head);
diff --git a/validation/builtin-args-checking.c b/validation/builtin-args-checking.c
new file mode 100644
index 000000000..55f34d2bc
--- /dev/null
+++ b/validation/builtin-args-checking.c
@@ -0,0 +1,45 @@
+static unsigned int bad_nbr_args_cte(int a)
+{
+	int r = 0;
+	r |= __builtin_bswap16();
+	r |= __builtin_bswap16(1, 2);
+	r |= __builtin_bswap32();
+	r |= __builtin_bswap32(1, 2);
+	r |= __builtin_bswap64();
+	r |= __builtin_bswap64(1, 2);
+	return r;
+}
+
+static unsigned int bad_nbr_args_var(int a, int b)
+{
+	int r = 0;
+	r |= __builtin_bswap16();
+	r |= __builtin_bswap16(a, b);
+	r |= __builtin_bswap32();
+	r |= __builtin_bswap32(a, b);
+	r |= __builtin_bswap64();
+	r |= __builtin_bswap64(a, b);
+	return r;
+}
+
+/*
+ * check-name: builtin-args-checking
+ * check-command: sparse $file
+ * check-description: Check that the arguments checking is done
+ *                    for expanded builtins with a prototype.
+ *
+ * check-error-start
+builtin-args-checking.c:4:31: error: not enough arguments for function __builtin_bswap16
+builtin-args-checking.c:5:31: error: too many arguments for function __builtin_bswap16
+builtin-args-checking.c:6:31: error: not enough arguments for function __builtin_bswap32
+builtin-args-checking.c:7:31: error: too many arguments for function __builtin_bswap32
+builtin-args-checking.c:8:31: error: not enough arguments for function __builtin_bswap64
+builtin-args-checking.c:9:31: error: too many arguments for function __builtin_bswap64
+builtin-args-checking.c:16:31: error: not enough arguments for function __builtin_bswap16
+builtin-args-checking.c:17:31: error: too many arguments for function __builtin_bswap16
+builtin-args-checking.c:18:31: error: not enough arguments for function __builtin_bswap32
+builtin-args-checking.c:19:31: error: too many arguments for function __builtin_bswap32
+builtin-args-checking.c:20:31: error: not enough arguments for function __builtin_bswap64
+builtin-args-checking.c:21:31: error: too many arguments for function __builtin_bswap64
+ * check-error-end
+ */
diff --git a/validation/builtin-constant-eval.c b/validation/builtin-bswap-constant.c
similarity index 93%
rename from validation/builtin-constant-eval.c
rename to validation/builtin-bswap-constant.c
index 63e586e99..4560f67e9 100644
--- a/validation/builtin-constant-eval.c
+++ b/validation/builtin-bswap-constant.c
@@ -17,7 +17,7 @@ unsigned long long bswap64(void)
 }
 
 /*
- * check-name: builtin-eval
+ * check-name: builtin-bswap-constant
  * check-command: test-linearize $file
  *
  * check-output-ignore
diff --git a/validation/builtin-bswap-variable.c b/validation/builtin-bswap-variable.c
new file mode 100644
index 000000000..738ba2a45
--- /dev/null
+++ b/validation/builtin-bswap-variable.c
@@ -0,0 +1,32 @@
+typedef unsigned short	   u16;
+typedef unsigned int	   u32;
+typedef unsigned long long u64;
+
+static u16 swap16v(u16 a)
+{
+	return __builtin_bswap16(a);
+}
+
+static u32 swap32v(u64 a)
+{
+	return __builtin_bswap32(a);
+}
+
+static u64 swap64v(u32 a)
+{
+	return __builtin_bswap64(a);
+}
+
+/*
+ * check-name: builtin-bswap
+ * check-command: test-linearize $file
+ * check-description: Check that the right builtin function is called, and
+ *                    that the args are correctly promoted or truncated.
+ *
+ * check-output-ignore
+ * check-output-contains:call.16 .* __builtin_bswap16
+ * check-output-contains:cast.32 .* (64) %arg1
+ * check-output-contains:call.32 .* __builtin_bswap32
+ * check-output-contains:cast.64 .* (32) %arg1
+ * check-output-contains:call.64 .* __builtin_bswap64
+ */
-- 
2.11.0

Re: [PATCH v2 0/3] builtins expansion

[Christopher Li]

On Sun, Feb 12, 2017 at 11:10 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> This serie solves the problem of the expansion of some builtins
> like __builtin_bswap16() which gcc consider as an integer constant
> expression when the arg is itself an integer constant.
> Such builtins are used as such in the kernel and their non-expansion
> create undesirable warnings from sparse.

Applied in sparse-next.

Chris

Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

[Chris Li]

On Sun, Feb 12, 2017 at 11:03 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> OK, I've look at this.
> The difference between your patch and mine is that yours share/
> migrate the methods for all symbols while mine purposely did that
> for builtins only. The reasons I did this way was to avoid to share

That to me is actually not the main difference. Yes, the one line patch
is simple and apply op to all symbol has the same name.
If we need, we can make that migrate code into a function and do
some special handing for builtin etc. I just don't see the need for
that right now.

The big difference I see is the solve the problem at symbol creating
side rather than symbol usage side.

In the future, we can make migrate symbol only return symbol are
new (and has incremental difference). Totally drop the symbol which
is the exact same abstract declare as the previous one.

> methods between things that don't need to, for example if someone
> define a function that have the same name as one of the numerous
> attributes (which are not reserved keywords and aren't necessarily
> protected by leading double underscore). But having look at this now,
> I see that it should never be a problem when this happen (but is it
> a good idea?).

Even if that became a problem, we can add some code to deal with
it. E.g. conditionally migrate some symbol information.

Chris

[PATCH 3/3] expand __builtin_bswap*() with constant args

[Luc Van Oostenryck]

Things are greatly simplified now that such builtins can have a
prototype: the args and result are already evaluated, the argument
number and type are already checked, ...

Based-on-patch-by: Christopher Li <sparse@chrisli.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 builtin.c                           | 30 +++++++++++++++++++++
 lib.c                               | 35 +++----------------------
 lib.h                               |  5 ++++
 validation/builtin-bswap-constant.c | 48 ++++++++++++++++++++++++++++++++++
 validation/builtin-bswap.c          | 52 +++++++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+), 32 deletions(-)
 create mode 100644 validation/builtin-bswap-constant.c
 create mode 100644 validation/builtin-bswap.c

diff --git a/builtin.c b/builtin.c
index ddc71f785..4dddb1e1d 100644
--- a/builtin.c
+++ b/builtin.c
@@ -176,6 +176,33 @@ static struct symbol_op choose_op = {
 	.args = arguments_choose,
 };
 
+/* The argument is constant and valid if the cost is zero */
+static int expand_bswap(struct expression *expr, int cost)
+{
+	long long input;
+
+	if (cost)
+		return cost;
+
+	/* the argument's number have already been checked */
+	input = const_expression_value(first_expression(expr->args));
+	switch (expr->ctype->bit_size) {
+	case 16: expr->value = __builtin_bswap16(input); break;
+	case 32: expr->value = __builtin_bswap32(input); break;
+	case 64: expr->value = __builtin_bswap64(input); break;
+	default: /* impossible error */
+		return SIDE_EFFECTS;
+	}
+
+	expr->type = EXPR_VALUE;
+	expr->taint = 0;
+	return 0;
+}
+
+static struct symbol_op bswap_op = {
+	.expand = expand_bswap,
+};
+
 
 /*
  * Builtin functions
@@ -192,6 +219,9 @@ static struct sym_init {
 	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
 	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
 	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
+	{ "__builtin_bswap16", NULL, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap32", NULL, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap64", NULL, MOD_TOPLEVEL, &bswap_op },
 	{ NULL,		NULL,		0 }
 };
 
diff --git a/lib.c b/lib.c
index 2660575b1..460e07609 100644
--- a/lib.c
+++ b/lib.c
@@ -820,38 +820,9 @@ void declare_builtin_functions(void)
 	add_pre_buffer("extern int __builtin_popcountll(unsigned long long);\n");
 
 	/* And byte swaps.. */
-	add_pre_buffer("extern unsigned short ____builtin_bswap16(unsigned short);\n");
-	add_pre_buffer("extern unsigned int ____builtin_bswap32(unsigned int);\n");
-	add_pre_buffer("extern unsigned long long ____builtin_bswap64(unsigned long long);\n");
-	add_pre_buffer("#define __sparse_constant_swab16(x) ((unsigned short)("
-		       "	(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) |"
-		       "	(((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))\n");
-	add_pre_buffer("#define __sparse_constant_swab32(x) ((unsigned int)("
-		       "	(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))\n");
-	add_pre_buffer("#define __sparse_constant_swab64(x) ((unsigned long long)("
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000000000ffULL) << 56) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000000000ff00ULL) << 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000000000ff0000ULL) << 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000ff000000ULL) <<  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000ff00000000ULL) >>  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000ff0000000000ULL) >> 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00ff000000000000ULL) >> 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0xff00000000000000ULL) >> 56)))\n");
-	add_pre_buffer("#define __builtin_bswap16(x)"
-		       "	(__builtin_constant_p((unsigned short)(x)) ?"
-		       "	__sparse_constant_swab16(x) :"
-		       "	____builtin_bswap16(x))\n");
-	add_pre_buffer("#define __builtin_bswap32(x)"
-		       "	(__builtin_constant_p((unsigned int)(x)) ?"
-		       "	__sparse_constant_swab32(x) :"
-		       "	____builtin_bswap32(x))\n");
-	add_pre_buffer("#define __builtin_bswap64(x)"
-		       "	(__builtin_constant_p((unsigned long long)(x)) ?"
-		       "	__sparse_constant_swab64(x) :"
-		       "	____builtin_bswap64(x))\n");
+	add_pre_buffer("extern unsigned short __builtin_bswap16(unsigned short);\n");
+	add_pre_buffer("extern unsigned int __builtin_bswap32(unsigned int);\n");
+	add_pre_buffer("extern unsigned long long __builtin_bswap64(unsigned long long);\n");
 
 	/* And atomic memory access functions.. */
 	add_pre_buffer("extern int __sync_fetch_and_add(void *, ...);\n");
diff --git a/lib.h b/lib.h
index b778bdcd0..306ee4545 100644
--- a/lib.h
+++ b/lib.h
@@ -200,6 +200,11 @@ static inline struct instruction *first_instruction(struct instruction_list *hea
 	return first_ptr_list((struct ptr_list *)head);
 }
 
+static inline struct expression *first_expression(struct expression_list *head)
+{
+	return first_ptr_list((struct ptr_list *)head);
+}
+
 static inline pseudo_t first_pseudo(struct pseudo_list *head)
 {
 	return first_ptr_list((struct ptr_list *)head);
diff --git a/validation/builtin-bswap-constant.c b/validation/builtin-bswap-constant.c
new file mode 100644
index 000000000..788806f0b
--- /dev/null
+++ b/validation/builtin-bswap-constant.c
@@ -0,0 +1,48 @@
+unsigned short bswap16(void);
+unsigned short bswap16(void)
+{
+	return __builtin_bswap16(0x1234);
+}
+
+unsigned int bswap32(void);
+unsigned int bswap32(void)
+{
+	return __builtin_bswap32(0x12345678);
+}
+
+unsigned long long bswap64(void);
+unsigned long long bswap64(void)
+{
+	return __builtin_bswap64(0x123456789abcdef0ULL);
+}
+
+static unsigned int bad_nbr_args(int a)
+{
+	a |=  __builtin_bswap16();
+	a |=  __builtin_bswap16(1, 2);
+	a |=  __builtin_bswap32();
+	a |=  __builtin_bswap32(1, 2);
+	a |=  __builtin_bswap64();
+	a |=  __builtin_bswap64(1, 2);
+	return a;
+}
+
+/*
+ * check-name: builtin-bswap-constant
+ * check-command: test-linearize $file
+ *
+ * check-output-ignore
+ * check-output-excludes: __builtin_bswap
+ * check-output-contains:ret.16 *.0x3412
+ * check-output-contains:ret.32 *.0x78563412
+ * check-output-contains:ret.64 *.0xf0debc9a78563412
+ *
+ * check-error-start
+builtin-bswap-constant.c:21:32: error: not enough arguments for function __builtin_bswap16
+builtin-bswap-constant.c:22:32: error: too many arguments for function __builtin_bswap16
+builtin-bswap-constant.c:23:32: error: not enough arguments for function __builtin_bswap32
+builtin-bswap-constant.c:24:32: error: too many arguments for function __builtin_bswap32
+builtin-bswap-constant.c:25:32: error: not enough arguments for function __builtin_bswap64
+builtin-bswap-constant.c:26:32: error: too many arguments for function __builtin_bswap64
+ * check-error-end
+ */
diff --git a/validation/builtin-bswap.c b/validation/builtin-bswap.c
new file mode 100644
index 000000000..f98b7fc07
--- /dev/null
+++ b/validation/builtin-bswap.c
@@ -0,0 +1,52 @@
+typedef unsigned short	   u16;
+typedef unsigned int	   u32;
+typedef unsigned long long u64;
+
+static u16 swap16v(u16 a)
+{
+	return __builtin_bswap16(a);
+}
+
+static u32 swap32v(u64 a)
+{
+	return __builtin_bswap32(a);
+}
+
+static u64 swap64v(u32 a)
+{
+	return __builtin_bswap64(a);
+}
+
+static unsigned int bad_nbr_args(int a, int b)
+{
+	a |=  __builtin_bswap16();
+	a |=  __builtin_bswap16(a, b);
+	a |=  __builtin_bswap32();
+	a |=  __builtin_bswap32(a, b);
+	a |=  __builtin_bswap64();
+	a |=  __builtin_bswap64(a, b);
+	return a;
+}
+
+/*
+ * check-name: builtin-bswap
+ * check-command: test-linearize $file
+ * check-description: Check that the right builtin function is called, and
+ *                    that the args are correctly promoted or truncated.
+ *
+ * check-error-start
+builtin-bswap.c:22:32: error: not enough arguments for function __builtin_bswap16
+builtin-bswap.c:23:32: error: too many arguments for function __builtin_bswap16
+builtin-bswap.c:24:32: error: not enough arguments for function __builtin_bswap32
+builtin-bswap.c:25:32: error: too many arguments for function __builtin_bswap32
+builtin-bswap.c:26:32: error: not enough arguments for function __builtin_bswap64
+builtin-bswap.c:27:32: error: too many arguments for function __builtin_bswap64
+ * check-error-end
+ *
+ * check-output-ignore
+ * check-output-contains:call.16 .* __builtin_bswap16
+ * check-output-contains:cast.32 .* (64) %arg1
+ * check-output-contains:call.32 .* __builtin_bswap32
+ * check-output-contains:cast.64 .* (32) %arg1
+ * check-output-contains:call.64 .* __builtin_bswap64
+ */
-- 
2.11.0