public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* Re: smatch warnings in current upstream kernel
@ 2012-09-11 10:51 Dan Carpenter
  2012-09-11 11:06 ` Fengguang Wu
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-11 10:51 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Sep 11, 2012 at 06:10:02PM +0800, Fengguang Wu wrote:
> Here are the 7161 smatch warnings for 2139 files in upstream head
> 1a95620f45155ac523cd1419d89150fbb4eb858b.  It's not a complete build
> (no make clean), so does not include all possible warnings.
> 

There are some sparse warnings mixed in as well.  They would have
been sent to stderr and smatch warnings get sent to stdout.

But if you want you could look at smatch_scripts/test_kernel.sh
which allows for a parallel build and saves all the warnings in
warns.txt.

I guess you have your own parallel build technique as well and that
works too.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: smatch warnings in current upstream kernel
  2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
@ 2012-09-11 11:06 ` Fengguang Wu
  2012-09-11 14:29 ` Dan Carpenter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Fengguang Wu @ 2012-09-11 11:06 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Sep 11, 2012 at 01:51:27PM +0300, Dan Carpenter wrote:
> On Tue, Sep 11, 2012 at 06:10:02PM +0800, Fengguang Wu wrote:
> > Here are the 7161 smatch warnings for 2139 files in upstream head
> > 1a95620f45155ac523cd1419d89150fbb4eb858b.  It's not a complete build
> > (no make clean), so does not include all possible warnings.
> > 
> 
> There are some sparse warnings mixed in as well.  They would have
> been sent to stderr and smatch warnings get sent to stdout.

Thanks for pointing this out!

> But if you want you could look at smatch_scripts/test_kernel.sh
> which allows for a parallel build and saves all the warnings in
> warns.txt.
 
Yeah I looked at the script, however did not want to touch the source
tree, so...

> I guess you have your own parallel build technique as well and that
> works too.

I have a tiny smatch wrapper script for passing to CHECK=.
According to your advice, I will only save its stdout:

$SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: smatch warnings in current upstream kernel
  2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
  2012-09-11 11:06 ` Fengguang Wu
@ 2012-09-11 14:29 ` Dan Carpenter
  2012-09-11 14:49 ` Fengguang Wu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-11 14:29 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Sep 11, 2012 at 07:06:40PM +0800, Fengguang Wu wrote:
> On Tue, Sep 11, 2012 at 01:51:27PM +0300, Dan Carpenter wrote:
>
> $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM

Some of the warnings you are getting seem to mean that you're not
picking up the smatch_data/ directory.  For example some of the
buffer overflow errors:

drivers/mtd/ubi/wl.c:336 prot_queue_add() warn: buffer overflow 'ubi->pq' 10 <= 10

What is happening in that function is that Smatch sees the:
	ubi_assert(pq_tail >= 0 && pq_tail < UBI_PROT_QUEUE_LEN);

and assumes that since there is a test for
"pq_tail < UBI_PROT_QUEUE_LEN" then it must be possible for
pq_tail = UBI_PROT_QUEUE_LEN.  There is a file called:
smatch_data/kernel.ignored_macros which has ubi_assert() and it
means to ignore everything that happens inside a ubi_assert().

Normally Smatch looks for the data dir with the binary, but you can
also specify a directory with --data=/path/to/smatch_data/.  Then
you can test that it's working by doing a:
	kchecker drivers/mtd/ubi/wl.c
to verify that the warning goes away.

It may cause a lot of new warnings to show up though...

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: smatch warnings in current upstream kernel
  2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
  2012-09-11 11:06 ` Fengguang Wu
  2012-09-11 14:29 ` Dan Carpenter
@ 2012-09-11 14:49 ` Fengguang Wu
  2012-09-12  2:48 ` Fengguang Wu
  2012-09-12  6:26 ` Dan Carpenter
  4 siblings, 0 replies; 6+ messages in thread
From: Fengguang Wu @ 2012-09-11 14:49 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Sep 11, 2012 at 05:29:26PM +0300, Dan Carpenter wrote:
> On Tue, Sep 11, 2012 at 07:06:40PM +0800, Fengguang Wu wrote:
> > On Tue, Sep 11, 2012 at 01:51:27PM +0300, Dan Carpenter wrote:
> >
> > $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM

FYI, the wrapper script is as simple as

SMATCH=/c/smatch/smatch
$SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM

> Some of the warnings you are getting seem to mean that you're not
> picking up the smatch_data/ directory.  For example some of the
> buffer overflow errors:
> 
> drivers/mtd/ubi/wl.c:336 prot_queue_add() warn: buffer overflow 'ubi->pq' 10 <= 10
> 
> What is happening in that function is that Smatch sees the:
> 	ubi_assert(pq_tail >= 0 && pq_tail < UBI_PROT_QUEUE_LEN);
> 
> and assumes that since there is a test for
> "pq_tail < UBI_PROT_QUEUE_LEN" then it must be possible for
> pq_tail = UBI_PROT_QUEUE_LEN.  There is a file called:
> smatch_data/kernel.ignored_macros which has ubi_assert() and it
> means to ignore everything that happens inside a ubi_assert().

Yes, I can see the data and function. Thank you very much for the tips!

> Normally Smatch looks for the data dir with the binary, but you can
> also specify a directory with --data=/path/to/smatch_data/.  Then

The directory layout here is

/c/smatch/                      # git tree 
/c/smatch/smatch                # binary
/c/smatch/smatch_data/          # kernel.* data

> you can test that it's working by doing a:
> 	kchecker drivers/mtd/ubi/wl.c
> to verify that the warning goes away.

Both of these two tests run well w/o the above warning:

/c/smatch/smatch_scripts/kchecker drivers/mtd/ubi/wl.c

make C=1 CHECK=/c/kernel-tests/smatchcheck drivers/mtd/ubi/wl.o

This is interesting. I'll try adding --data=/c/smatch/smatch_data and
check whether the end result becomes better.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: smatch warnings in current upstream kernel
  2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
                   ` (2 preceding siblings ...)
  2012-09-11 14:49 ` Fengguang Wu
@ 2012-09-12  2:48 ` Fengguang Wu
  2012-09-12  6:26 ` Dan Carpenter
  4 siblings, 0 replies; 6+ messages in thread
From: Fengguang Wu @ 2012-09-12  2:48 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Sep 11, 2012 at 10:49:39PM +0800, Fengguang Wu wrote:
> On Tue, Sep 11, 2012 at 05:29:26PM +0300, Dan Carpenter wrote:
> > On Tue, Sep 11, 2012 at 07:06:40PM +0800, Fengguang Wu wrote:
> > > On Tue, Sep 11, 2012 at 01:51:27PM +0300, Dan Carpenter wrote:
> > >
> > > $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM
> 
> FYI, the wrapper script is as simple as
> 
> SMATCH=/c/smatch/smatch
> $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM
> 
> > Some of the warnings you are getting seem to mean that you're not
> > picking up the smatch_data/ directory.  For example some of the
> > buffer overflow errors:
> > 
> > drivers/mtd/ubi/wl.c:336 prot_queue_add() warn: buffer overflow 'ubi->pq' 10 <= 10
> > 
> > What is happening in that function is that Smatch sees the:
> > 	ubi_assert(pq_tail >= 0 && pq_tail < UBI_PROT_QUEUE_LEN);
> > 
> > and assumes that since there is a test for
> > "pq_tail < UBI_PROT_QUEUE_LEN" then it must be possible for
> > pq_tail = UBI_PROT_QUEUE_LEN.  There is a file called:
> > smatch_data/kernel.ignored_macros which has ubi_assert() and it
> > means to ignore everything that happens inside a ubi_assert().
> 
> Yes, I can see the data and function. Thank you very much for the tips!
> 
> > Normally Smatch looks for the data dir with the binary, but you can
> > also specify a directory with --data=/path/to/smatch_data/.  Then
> 
> The directory layout here is
> 
> /c/smatch/                      # git tree 
> /c/smatch/smatch                # binary
> /c/smatch/smatch_data/          # kernel.* data
> 
> > you can test that it's working by doing a:
> > 	kchecker drivers/mtd/ubi/wl.c
> > to verify that the warning goes away.
> 
> Both of these two tests run well w/o the above warning:
> 
> /c/smatch/smatch_scripts/kchecker drivers/mtd/ubi/wl.c
> 
> make C=1 CHECK=/c/kernel-tests/smatchcheck drivers/mtd/ubi/wl.o
> 
> This is interesting. I'll try adding --data=/c/smatch/smatch_data and
> check whether the end result becomes better.

It works now, after changing '-p kernel' to '--project=kernel'.

Next step would be to regenerate the smatch_data/ contents for each
new branch to make it work more reliably.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: smatch warnings in current upstream kernel
  2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
                   ` (3 preceding siblings ...)
  2012-09-12  2:48 ` Fengguang Wu
@ 2012-09-12  6:26 ` Dan Carpenter
  4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-12  6:26 UTC (permalink / raw)
  To: kernel-janitors

On Wed, Sep 12, 2012 at 10:48:42AM +0800, Fengguang Wu wrote:
> On Tue, Sep 11, 2012 at 10:49:39PM +0800, Fengguang Wu wrote:
> > On Tue, Sep 11, 2012 at 05:29:26PM +0300, Dan Carpenter wrote:
> > > On Tue, Sep 11, 2012 at 07:06:40PM +0800, Fengguang Wu wrote:
> > > > On Tue, Sep 11, 2012 at 01:51:27PM +0300, Dan Carpenter wrote:
> > > >
> > > > $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM
> > 
> > FYI, the wrapper script is as simple as
> > 
> > SMATCH=/c/smatch/smatch
> > $SMATCH -p kernel "$@" > $SMATCH_OUT_ROOT/$PPID-$$-$RANDOM
> > 
> > > Some of the warnings you are getting seem to mean that you're not
> > > picking up the smatch_data/ directory.  For example some of the
> > > buffer overflow errors:
> > > 
> > > drivers/mtd/ubi/wl.c:336 prot_queue_add() warn: buffer overflow 'ubi->pq' 10 <= 10
> > > 
> > > What is happening in that function is that Smatch sees the:
> > > 	ubi_assert(pq_tail >= 0 && pq_tail < UBI_PROT_QUEUE_LEN);
> > > 
> > > and assumes that since there is a test for
> > > "pq_tail < UBI_PROT_QUEUE_LEN" then it must be possible for
> > > pq_tail = UBI_PROT_QUEUE_LEN.  There is a file called:
> > > smatch_data/kernel.ignored_macros which has ubi_assert() and it
> > > means to ignore everything that happens inside a ubi_assert().
> > 
> > Yes, I can see the data and function. Thank you very much for the tips!
> > 
> > > Normally Smatch looks for the data dir with the binary, but you can
> > > also specify a directory with --data=/path/to/smatch_data/.  Then
> > 
> > The directory layout here is
> > 
> > /c/smatch/                      # git tree 
> > /c/smatch/smatch                # binary
> > /c/smatch/smatch_data/          # kernel.* data
> > 
> > > you can test that it's working by doing a:
> > > 	kchecker drivers/mtd/ubi/wl.c
> > > to verify that the warning goes away.
> > 
> > Both of these two tests run well w/o the above warning:
> > 
> > /c/smatch/smatch_scripts/kchecker drivers/mtd/ubi/wl.c
> > 
> > make C=1 CHECK=/c/kernel-tests/smatchcheck drivers/mtd/ubi/wl.o
> > 
> > This is interesting. I'll try adding --data=/c/smatch/smatch_data and
> > check whether the end result becomes better.
> 
> It works now, after changing '-p kernel' to '--project=kernel'.

Oops.  Yeah.  -p=kernel would also work.

> 
> Next step would be to regenerate the smatch_data/ contents for each
> new branch to make it work more reliably.
> 

The smatch_scripts/build_kernel_data.sh does this.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-09-12  6:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-11 10:51 smatch warnings in current upstream kernel Dan Carpenter
2012-09-11 11:06 ` Fengguang Wu
2012-09-11 14:29 ` Dan Carpenter
2012-09-11 14:49 ` Fengguang Wu
2012-09-12  2:48 ` Fengguang Wu
2012-09-12  6:26 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox