[PATCH 0/7] fix function or array address in conditionals

[PATCH 0/7] fix function or array address in conditionals

[Luc Van Oostenryck]

This serie fixes the evaluation of function or array used
in conditionals which were not degenerated as pointers.
It also teaches sparse to now give some warnings for such uses
as it's a probable sign of an error.

This serie can also be found on github as:
	https://github.com/lucvoo/sparse fix-cond-address


Luc Van Oostenryck (7):
  teach sparse about -Waddress
  add is_func_type()
  warn if testing the address of a function
  add is_array_type()
  warn if testing the address of an array
  fix evaluation of a function or array symbol in conditionals
  fix is_scalar_type()

 evaluate.c                         |  9 ++++++++-
 lib.c                              |  2 ++
 lib.h                              |  1 +
 symbol.h                           | 16 ++++++++++++++--
 validation/cond-address-array.c    | 26 ++++++++++++++++++++++++++
 validation/cond-address-function.c | 18 ++++++++++++++++++
 validation/cond-address.c          | 14 ++++++++++++++
 7 files changed, 83 insertions(+), 3 deletions(-)
 create mode 100644 validation/cond-address-array.c
 create mode 100644 validation/cond-address-function.c
 create mode 100644 validation/cond-address.c

-- 
2.12.0

[PATCH 1/7] teach sparse about -Waddress

[Luc Van Oostenryck]

This is used by GCC to report suspicious use of addresses.

So, reuse the same name for same or similar use.

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

diff --git a/lib.c b/lib.c
index 272d2c88a..f4d5760b7 100644
--- a/lib.c
+++ b/lib.c
@@ -215,6 +215,7 @@ void die(const char *fmt, ...)
 static struct token *pre_buffer_begin = NULL;
 static struct token *pre_buffer_end = NULL;
 
+int Waddress = 0;
 int Waddress_space = 1;
 int Wbitwise = 1;
 int Wcast_to_as = 0;
@@ -467,6 +468,7 @@ static const struct warning {
 	const char *name;
 	int *flag;
 } warnings[] = {
+	{ "address", &Waddress },
 	{ "address-space", &Waddress_space },
 	{ "bitwise", &Wbitwise },
 	{ "cast-to-as", &Wcast_to_as },
diff --git a/lib.h b/lib.h
index 134e56040..0bf8c97e4 100644
--- a/lib.h
+++ b/lib.h
@@ -101,6 +101,7 @@ extern void add_pre_buffer(const char *fmt, ...) FORMAT_ATTR(1);
 
 extern int preprocess_only;
 
+extern int Waddress;
 extern int Waddress_space;
 extern int Wbitwise;
 extern int Wcast_to_as;
-- 
2.12.0

[PATCH 2/7] add is_func_type()

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/symbol.h b/symbol.h
index 36f8345b5..ee17ba643 100644
--- a/symbol.h
+++ b/symbol.h
@@ -347,6 +347,13 @@ static inline int is_ptr_type(struct symbol *type)
 	return type->type == SYM_PTR || type->type == SYM_ARRAY || type->type == SYM_FN;
 }
 
+static inline int is_func_type(struct symbol *type)
+{
+	if (type->type == SYM_NODE)
+		type = type->ctype.base_type;
+	return type->type == SYM_FN;
+}
+
 static inline int is_float_type(struct symbol *type)
 {
 	if (type->type == SYM_NODE)
-- 
2.12.0

[PATCH 3/7] warn if testing the address of a function

[Luc Van Oostenryck]

Testing the address of a function is quite suspicious:
it's most probably the sign of an error somewhere.
Furthermore, such uses always evaluate to true.

So, add a warning about such use (but only if -Waddress was given).

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c                         |  5 ++++-
 validation/cond-address-function.c | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 validation/cond-address-function.c

diff --git a/evaluate.c b/evaluate.c
index 47eeaef2e..d8ec1c2e3 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -869,7 +869,10 @@ static struct symbol *evaluate_conditional(struct expression *expr, int iterator
 	if (ctype) {
 		if (is_safe_type(ctype))
 			warning(expr->pos, "testing a 'safe expression'");
-		if (!is_scalar_type(ctype)) {
+		if (is_func_type(ctype)) {
+			if (Waddress)
+				warning(expr->pos, "the address of %s will always evaluate as true", "a function");
+		} else if (!is_scalar_type(ctype)) {
 			sparse_error(expr->pos, "incorrect type in conditional");
 			info(expr->pos, "   got %s", show_typename(ctype));
 			ctype = NULL;
diff --git a/validation/cond-address-function.c b/validation/cond-address-function.c
new file mode 100644
index 000000000..9a143a009
--- /dev/null
+++ b/validation/cond-address-function.c
@@ -0,0 +1,18 @@
+extern void func(void);
+
+int global_function(void)
+{
+	if (func)
+		return 1;
+	return 0;
+}
+
+/*
+ * check-name: cond-address-function
+ * check-command: test-linearize -Wno-decl -Waddress $file
+ * check-output-ignore
+ *
+ * check-error-start
+cond-address-function.c:5:13: warning: the address of a function will always evaluate as true
+ * check-error-end
+ */
-- 
2.12.0

[PATCH 4/7] add is_array_type()

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/symbol.h b/symbol.h
index ee17ba643..512a2f69f 100644
--- a/symbol.h
+++ b/symbol.h
@@ -354,6 +354,13 @@ static inline int is_func_type(struct symbol *type)
 	return type->type == SYM_FN;
 }
 
+static inline int is_array_type(struct symbol *type)
+{
+	if (type->type == SYM_NODE)
+		type = type->ctype.base_type;
+	return type->type == SYM_ARRAY;
+}
+
 static inline int is_float_type(struct symbol *type)
 {
 	if (type->type == SYM_NODE)
-- 
2.12.0

[PATCH 5/7] warn if testing the address of an array

[Luc Van Oostenryck]

Testing the address of an array is quite suspicious:
it's most probably the sign of an error somewhere.
Furthermore, such uses always evaluate to true.

So, add a warning about such use (but only if -Waddress was given).
---
 evaluate.c                      |  3 +++
 validation/cond-address-array.c | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 validation/cond-address-array.c

diff --git a/evaluate.c b/evaluate.c
index d8ec1c2e3..f2d95c79d 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -872,6 +872,9 @@ static struct symbol *evaluate_conditional(struct expression *expr, int iterator
 		if (is_func_type(ctype)) {
 			if (Waddress)
 				warning(expr->pos, "the address of %s will always evaluate as true", "a function");
+		} else if (is_array_type(ctype)) {
+			if (Waddress)
+				warning(expr->pos, "the address of %s will always evaluate as true", "an array");
 		} else if (!is_scalar_type(ctype)) {
 			sparse_error(expr->pos, "incorrect type in conditional");
 			info(expr->pos, "   got %s", show_typename(ctype));
diff --git a/validation/cond-address-array.c b/validation/cond-address-array.c
new file mode 100644
index 000000000..e1d2f87f8
--- /dev/null
+++ b/validation/cond-address-array.c
@@ -0,0 +1,26 @@
+int foo(void) {
+	extern int a[];
+
+	if (a)
+		return 1;
+	return 0;
+}
+
+int bar(void) {
+	int a[2];
+
+	if (a)
+		return 1;
+	return 0;
+}
+
+/*
+ * check-name: cond-address-array.c
+ * check-command: test-linearize -Wno-decl -Waddress $file
+ * check-output-ignore
+ *
+ * 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
+ * check-error-end
+ */
-- 
2.12.0

[PATCH 6/7] fix evaluation of a function or array symbol in conditionals

[Luc Van Oostenryck]

Functions and arrays degenerate as pointers in most context
but this wasn't done in the case of conditionals.

As a result the value of the conditional after linearization
was invalid.

Fix this by calling degenerate() in evaluate_conditional().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c                |  1 +
 validation/cond-address.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 validation/cond-address.c

diff --git a/evaluate.c b/evaluate.c
index f2d95c79d..976857915 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -881,6 +881,7 @@ static struct symbol *evaluate_conditional(struct expression *expr, int iterator
 			ctype = NULL;
 		}
 	}
+	ctype = degenerate(expr);
 
 	return ctype;
 }
diff --git a/validation/cond-address.c b/validation/cond-address.c
new file mode 100644
index 000000000..2a69f4b92
--- /dev/null
+++ b/validation/cond-address.c
@@ -0,0 +1,14 @@
+extern void f(void);
+extern int a[];
+
+int foo(void) { if (f) return 1; return 0; }
+int bar(void) { if (a) return 1; return 0; }
+int qux(void) { if (f && a) return 1; return 0; }
+
+/*
+ * check-name: cond-address.c
+ * check-command: test-linearize -Wno-decl $file
+ * check-output-ignore
+ *
+ * check-excludes: VOID
+ */
-- 
2.12.0

[PATCH 7/7] fix is_scalar_type()

[Luc Van Oostenryck]

This helper was introduced to add some warning in conditionals
which must accept arrays and functions as those degenerate as
pointers in this context.

There was some sign of a bug somewhere but it wasn't seen
and as consequence arrays and function type were added to
scalar type (yeah, brown paper bag).

Now that the bug is solved, fix this by removing array and
function type from is_scalar_type().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/symbol.h b/symbol.h
index 512a2f69f..327449611 100644
--- a/symbol.h
+++ b/symbol.h
@@ -395,8 +395,6 @@ static inline int is_scalar_type(struct symbol *type)
 	case SYM_ENUM:
 	case SYM_BITFIELD:
 	case SYM_PTR:
-	case SYM_ARRAY:		// OK, will be a PTR after conversion
-	case SYM_FN:
 	case SYM_RESTRICT:	// OK, always integer types
 		return 1;
 	default:
-- 
2.12.0

[GIT PULL] fix function or array address in conditionals

[Luc Van Oostenryck]

This series is available at:
	git://github.com/lucvoo/sparse.git fix-cond-address
based on commit:
	97ebb345943918a0688b62cbc9bf878de01ccba4 (sparse-next-2017-03-07)
up to commit:
	ecda2093f19088fa94c3a4ac85359c6d0f1c64af


-- Luc Van Oostenryck