[PATCH 0/5] expressions without a type

[PATCH 0/5] expressions without a type

[Luc Van Oostenryck]

This series contains a few patches dealing with expressions
without a type and improving the diagnostics related to them.


The series is also available for review in the git repository at:

  git://github.com/lucvoo/sparse.git expr-notype

----------------------------------------------------------------
Luc Van Oostenryck (5):
      do not linearize expressions without a type
      add helper: valid_type()
      add helper: valid_expr_subtype()
      do not report bad types twice or more
      always evaluate both operands

 evaluate.c  | 38 ++++++++++++++++++++++++++++----------
 linearize.c |  2 +-
 symbol.h    |  5 +++++
 3 files changed, 34 insertions(+), 11 deletions(-)

[PATCH 1/5] do not linearize expressions without a type

[Luc Van Oostenryck]

Expressions without a type are the result of previous
errors which left them unevaluated or only partially
evaluated.

Further processing, like here linearization, then
produce nonsensical or erroneous results, with error
messages unrelated to the cause of the error, like:
"unknown expression (...)" or "call with no type!".

Fix this by refusing to linearize an expressions without
a type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linearize.c b/linearize.c
index ba76397ea..c36735cdd 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1581,7 +1581,7 @@ static void linearize_argument(struct entrypoint *ep, struct symbol *arg, int nr
 
 pseudo_t linearize_expression(struct entrypoint *ep, struct expression *expr)
 {
-	if (!expr)
+	if (!expr || !expr->ctype)
 		return VOID;
 
 	current_pos = expr->pos;
-- 
2.14.0

[PATCH 2/5] add helper: valid_type()

[Luc Van Oostenryck]

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

diff --git a/symbol.h b/symbol.h
index 327449611..230057d1d 100644
--- a/symbol.h
+++ b/symbol.h
@@ -313,6 +313,11 @@ extern void debug_symbol(struct symbol *);
 extern void merge_type(struct symbol *sym, struct symbol *base_type);
 extern void check_declaration(struct symbol *sym);
 
+static inline int valid_type(const struct symbol *ctype)
+{
+	return ctype && ctype != &bad_ctype;
+}
+
 static inline struct symbol *get_base_type(const struct symbol *sym)
 {
 	return examine_symbol_type(sym->ctype.base_type);
-- 
2.14.0

[PATCH 3/5] add helper: valid_expr_subtype()

[Luc Van Oostenryck]

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

diff --git a/evaluate.c b/evaluate.c
index 649e132b8..0394dcbb9 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -47,6 +47,17 @@ struct symbol *current_fn;
 static struct symbol *degenerate(struct expression *expr);
 static struct symbol *evaluate_symbol(struct symbol *sym);
 
+static inline int valid_expr_type(struct expression *expr)
+{
+	return expr && valid_type(expr->ctype);
+}
+
+static inline int valid_expr_subtype(struct expression *expr)
+{
+	return valid_expr_type(expr->left)
+	    && valid_expr_type(expr->right);
+}
+
 static struct symbol *evaluate_symbol_expression(struct expression *expr)
 {
 	struct expression *addr;
-- 
2.14.0

[PATCH 4/5] do not report bad types twice or more

[Luc Van Oostenryck]

The type 'bad_ctype' is only used after an error has been detected.
Since this error has also been reported, there is no reasons
to issue more errors when a 'bad_ctype' is involved. This allow
to focus on the root cause of the error.

Fix this by checking in bad_expr_type() if one of the operands
is already a 'bad_ctype' and do not issue an diagnostic message
in this case.

Note: the kernel has a bunch of these situation where sometimes
      the exact same warning is given several times in a row,
      sometimes as much as a dozen time.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/evaluate.c b/evaluate.c
index 0394dcbb9..c16ee9624 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -400,15 +400,20 @@ static inline int is_string_type(struct symbol *type)
 
 static struct symbol *bad_expr_type(struct expression *expr)
 {
-	sparse_error(expr->pos, "incompatible types for operation (%s)", show_special(expr->op));
 	switch (expr->type) {
 	case EXPR_BINOP:
 	case EXPR_COMPARE:
+		if (!valid_expr_subtype(expr))
+			break;
+		sparse_error(expr->pos, "incompatible types for operation (%s)", show_special(expr->op));
 		info(expr->pos, "   left side has type %s", show_typename(expr->left->ctype));
 		info(expr->pos, "   right side has type %s", show_typename(expr->right->ctype));
 		break;
 	case EXPR_PREOP:
 	case EXPR_POSTOP:
+		if (!valid_type(expr->unop->ctype))
+			break;
+		sparse_error(expr->pos, "incompatible types for operation (%s)", show_special(expr->op));
 		info(expr->pos, "   argument has type %s", show_typename(expr->unop->ctype));
 		break;
 	default:
@@ -887,6 +892,8 @@ static struct symbol *evaluate_conditional(struct expression *expr, int iterator
 			if (Waddress)
 				warning(expr->pos, "the address of %s will always evaluate as true", "an array");
 		} else if (!is_scalar_type(ctype)) {
+			if (!valid_type(ctype))
+				return NULL;
 			sparse_error(expr->pos, "incorrect type in conditional");
 			info(expr->pos, "   got %s", show_typename(ctype));
 			ctype = NULL;
-- 
2.14.0

[PATCH 5/5] always evaluate both operands

[Luc Van Oostenryck]

When evaluating a binary expression, the evaluation
already stop if the left operand is found erroneous.
The right one is thus not evaluated but it may contain
another error which will only be diagnosticated after the
first one is corrected and the file rechecked.
This is especially annoying when there are several independent
errors in some complex expression since it will need several
cycles of check-edit-recheck to get all errrors out.

Fix this by always evaluating both left & right operands
(and returning NULL if one of them is erroneous).

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index c16ee9624..9f8cf5be4 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3101,9 +3101,9 @@ struct symbol *evaluate_expression(struct expression *expr)
 	case EXPR_SYMBOL:
 		return evaluate_symbol_expression(expr);
 	case EXPR_BINOP:
-		if (!evaluate_expression(expr->left))
-			return NULL;
-		if (!evaluate_expression(expr->right))
+		evaluate_expression(expr->left);
+		evaluate_expression(expr->right);
+		if (!valid_expr_subtype(expr))
 			return NULL;
 		return evaluate_binop(expr);
 	case EXPR_LOGICAL:
@@ -3114,15 +3114,15 @@ struct symbol *evaluate_expression(struct expression *expr)
 			return NULL;
 		return evaluate_comma(expr);
 	case EXPR_COMPARE:
-		if (!evaluate_expression(expr->left))
-			return NULL;
-		if (!evaluate_expression(expr->right))
+		evaluate_expression(expr->left);
+		evaluate_expression(expr->right);
+		if (!valid_expr_subtype(expr))
 			return NULL;
 		return evaluate_compare(expr);
 	case EXPR_ASSIGNMENT:
-		if (!evaluate_expression(expr->left))
-			return NULL;
-		if (!evaluate_expression(expr->right))
+		evaluate_expression(expr->left);
+		evaluate_expression(expr->right);
+		if (!valid_expr_subtype(expr))
 			return NULL;
 		return evaluate_assignment(expr);
 	case EXPR_PREOP:
-- 
2.14.0