[PATCH 0/3] fix function calls via pointers

[PATCH 0/3] fix function calls via pointers

[Luc Van Oostenryck]

This series contains a fix for the linearization of
function calls via function pointers.

Luc Van Oostenryck (3):
  add testcases for the linearization of calls
  simplify linearize_call_expression()
  fix linearize (*fun)()

 linearize.c                              | 21 ++++--------
 validation/builtin-arith.c               | 52 +++++++++++++++++++++++++++++
 validation/function-pointer-type.c       |  9 +++++
 validation/linear/call-basic.c           | 57 ++++++++++++++++++++++++++++++++
 validation/linear/call-builtin.c         | 14 ++++++++
 validation/linear/call-casted-pointer.c  | 31 +++++++++++++++++
 validation/linear/call-complex-pointer.c | 32 ++++++++++++++++++
 validation/linear/call-direct.c          | 14 ++++++++
 validation/linear/call-indirect.c        | 13 ++++++++
 validation/linear/call-inline.c          | 15 +++++++++
 validation/optim/call-complex-pointer.c  | 13 ++++++++
 validation/sizeof-builtin.c              | 15 +++++++++
 validation/sizeof-function.c             | 50 ++++++++++++++++++++++++++++
 13 files changed, 321 insertions(+), 15 deletions(-)
 create mode 100644 validation/builtin-arith.c
 create mode 100644 validation/function-pointer-type.c
 create mode 100644 validation/linear/call-basic.c
 create mode 100644 validation/linear/call-builtin.c
 create mode 100644 validation/linear/call-casted-pointer.c
 create mode 100644 validation/linear/call-complex-pointer.c
 create mode 100644 validation/linear/call-direct.c
 create mode 100644 validation/linear/call-indirect.c
 create mode 100644 validation/linear/call-inline.c
 create mode 100644 validation/optim/call-complex-pointer.c
 create mode 100644 validation/sizeof-builtin.c
 create mode 100644 validation/sizeof-function.c

-- 
2.15.0

[PATCH 1/3] add testcases for the linearization of calls

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/builtin-arith.c               | 52 ++++++++++++++++++++++++++++
 validation/function-pointer-type.c       |  9 +++++
 validation/linear/call-basic.c           | 58 ++++++++++++++++++++++++++++++++
 validation/linear/call-builtin.c         | 14 ++++++++
 validation/linear/call-casted-pointer.c  | 32 ++++++++++++++++++
 validation/linear/call-complex-pointer.c | 32 ++++++++++++++++++
 validation/linear/call-direct.c          | 14 ++++++++
 validation/linear/call-indirect.c        | 14 ++++++++
 validation/linear/call-inline.c          | 15 +++++++++
 validation/optim/call-complex-pointer.c  | 13 +++++++
 validation/sizeof-builtin.c              | 15 +++++++++
 validation/sizeof-function.c             | 50 +++++++++++++++++++++++++++
 12 files changed, 318 insertions(+)
 create mode 100644 validation/builtin-arith.c
 create mode 100644 validation/function-pointer-type.c
 create mode 100644 validation/linear/call-basic.c
 create mode 100644 validation/linear/call-builtin.c
 create mode 100644 validation/linear/call-casted-pointer.c
 create mode 100644 validation/linear/call-complex-pointer.c
 create mode 100644 validation/linear/call-direct.c
 create mode 100644 validation/linear/call-indirect.c
 create mode 100644 validation/linear/call-inline.c
 create mode 100644 validation/optim/call-complex-pointer.c
 create mode 100644 validation/sizeof-builtin.c
 create mode 100644 validation/sizeof-function.c

diff --git a/validation/builtin-arith.c b/validation/builtin-arith.c
new file mode 100644
index 000000000..d08c93dab
--- /dev/null
+++ b/validation/builtin-arith.c
@@ -0,0 +1,52 @@
+
+
+void test(void (*fun)(void));
+void test(void (*fun)(void))
+{
+	typedef typeof(__builtin_trap) t;	// OK
+	void (*f)(void);
+	int i;
+
+	f =  __builtin_trap;
+	f = &__builtin_trap;
+	f = *__builtin_trap;			// OK for GCC
+	f =  __builtin_trap + 0;
+	f =  __builtin_trap + 1;
+	f =  __builtin_trap - 1;
+
+	// (void) __builtin_trap;
+	f = (void*) __builtin_trap;
+	f = (unsigned long) __builtin_trap;
+
+	i = !__builtin_trap;
+	i = (__builtin_trap > fun);
+	i = (__builtin_trap == fun);
+	i = (fun <  __builtin_trap);
+	i = (fun == __builtin_trap);
+
+	__builtin_trap - fun;
+	fun - __builtin_trap;
+}
+
+/*
+ * check-name: builtin arithmetic
+ * check-command: sparse -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-start
+builtin-arith.c:10:xx: error: ...
+builtin-arith.c:11:xx: error: ...
+builtin-arith.c:13:xx: error: arithmetics on pointers to functions
+builtin-arith.c:14:xx: error: arithmetics on pointers to functions
+builtin-arith.c:15:xx: error: arithmetics on pointers to functions
+builtin-arith.c:18:xx: error: ...
+builtin-arith.c:19:xx: error: ...
+builtin-arith.c:21:xx: error: ...
+builtin-arith.c:22:xx: error: ...
+builtin-arith.c:23:xx: error: ...
+builtin-arith.c:24:xx: error: ...
+builtin-arith.c:25:xx: error: ...
+builtin-arith.c:27:24: error: subtraction of functions? Share your drugs
+builtin-arith.c:28:13: error: subtraction of functions? Share your drugs
+ * check-error-end
+ */
diff --git a/validation/function-pointer-type.c b/validation/function-pointer-type.c
new file mode 100644
index 000000000..8fd792b3b
--- /dev/null
+++ b/validation/function-pointer-type.c
@@ -0,0 +1,9 @@
+extern int fun(void);
+
+void fa(void) { int (*f)(void); f = &fun; }
+void f0(void) { int (*f)(void); f = fun; }	// C99,C11 6.3.2.1p4
+
+/*
+ * check-name: type of function pointers
+ * check-command: sparse -Wno-decl $file
+ */
diff --git a/validation/linear/call-basic.c b/validation/linear/call-basic.c
new file mode 100644
index 000000000..235b443d2
--- /dev/null
+++ b/validation/linear/call-basic.c
@@ -0,0 +1,58 @@
+extern int fun(int a);
+
+int symbol(int a)
+{
+	fun(a);
+}
+
+int pointer0(int a, int (*fun)(int))
+{
+	fun(a);
+}
+
+int pointer1(int a, int (*fun)(int))
+{
+	(*fun)(a);
+}
+
+int builtin(int a)
+{
+	__builtin_popcount(a);
+}
+
+/*
+ * check-name: basic function calls
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-start
+symbol:
+.L0:
+	<entry-point>
+	call.32     %r2 <- fun, %arg1
+	ret.32      %r2
+
+
+pointer0:
+.L2:
+	<entry-point>
+	call.32     %r5 <- %arg2, %arg1
+	ret.32      %r5
+
+
+pointer1:
+.L4:
+	<entry-point>
+	call.32     %r8 <- %arg2, %arg1
+	ret.32      %r8
+
+
+builtin:
+.L6:
+	<entry-point>
+	call.32     %r11 <- __builtin_popcount, %arg1
+	ret.32      %r11
+
+
+ * check-output-end
+ */
diff --git a/validation/linear/call-builtin.c b/validation/linear/call-builtin.c
new file mode 100644
index 000000000..af0148aa4
--- /dev/null
+++ b/validation/linear/call-builtin.c
@@ -0,0 +1,14 @@
+typedef unsigned int u32;
+
+u32 ff(u32 a) { return __builtin_popcount(a); }
+
+u32 f0(u32 a) { return (__builtin_popcount)(a); }
+
+/*
+ * check-name: builtin calls
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-pattern(2): call\..*__builtin_.*, %arg1
+ */
diff --git a/validation/linear/call-casted-pointer.c b/validation/linear/call-casted-pointer.c
new file mode 100644
index 000000000..e66f69990
--- /dev/null
+++ b/validation/linear/call-casted-pointer.c
@@ -0,0 +1,32 @@
+typedef int (*fun_t)(void*);
+
+int foo(void *a, void *fun)
+{
+	return ((fun_t)fun)(a);
+}
+
+int bar(void *a, void *fun)
+{
+	return ((int (*)(void *))fun)(a);
+}
+
+int qux(void *a, void *fun)
+{
+	return (*(fun_t)fun)(a);
+}
+
+int quz(void *a, void *fun)
+{
+	return (*(int (*)(void *))fun)(a);
+}
+
+/*
+ * check-name: call via casted function pointer
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-pattern(4): ptrcast\..* %arg2
+ * check-output-pattern(4): call\..* %arg1
+ */
diff --git a/validation/linear/call-complex-pointer.c b/validation/linear/call-complex-pointer.c
new file mode 100644
index 000000000..d95e18057
--- /dev/null
+++ b/validation/linear/call-complex-pointer.c
@@ -0,0 +1,32 @@
+int foo(int p, int (*f0)(int), int (*f1)(int), int arg)
+{
+	return (p ? f0 : f1)(arg);
+}
+
+/*
+ * check-name: call-complex-pointer
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-start
+foo:
+.L0:
+	<entry-point>
+	cbr         %arg1, .L2, .L3
+
+.L2:
+	phisrc.64   %phi1 <- %arg2
+	br          .L4
+
+.L3:
+	ptrcast.64  %r5 <- (64) %arg3
+	phisrc.64   %phi2 <- %r5
+	br          .L4
+
+.L4:
+	phi.64      %r6 <- %phi1, %phi2
+	call.32     %r7 <- %r6, %arg4
+	ret.32      %r7
+
+
+ * check-output-end
+ */
diff --git a/validation/linear/call-direct.c b/validation/linear/call-direct.c
new file mode 100644
index 000000000..78321ab02
--- /dev/null
+++ b/validation/linear/call-direct.c
@@ -0,0 +1,14 @@
+extern int fun(void);
+
+int ff(void) { return fun(); }
+
+int f0(void) { return (fun)(); }
+
+/*
+ * check-name: direct calls
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-pattern(2): call\..* fun
+ */
diff --git a/validation/linear/call-indirect.c b/validation/linear/call-indirect.c
new file mode 100644
index 000000000..8ab78676b
--- /dev/null
+++ b/validation/linear/call-indirect.c
@@ -0,0 +1,14 @@
+int gg(int (*fun)(void)) { return fun(); }
+
+int g0(int (*fun)(void)) { return (fun)(); }
+int g1(int (*fun)(void)) { return (*fun)(); }	// C99,C11 6.5.3.2p4
+
+/*
+ * check-name: indirect calls
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-pattern(3): call\..* %arg1
+ */
diff --git a/validation/linear/call-inline.c b/validation/linear/call-inline.c
new file mode 100644
index 000000000..32f32461c
--- /dev/null
+++ b/validation/linear/call-inline.c
@@ -0,0 +1,15 @@
+static inline int fun(void) { return 42; }
+
+int fi(void) { return fun(); }
+
+int i0(void) { return (fun)(); }
+
+/*
+ * check-name: inline calls
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-excludes: call
+ * check-output-pattern(2): ret\..* \\$42
+ */
diff --git a/validation/optim/call-complex-pointer.c b/validation/optim/call-complex-pointer.c
new file mode 100644
index 000000000..6cfeb6abd
--- /dev/null
+++ b/validation/optim/call-complex-pointer.c
@@ -0,0 +1,13 @@
+int foo(int p, int (*f0)(int), int (*f1)(int), int arg)
+{
+	return (p ? f0 : f1)(arg);
+}
+/*
+ * check-name: call-complex-pointer
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: ptrcast\.
+ * check-output-contains: select\.
+ */
diff --git a/validation/sizeof-builtin.c b/validation/sizeof-builtin.c
new file mode 100644
index 000000000..7123e4deb
--- /dev/null
+++ b/validation/sizeof-builtin.c
@@ -0,0 +1,15 @@
+int test(void);
+int test(void)
+{
+	return sizeof &__builtin_trap;
+}
+
+/*
+ * check-name: sizeof-builtin
+ * check-command: sparse -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-start
+sizeof-function.c:4:16: error: expression using addressof on a builtin function
+ * check-error-end
+ */
diff --git a/validation/sizeof-function.c b/validation/sizeof-function.c
new file mode 100644
index 000000000..20c795e94
--- /dev/null
+++ b/validation/sizeof-function.c
@@ -0,0 +1,50 @@
+extern int fun(void);
+extern int (*ptr)(void);
+
+static inline int inl(int *a)
+{
+	return *a + 1;
+}
+
+
+int test(void);
+int test(void)
+{
+	unsigned int s = 0;
+
+	// OK
+	s += sizeof &fun;
+	s += sizeof  ptr;
+	s += sizeof &ptr;
+	s += sizeof &inl;
+
+	// KO
+	s += sizeof  fun;
+	s += sizeof *fun;
+
+	s += sizeof *ptr;
+
+	s += sizeof  inl;
+	s += sizeof *inl;
+
+	s += sizeof  __builtin_trap;
+	s += sizeof *__builtin_trap;
+
+	return s;
+}
+
+/*
+ * check-name: sizeof-function
+ * check-command: sparse -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-error-start
+sizeof-function.c:22:14: warning: expression using sizeof on a function
+sizeof-function.c:23:14: warning: expression using sizeof on a function
+sizeof-function.c:25:14: warning: expression using sizeof on a function
+sizeof-function.c:27:14: warning: expression using sizeof on a function
+sizeof-function.c:28:14: warning: expression using sizeof on a function
+sizeof-function.c:30:14: warning: expression using sizeof on a function
+sizeof-function.c:31:14: warning: expression using sizeof on a function
+ * check-error-end
+ */
-- 
2.15.0

[PATCH 2/3] simplify linearize_call_expression()

[Luc Van Oostenryck]

Use the fact that fn->ctype can't be NULL if expr->ctype
is not NULL.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/linearize.c b/linearize.c
index eff7d95f1..7ad348c00 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1279,15 +1279,10 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
 	}
 
 	fn = expr->fn;
-
-	if (fn->ctype)
-		ctype = &fn->ctype->ctype;

[PATCH 3/3] fix linearize (*fun)()

[Luc Van Oostenryck]

A function call via a function pointer can be written like:
	fp(), (fp)() or (*fp)()
In the latter case the dereference is unneeded but legal and
idiomatic.

However, the linearization doesn't handle this unneeded deref
and leads to the generation of a load of the pointer:

	int foo(int a, int (*fun)(int))
	{
		(*fun)(a);
	}

gives something like:
	foo:
		load        %r2 <- 0[%arg2]
		call.32     %r3 <- %r2, %arg1
		ret.32      %r3

This happens because, at linearization, the deref is dropped
but only if the sub-expression is a symbol and the test for
node is not done.

Fix this by using is_func_type() to test the type of all call
expressions.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c                             | 10 +++-------
 validation/linear/call-basic.c          |  1 -
 validation/linear/call-casted-pointer.c |  1 -
 validation/linear/call-indirect.c       |  1 -
 4 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/linearize.c b/linearize.c
index 7ad348c00..2e146de7a 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1291,13 +1291,9 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
 		add_symbol(&insn->fntypes, arg->ctype);
 	} END_FOR_EACH_PTR(arg);
 
-	if (fn->type == EXPR_PREOP) {
-		if (fn->unop->type == EXPR_SYMBOL) {
-			struct symbol *sym = fn->unop->symbol;
-			if (sym->ctype.base_type->type == SYM_FN)
-				fn = fn->unop;
-		}
-	}
+	if (fn->type == EXPR_PREOP && fn->op == '*' && is_func_type(fn->ctype))
+		fn = fn->unop;
+
 	if (fn->type == EXPR_SYMBOL) {
 		call = symbol_pseudo(ep, fn->symbol);
 	} else {
diff --git a/validation/linear/call-basic.c b/validation/linear/call-basic.c
index 235b443d2..60517e2e3 100644
--- a/validation/linear/call-basic.c
+++ b/validation/linear/call-basic.c
@@ -23,7 +23,6 @@ int builtin(int a)
 /*
  * check-name: basic function calls
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-output-start
 symbol:
diff --git a/validation/linear/call-casted-pointer.c b/validation/linear/call-casted-pointer.c
index e66f69990..610d67482 100644
--- a/validation/linear/call-casted-pointer.c
+++ b/validation/linear/call-casted-pointer.c
@@ -23,7 +23,6 @@ int quz(void *a, void *fun)
 /*
  * check-name: call via casted function pointer
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-output-ignore
  * check-output-excludes: load
diff --git a/validation/linear/call-indirect.c b/validation/linear/call-indirect.c
index 8ab78676b..f5f2adaf1 100644
--- a/validation/linear/call-indirect.c
+++ b/validation/linear/call-indirect.c
@@ -6,7 +6,6 @@ int g1(int (*fun)(void)) { return (*fun)(); }	// C99,C11 6.5.3.2p4
 /*
  * check-name: indirect calls
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-output-ignore
  * check-output-excludes: load
-- 
2.15.0