Powered by Linux
Re: [PATCH 5/5] check_kernel_printf.c: warn about "%lx", (long)ptr — Semantic Matching Tool

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 26, 2017 at 01:23:42PM +0200, Rasmus Villemoes wrote:
> On 26 October 2017 at 13:10, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > On Thu, Oct 26, 2017 at 12:54:20PM +0200, Rasmus Villemoes wrote:
> >> On 26 October 2017 at 12:48, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >> > On Wed, Oct 25, 2017 at 09:25:15PM +0200, Rasmus Villemoes 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().
> >> >
> >> > Well, it is a pointer type.  If you do pointer math, you get pointer
> >> > results.  You can't really treat subtract different from addition
> >> > because container_of() is a subtraction.
> >>
> >> Huh? When I subtract one pointer from another, I get an _integer_.
> >
> > No.  You still get a pointer.  :P  That's just how type promotion works
> > in C.
> 
> C99, 6.5.6:
> 9 When two pointers are subtracted, both shall point to elements of
> the same array object,
> or one past the last element of the array object; the result is the
> difference of the
> subscripts of the two array elements. The size of the result is
> implementation-defined,
> and its type (a signed integer type) is ptrdiff_t defined in the
> <stddef.h> header.
> 
> If sparse behaves differently, it's a bug in sparse. But it may well
> just be me that don't quite understand the various helpers for
> examining type, or how types are actually implemented in sparse. I see
> that evaluate_ptr_sub does expr->ctype = ssize_t_ctype and return
> ssize_t_ctype, so something else must be going on.

Yeah...  This is a bug in Smatch not Sparse.  The problem is that Sparse
was never really designed to be used how Smatch does it so sometimes the
ctype information isn't set up.  Sparse wants people to use the
evaluated tree, but then information like sizeof() is gone and replaced
with literals.  Anway, sometimes it's there and sometimes not.  When
it's not there I cobbled it together in smatch_type.c.

I'm sort of surprised that Sparse does this:

struct symbol *ssize_t_ctype = &int_ctype;

I really thought it should have been &long_ctype instead...  Anyway,
this patch should fix it.  I've pushed your first four patches.  If this
patch is Ok, then resend me patch 5 and I'll push that as well.

regards,
dan carpenter

diff --git a/smatch_type.c b/smatch_type.c
index bb9a816a..e41a1837 100644
--- a/smatch_type.c
+++ b/smatch_type.c
@@ -71,13 +71,16 @@ 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 &&
+	    types_equiv(left, right))
+		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;
 

 	if (!one && !two)
--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux