[PATCH 0/3] add missing degenerate() for logical-not

[PATCH 0/3] add missing degenerate() for logical-not

[Luc Van Oostenryck]

The goal of this series is to fix the evaluation of expressions
doing the (logical) negation of arrays or functions.
In this case, of course, the array or function must degenerate
into a pointer but it wasn't done. It doesn't make any
visible difference for the checker but the linearization
of such expressions was wrong.

Luc Van Oostenryck (3):
  add more testcases for using addresses in conditionals
  add testcases linearization of degenerated arrays/functions
  fix: add missing degenerate() for logical not

 evaluate.c                                         |   1 +
 .../{cond-address-array.c => Waddress-array.c}     |   9 +-
 ...cond-address-function.c => Waddress-function.c} |   7 +-
 validation/Waddress-weak.c                         |  25 +++++
 validation/Waddress.c                              | 110 +++++++++++++++++++++
 validation/linear/degen-array.c                    |  31 ++++++
 validation/linear/degen-function.c                 |  38 +++++++
 validation/linear/degen-log-not.c                  |  40 ++++++++
 8 files changed, 252 insertions(+), 9 deletions(-)
 rename validation/{cond-address-array.c => Waddress-array.c} (37%)
 rename validation/{cond-address-function.c => Waddress-function.c} (39%)
 create mode 100644 validation/Waddress-weak.c
 create mode 100644 validation/Waddress.c
 create mode 100644 validation/linear/degen-array.c
 create mode 100644 validation/linear/degen-function.c
 create mode 100644 validation/linear/degen-log-not.c

-- 
2.15.0

[PATCH 1/3] add more testcases for using addresses in conditionals

[Luc Van Oostenryck]

and unify the existing ones.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 .../{cond-address-array.c => Waddress-array.c}     |   9 +-
 ...cond-address-function.c => Waddress-function.c} |   7 +-
 validation/Waddress-weak.c                         |  25 +++++
 validation/Waddress.c                              | 110 +++++++++++++++++++++
 4 files changed, 142 insertions(+), 9 deletions(-)
 rename validation/{cond-address-array.c => Waddress-array.c} (37%)
 rename validation/{cond-address-function.c => Waddress-function.c} (39%)
 create mode 100644 validation/Waddress-weak.c
 create mode 100644 validation/Waddress.c

diff --git a/validation/cond-address-array.c b/validation/Waddress-array.c
similarity index 37%
rename from validation/cond-address-array.c
rename to validation/Waddress-array.c
index e1d2f87f8..831784548 100644
--- a/validation/cond-address-array.c
+++ b/validation/Waddress-array.c
@@ -15,12 +15,11 @@ int bar(void) {
 }
 
 /*
- * check-name: cond-address-array.c
- * check-command: test-linearize -Wno-decl -Waddress $file
- * check-output-ignore
+ * check-name: Waddress-array
+ * check-command: sparse -Wno-decl -Waddress $file
  *
  * check-error-start
-cond-address-array.c:4:13: warning: the address of an array will always evaluate as true
-cond-address-array.c:12:13: warning: the address of an array will always evaluate as true
+Waddress-array.c:4:13: warning: the address of an array will always evaluate as true
+Waddress-array.c:12:13: warning: the address of an array will always evaluate as true
  * check-error-end
  */
diff --git a/validation/cond-address-function.c b/validation/Waddress-function.c
similarity index 39%
rename from validation/cond-address-function.c
rename to validation/Waddress-function.c
index 9a143a009..fccbe2d89 100644
--- a/validation/cond-address-function.c
+++ b/validation/Waddress-function.c
@@ -8,11 +8,10 @@ int global_function(void)
 }
 
 /*
- * check-name: cond-address-function
- * check-command: test-linearize -Wno-decl -Waddress $file
- * check-output-ignore
+ * check-name: Waddress-function
+ * check-command: sparse -Wno-decl -Waddress $file
  *
  * check-error-start
-cond-address-function.c:5:13: warning: the address of a function will always evaluate as true
+Waddress-function.c:5:13: warning: the address of a function will always evaluate as true
  * check-error-end
  */
diff --git a/validation/Waddress-weak.c b/validation/Waddress-weak.c
new file mode 100644
index 000000000..1fe8d33c8
--- /dev/null
+++ b/validation/Waddress-weak.c
@@ -0,0 +1,25 @@
+extern int var       __attribute__((weak));
+extern int arr[]     __attribute__((weak));
+extern int fun(void) __attribute__((weak));
+
+int test_addr_weak_fun(void)
+{
+	if ( &var) return 1;
+	if (  arr) return 1;
+	if ( &arr) return 1;
+	if (  fun) return 1;
+	if ( &fun) return 1;
+	if (!&var) return 0;
+	if (! arr) return 0;
+	if (!&arr) return 0;
+	if (! fun) return 0;
+	if (!&fun) return 0;
+	return -1;
+}
+
+/*
+ * check-name: Waddress-weak
+ * check-note: Undefined weak symbols (can) have a null address.
+ * check-command: sparse -Wno-decl -Waddress $file
+ * check-known-to-fail
+ */
diff --git a/validation/Waddress.c b/validation/Waddress.c
new file mode 100644
index 000000000..10556c3aa
--- /dev/null
+++ b/validation/Waddress.c
@@ -0,0 +1,110 @@
+extern int fun(void);
+extern int arr[];
+extern int var;
+
+int test_address(int arg, int ptr[])
+{
+
+	if (fun())	return -1;
+	if (var)	return -1;
+	if (arg)	return -1;
+	if (ptr)	return -1;
+
+lab:
+	if (arr)	return 1;
+	if (&arr)	return 1;
+	if (fun)	return 1;
+	if (&fun)	return 1;
+	if (&var)	return 1;
+	if (&arg)	return 1;
+	if (&&lab)	return 1;
+
+	return -1;
+}
+
+int test_address_not(int arg, int ptr[])
+{
+
+	if (!fun())	return -1;
+	if (!var)	return -1;
+	if (!arg)	return -1;
+	if (!ptr)	return -1;
+
+lab:
+	if (!arr)	return 0;
+	if (!&arr)	return 0;
+	if (!fun)	return 0;
+	if (!&fun)	return 0;
+	if (!&var)	return 0;
+	if (!&arg)	return 0;
+	if (!&&lab)	return 0;
+
+	return -1;
+}
+
+int test_address_cmp(int arg, int ptr[])
+{
+	if (fun() == 0)	return -1;
+	if (0 == fun())	return -1;
+	if (var == 0)	return -1;
+	if (0 == var)	return -1;
+	if (arg == 0)	return -1;
+	if (0 == arg)	return -1;
+	if (ptr == 0)	return -1;
+	if (0 == ptr)	return -1;
+
+lab:
+	if (arr == 0)	return 0;
+	if (0 == arr)	return 0;
+	if (&arr == 0)	return 0;
+	if (0 == &arr)	return 0;
+	if (fun == 0)	return 0;
+	if (0 == fun)	return 0;
+	if (&fun == 0)	return 0;
+	if (0 == &fun)	return 0;
+	if (&var == 0)	return 0;
+	if (0 == &var)	return 0;
+	if (&arg == 0)	return 0;
+	if (0 == &arg)	return 0;
+	if (&&lab == 0)	return 0;
+	if (0 == &&lab)	return 0;
+
+	return -1;
+}
+
+/*
+ * check-name: Waddress
+ * check-command: sparse -Wno-decl -Wno-non-pointer-null -Waddress $file
+ * check-known-to-fail
+ *
+ * check-error-start
+Waddress.c:14:13: warning: the address of an array will always evaluate as true
+Waddress.c:15:14: warning: the address of an array will always evaluate as true
+Waddress.c:16:13: warning: the address of a function will always evaluate as true
+Waddress.c:17:14: warning: the address of a function will always evaluate as true
+Waddress.c:18:13: warning: the address of a variable will always evaluate as true
+Waddress.c:19:13: warning: the address of a variable will always evaluate as true
+Waddress.c:20:13: warning: the address of a label will always evaluate as true
+Waddress.c:34:13: warning: the address of an array will always evaluate as true
+Waddress.c:35:13: warning: the address of an array will always evaluate as true
+Waddress.c:36:13: warning: the address of a function will always evaluate as true
+Waddress.c:37:13: warning: the address of a function will always evaluate as true
+Waddress.c:38:13: warning: the address of a variable will always evaluate as true
+Waddress.c:39:13: warning: the address of a variable will always evaluate as true
+Waddress.c:40:13: warning: the address of a label will always evaluate as true
+Waddress.c:57:13: warning: the address of an array will always evaluate as true
+Waddress.c:58:13: warning: the address of an array will always evaluate as true
+Waddress.c:59:13: warning: the address of an array will always evaluate as true
+Waddress.c:60:13: warning: the address of an array will always evaluate as true
+Waddress.c:61:13: warning: the address of a function will always evaluate as true
+Waddress.c:62:13: warning: the address of a function will always evaluate as true
+Waddress.c:63:13: warning: the address of a function will always evaluate as true
+Waddress.c:64:13: warning: the address of a function will always evaluate as true
+Waddress.c:65:13: warning: the address of a variable will always evaluate as true
+Waddress.c:66:13: warning: the address of a variable will always evaluate as true
+Waddress.c:67:13: warning: the address of a variable will always evaluate as true
+Waddress.c:68:13: warning: the address of a variable will always evaluate as true
+Waddress.c:69:13: warning: the address of a label will always evaluate as true
+Waddress.c:70:13: warning: the address of a label will always evaluate as true
+ * check-error-end
+ */
-- 
2.15.0

[PATCH 2/3] add testcases linearization of degenerated arrays/functions

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/linear/degen-array.c    | 31 ++++++++++++++++++++++++++++
 validation/linear/degen-function.c | 38 +++++++++++++++++++++++++++++++++++
 validation/linear/degen-log-not.c  | 41 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 validation/linear/degen-array.c
 create mode 100644 validation/linear/degen-function.c
 create mode 100644 validation/linear/degen-log-not.c

diff --git a/validation/linear/degen-array.c b/validation/linear/degen-array.c
new file mode 100644
index 000000000..b8a6f4f66
--- /dev/null
+++ b/validation/linear/degen-array.c
@@ -0,0 +1,31 @@
+extern int a[3];
+
+int (*fa(int i))[] { return &a; }
+int *f0(int i) { return &a[0]; }
+int *fd(int i) { return  a; }
+
+/*
+ * check-name: degen-array
+ * check-command: test-linearize -m64 -Wno-decl $file
+ *
+ * check-output-start
+fa:
+.L0:
+	<entry-point>
+	ret.64      a
+
+
+f0:
+.L2:
+	<entry-point>
+	ret.64      a
+
+
+fd:
+.L4:
+	<entry-point>
+	ret.64      a
+
+
+ * check-output-end
+ */
diff --git a/validation/linear/degen-function.c b/validation/linear/degen-function.c
new file mode 100644
index 000000000..6dd3123b8
--- /dev/null
+++ b/validation/linear/degen-function.c
@@ -0,0 +1,38 @@
+extern int fun(int);
+
+typedef int (*fun_t)(int);
+
+fun_t fa(void) { return &fun; }
+fun_t f0(void) { return  fun; }
+
+/*
+ * check-name: degen-function
+ * check-command: test-linearize -m64 -Wno-decl -fdump-ir=linearize $file
+ *
+ * check-output-start
+fa:
+.L0:
+	<entry-point>
+	symaddr.64  %r1 <- fun
+	phisrc.64   %phi1(return) <- %r1
+	br          .L1
+
+.L1:
+	phi.64      %r2 <- %phi1(return)
+	ret.64      %r1
+
+
+f0:
+.L2:
+	<entry-point>
+	symaddr.64  %r3 <- fun
+	phisrc.64   %phi2(return) <- %r3
+	br          .L3
+
+.L3:
+	phi.64      %r4 <- %phi2(return)
+	ret.64      %r3
+
+
+ * check-output-end
+ */
diff --git a/validation/linear/degen-log-not.c b/validation/linear/degen-log-not.c
new file mode 100644
index 000000000..baa7d5366
--- /dev/null
+++ b/validation/linear/degen-log-not.c
@@ -0,0 +1,41 @@
+extern int arr[];
+int test_arr_addr(int i)
+{
+	if (!&arr) return 1;
+	return 0;
+}
+
+int test_arr_addr0(int i)
+{
+	if (!&arr[0]) return 1;
+	return 0;
+}
+
+int test_arr_degen(int i)
+{
+	if (!arr) return 1;
+	return 0;
+}
+
+extern int fun(void);
+int test_fun_addr(int i)
+{
+	if (!&fun) return 1;
+	return 0;
+}
+
+int test_fun_degen(int i)
+{
+	if (!fun) return 1;
+	return 0;
+}
+
+/*
+ * check-name: degenerate logical-not
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: load
+ * check-output-excludes: VOID
+ */
-- 
2.15.0

[PATCH 3/3] fix: add missing degenerate() for logical not

[Luc Van Oostenryck]

Expressions involving the logical-not '!' does not
call degenerate().

Since the result type is always 'int' and thus independent
of the expression being negated, this has no effect on the
type-checking but the linearization is wrong.
For example, code like:
	int foo(void)
	{
		if (!arr) return 1;
		return 0;
	}
generates:
	foo:
		load        %r6 <- 0[arr]
		seteq.32    %r7 <- VOID, $0
		ret.32      %r7
The 'load' being, obviously wrong.

Fix this by adding the missing degenerate().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c                        | 1 +
 validation/linear/degen-log-not.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/evaluate.c b/evaluate.c
index 4bca13542..6b3e2c257 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -1914,6 +1914,7 @@ static struct symbol *evaluate_preop(struct expression *expr)
 		return evaluate_postop(expr);
 
 	case '!':
+		ctype = degenerate(expr->unop);
 		expr->flags = expr->unop->flags & ~CEF_CONST_MASK;
 		/*
 		 * A logical negation never yields an address constant
diff --git a/validation/linear/degen-log-not.c b/validation/linear/degen-log-not.c
index baa7d5366..a982e34b2 100644
--- a/validation/linear/degen-log-not.c
+++ b/validation/linear/degen-log-not.c
@@ -33,7 +33,6 @@ int test_fun_degen(int i)
 /*
  * check-name: degenerate logical-not
  * check-command: test-linearize -Wno-decl $file
- * check-known-to-fail
  *
  * check-output-ignore
  * check-output-excludes: load
-- 
2.15.0

Re: [PATCH 3/3] fix: add missing degenerate() for logical not

[Christopher Li]

On Fri, Dec 8, 2017 at 1:16 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Expressions involving the logical-not '!' does not
> call degenerate().
>
> Since the result type is always 'int' and thus independent
> of the expression being negated, this has no effect on the
> type-checking but the linearization is wrong.
> For example, code like:
>         int foo(void)
>         {
>                 if (!arr) return 1;
>                 return 0;
>         }
> generates:
>         foo:
>                 load        %r6 <- 0[arr]
>                 seteq.32    %r7 <- VOID, $0
>                 ret.32      %r7
> The 'load' being, obviously wrong.
>
> Fix this by adding the missing degenerate().

Good catch. This series looks good to me.

Chris

Re: [PATCH 3/3] fix: add missing degenerate() for logical not

[Luc Van Oostenryck]

On Fri, Dec 08, 2017 at 09:49:22AM +0800, Christopher Li wrote:
> On Fri, Dec 8, 2017 at 1:16 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Expressions involving the logical-not '!' does not
> > call degenerate().
> >
> > Since the result type is always 'int' and thus independent
> > of the expression being negated, this has no effect on the
> > type-checking but the linearization is wrong.
> > For example, code like:
> >         int foo(void)
> >         {
> >                 if (!arr) return 1;
> >                 return 0;
> >         }
> > generates:
> >         foo:
> >                 load        %r6 <- 0[arr]
> >                 seteq.32    %r7 <- VOID, $0
> >                 ret.32      %r7
> > The 'load' being, obviously wrong.
> >
> > Fix this by adding the missing degenerate().
> 
> Good catch. This series looks good to me.
> 
> Chris

Thanks,
-- Luc

Re: [PATCH 0/3] add missing degenerate() for logical-not

[Luc Van Oostenryck]

This has now been pushed to:
	git://github.com/lucvoo/sparse.git

-- Luc Van Oostenryck