[PATCH 0/3] type fixes, printf warning

[PATCH 0/3] type fixes, printf warning

[Rasmus Villemoes]

Hi Dan

Your patch wasn't quite sufficient, since it didn't cover the (rather
common) case of ptr-arr - the array didn't undergo pointer decay in
the eyes of types_equiv. This seems to work ok for
check_kernel_printf, but I haven't done anything to see if this breaks
the rest of smatch.

Rasmus Villemoes (3):
  smatch_type.c: fix type of pointer diff
  smatch_type.c: comparison expressions always have type int
  check_kernel_printf.c: warn about "%lx", (long)ptr

 check_kernel_printf.c | 31 +++++++++++++++++++++++++++++++
 smatch_type.c         | 12 +++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH 1/3] smatch_type.c: fix type of pointer diff

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
---
 smatch_type.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/smatch_type.c b/smatch_type.c
index bb9a816a..0fa61725 100644
--- a/smatch_type.c
+++ b/smatch_type.c
@@ -71,13 +71,17 @@ static struct symbol *get_binop_type(struct expression *expr)
 			return &int_ctype;
 		return left;
 	}
-	if (left->type == SYM_PTR || left->type == SYM_ARRAY)
-		return left;
-
 	right = get_type(expr->right);
 	if (!right)
 		return NULL;
 
+	if (expr->op == '-' &&
+	    (left->type == SYM_PTR || left->type == SYM_ARRAY) &&
+	    (right->type == SYM_PTR || right->type == SYM_ARRAY))
+		return ssize_t_ctype;
+
+	if (left->type == SYM_PTR || left->type == SYM_ARRAY)
+		return left;
 	if (right->type == SYM_PTR || right->type == SYM_ARRAY)
 		return right;
 
-- 
2.11.0

[PATCH 2/3] smatch_type.c: comparison expressions always have type int

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
---
 smatch_type.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/smatch_type.c b/smatch_type.c
index 0fa61725..9080ce62 100644
--- a/smatch_type.c
+++ b/smatch_type.c
@@ -255,6 +255,8 @@ struct symbol *get_type(struct expression *expr)
 		ret = get_real_base_type(expr->cast_type);
 		break;
 	case EXPR_COMPARE:
+		ret = &int_ctype;
+		break;
 	case EXPR_BINOP:
 		ret = get_binop_type(expr);
 		break;
-- 
2.11.0

[PATCH 3/3] check_kernel_printf.c: warn about "%lx", (long)ptr

[Rasmus Villemoes]

For some reason this spits out an enourmous amount of false positives,
making this entirely useless. We hit a lot of "%lx", (long)(a - b),
but I don't understand why the a-b expression (a pointer difference)
passes is_ptr_type().

Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
---
 check_kernel_printf.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/check_kernel_printf.c b/check_kernel_printf.c
index e5f788bd..f9f5ea6b 100644
--- a/check_kernel_printf.c
+++ b/check_kernel_printf.c
@@ -939,6 +939,35 @@ static bool is_integer_specifier(int type)
 	}
 }
 
+static int
+is_cast_expr(struct expression *expr)
+{
+	switch (expr->type) {
+	case EXPR_CAST:
+	case EXPR_FORCE_CAST:
+		/* not EXPR_IMPLIED_CAST for our purposes */
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static void
+check_cast_from_pointer(const char *fmt, int len, struct expression *arg, int va_idx)
+{
+	/*
+	 * This can easily be fooled by passing 0+(long)ptr or doing
+	 * "long local_var = (long)ptr" and passing local_var to
+	 * %lx. Tough.
+	 */
+	if (!is_cast_expr(arg))
+		return;
+	while (is_cast_expr(arg))
+		arg = arg->cast_expression;
+	if (is_ptr_type(get_type(arg)))
+		sm_msg("warn: argument %d to %.*s specifier is cast from pointer",
+			va_idx, len, fmt);
+}
 
 static void
 do_check_printf_call(const char *caller, const char *name, struct expression *callexpr, struct expression *fmtexpr, int vaidx)
@@ -1015,6 +1044,8 @@ do_check_printf_call(const char *caller, const char *name, struct expression *ca
 			if (spec.base != 16 && has_hex_prefix(orig_fmt, old_fmt))
 				sm_msg("warn: '%.2s' prefix is confusing together with '%.*s' specifier",
 				       old_fmt-2, (int)(fmt-old_fmt), old_fmt);
+
+			check_cast_from_pointer(old_fmt, read, arg, vaidx);
 		}
 
 		switch (spec.type) {
-- 
2.11.0

Re: [PATCH 3/3] check_kernel_printf.c: warn about "%lx", (long)ptr

[Rasmus Villemoes]

On 28 October 2017 at 01:22, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> For some reason this spits out an enourmous amount of false positives,
> making this entirely useless. We hit a lot of "%lx", (long)(a - b),
> but I don't understand why the a-b expression (a pointer difference)
> passes is_ptr_type().

gaah, forgot to update the commit message. This should just be
deleted; I think the subject says all that's needed.

Re: [PATCH 2/3] smatch_type.c: comparison expressions always have type int

[Dan Carpenter]

I applied these patches and then had to revert this one because a lot of
Smatch was written with the assumption that we could figure out the type
promotion from calling get_type() on the comparison condition...

regards,
dan carpenter