2010-01-09 08:53:55

by Dan Carpenter

[permalink] [raw]
Subject: [announce] smatch 1.54

Smatch is a static checker for C. Download it here:
http://repo.or.cz/w/smatch.git

These days there are at least seven static checkers people are using
to check the kernel source. I can't swear that it's better than all
the others... Smatch is open source. It's written in C. It's based
on sparse. It's not horribly slow. It has a git repo that gets updated
every few days.

Really what I would like is for smatch to become a smarter version
of checkpatch.pl. I'd like it to be something simple, that catches many
types of common bugs. Something that people run before submitting code.
It's still probably a year away from being good enough for that.

Since the last release:
* You need to pass "-p=kernel" to check the kernel source.
make CHECK="/path/to/smatch -p=kernel" C=1 bzImage

* The output has been cleaned up. You can still pass --spammy
and --info if you want the old output.

* The array overflow check has improved and finds around 30
bugs in the 2.6.33-rc2 kernel.

* There is a new "dma on the stack" check that finds 382
bugs. The ones from drivers/usb/serial/ have fixes already.
This could maybe be rewritten as a Coccinelle script. The
fixes are pretty mechanical.

That's the main stuff.

>From what I can tell about the open source checkers is that only the
authors use them. Smatch has two users besides me (they do userspace
stuff so they're not on this list), so it's one of the most popular.

Anyway, feedback is always good. :)

regards,
dan carpenter


2010-06-02 19:20:52

by Roland Dreier

[permalink] [raw]
Subject: Re: [announce] smatch 1.54

Hi Dan,

I'm trying out smatch on some kernel code now. Neat stuff, but I'm
seeing some puzzling false positives relating to signedness. Basically,
if I have code like:

__be16 foo;
__u16 bar;

foo = cpu_to_be16(128);
bar = cpu_to_be16(128);

then the first line warns about 32768 not fitting into foo, but while
the second line doesn't warn, even though __be16 is just a __bitwise__
__u16. I don't understand the sparse / smatch internals so I'm not sure
why applying __bitwise__ apparently kills the unsigned modifier. Maybe
it's an easy fix for you? Otherwise I'll poke at it a little later.

Thanks,
Roland
--
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

2010-06-02 20:43:06

by Dan Carpenter

[permalink] [raw]
Subject: Re: [announce] smatch 1.54

On Wed, Jun 02, 2010 at 12:20:46PM -0700, Roland Dreier wrote:
> Hi Dan,
>
> I'm trying out smatch on some kernel code now. Neat stuff, but I'm
> seeing some puzzling false positives relating to signedness.

Hm... Apparently the bug is caused because I don't understand sparse
internals very well. The following patch takes care of it, but I'll
need to look at it some more to make sure it's complete.

diff --git a/smatch_type.c b/smatch_type.c
index 8896c3a..afbbe68 100644
--- a/smatch_type.c
+++ b/smatch_type.c
@@ -14,12 +14,22 @@

#include "smatch.h"

+struct symbol *get_real_base_type(struct symbol *sym)
+{
+ struct symbol *ret;
+
+ ret = get_base_type(sym);
+ if (ret->type == SYM_RESTRICT)
+ return get_real_base_type(ret);
+ return ret;
+}
+
static struct symbol *get_type_symbol(struct expression *expr)
{
if (!expr || expr->type != EXPR_SYMBOL || !expr->symbol)
return NULL;

- return get_base_type(expr->symbol);
+ return get_real_base_type(expr->symbol);
}

static struct symbol *get_symbol_from_deref(struct expression *expr)
@@ -38,10 +48,10 @@ static struct symbol *get_symbol_from_deref(struct expression *expr)
return NULL;
}
if (struct_sym->type == SYM_PTR)
- struct_sym = get_base_type(struct_sym);
+ struct_sym = get_real_base_type(struct_sym);
FOR_EACH_PTR(struct_sym->symbol_list, tmp) {
if (tmp->ident == member)
- return get_base_type(tmp);
+ return get_real_base_type(tmp);
} END_FOR_EACH_PTR(tmp);
return NULL;
}
@@ -53,7 +63,7 @@ static struct symbol *get_return_type(struct expression *expr)
tmp = get_type(expr->fn);
if (!tmp)
return NULL;
- return get_base_type(tmp);
+ return get_real_base_type(tmp);
}

static struct symbol *get_pointer_type(struct expression *expr)
@@ -63,7 +73,7 @@ static struct symbol *get_pointer_type(struct expression *expr)
sym = get_type(expr);
if (!sym || sym->type != SYM_PTR)
return NULL;
- return get_base_type(sym);
+ return get_real_base_type(sym);
}

static struct symbol *fake_pointer_sym(struct expression *expr)
@@ -102,14 +112,14 @@ struct symbol *get_type(struct expression *expr)
case EXPR_CAST:
case EXPR_FORCE_CAST:
case EXPR_IMPLIED_CAST:
- return get_base_type(expr->cast_type);
+ return get_real_base_type(expr->cast_type);
case EXPR_BINOP:
if (expr->op != '+')
return NULL;
tmp = get_type(expr->left);
if (!tmp || (tmp->type != SYM_ARRAY && tmp->type != SYM_PTR))
return NULL;
- return get_base_type(tmp);
+ return get_real_base_type(tmp);
case EXPR_CALL:
return get_return_type(expr);
default:

2010-06-03 05:27:12

by Roland Dreier

[permalink] [raw]
Subject: Re: [announce] smatch 1.54

> Hm... Apparently the bug is caused because I don't understand sparse
> internals very well. The following patch takes care of it, but I'll
> need to look at it some more to make sure it's complete.

Thanks. FWIW the patch gets rid of the false positives I was seeing
without introducing any obvious problems.

- R.
--
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html