[#LU-5396] annotations for sparse static analyzer

[LU-5396] annotations for sparse static analyzer Created: 23/Jul/14 Updated: 26/Feb/15 Resolved: 26/Feb/15
Status:	Resolved
Project:	Lustre
Component/s:	None
Affects Version/s:	Lustre 2.6.0
Fix Version/s:	Lustre 2.7.0

Type:

Bug

Priority:

Trivial

Reporter:

Frank Zago (Inactive)

Assignee:

Dmitry Eremin (Inactive)

Resolution:

Fixed

Votes:

Labels:

patch

Issue Links:

Related
is related to	~~LU-2753~~	Tracking bug for static code analysis...	Resolved
is related to	~~LU-6215~~	Sync Lustre external tree with lustre...	Resolved
is related to	~~LU-6302~~	annotations for sparse static analyzer	Resolved

Severity:

Rank (Obsolete):

15020

Description

sparse (git://git.kernel.org/pub/scm/devel/sparse/sparse.git) is a static analyzer used in kernel dev. After installing it, it can easily be used with Lustre:

make C=1

Currently there is about 3900 warnings. Most of it is noise, and a few annotations will silence them. However in that noise, some bugs might be lurking.

Besides finding bugs, it currently tags 1000 functions that can be made static. Once marked static, that will enable the compiler to optimize them more aggressively, which may lead to some minor performance improvements.

So far I've cleared about 1000 of them and found only one bug (strcmp with userspace provided buffer in fld_proc_hash_seq_write) and several unused functions.

Comments

Comment by James A Simmons [ 23/Jul/14 ]

This should be linked with the ~~LU-2753~~ work.

Comment by Andreas Dilger [ 23/Jul/14 ]

Frank,
John has started some work using sparse and annotating functions/parameters with __user and such to find incorrect usage. I would welcome further patches for annotations of the Lustre code to clean up the false positives (and of course fix any resulting bugs) so that sparse can provide more useful output.

I'd recommend not to put too many changes into a single patch, so that they are easier to inspect and land independently and avoid conflicts with other patches.

Comment by Frank Zago (Inactive) [ 23/Jul/14 ]

Not a problem Andreas. This is a grinding work that is best done a bit at a time

use gfp_t: http://review.whamcloud.com/11200 (upstream)

Comment by Frank Zago (Inactive) [ 23/Jul/14 ]

Add __user annotations. This apparently may conflict with John's patches, but I've had these for a while already.
http://review.whamcloud.com/11480 (upstream)
http://review.whamcloud.com/11202 (upstream)
http://review.whamcloud.com/11203 (abandoned)
http://review.whamcloud.com/11204 (abandoned)
http://review.whamcloud.com/11205 (abandoned)
http://review.whamcloud.com/11206 (abandoned)

Comment by Frank Zago (Inactive) [ 31/Jul/14 ]

Locking annotations:
http://review.whamcloud.com/11481 (upstream)
http://review.whamcloud.com/11294 (upstream)
http://review.whamcloud.com/11295

Comment by Frank Zago (Inactive) [ 01/Aug/14 ]

Made some functions static:
http://review.whamcloud.com/11305 (upstream)
http://review.whamcloud.com/11306

http://review.whamcloud.com/11255 (upstream)
http://review.whamcloud.com/11256 (upstream)

Comment by Frank Zago (Inactive) [ 08/Sep/14 ]

Add __user annotations.

Comment by James A Simmons [ 08/Sep/14 ]

Thank you for doing this. A lot of the upstream kernel Lustre client patches are these types of patches. This will help us move closer to the upstream client for style issues much faster.

Comment by Frank Zago (Inactive) [ 08/Sep/14 ]

Add __user annotations (same as above, but 1 patch per branch)
http://review.whamcloud.com/11817 (upstream)
http://review.whamcloud.com/11818 (upstream)
http://review.whamcloud.com/11819 (upstream)
http://review.whamcloud.com/11820 (upstream)
http://review.whamcloud.com/11821 (upstream)
http://review.whamcloud.com/11822 (upstream)
http://review.whamcloud.com/11823 (upstream)
http://review.whamcloud.com/11824 (upstream)

Comment by Peter Jones [ 06/Oct/14 ]

Frank

I see that all the present patches tracked under this ticket have landed. Are there more to come or can we now close this ticket?

Peter

Comment by Frank Zago (Inactive) [ 06/Oct/14 ]

Hi Peter, there's quite a few more patches to come.

Number of warnings is down to less to 1000 now.

Comment by Peter Jones [ 06/Oct/14 ]

A 1000?! The final straight then

Comment by Frank Zago (Inactive) [ 07/Oct/14 ]

Make some static symbols:
http://review.whamcloud.com/12206
http://review.whamcloud.com/12207
http://review.whamcloud.com/12211
http://review.whamcloud.com/12219
http://review.whamcloud.com/12220
http://review.whamcloud.com/12221
http://review.whamcloud.com/12222
http://review.whamcloud.com/12223

Done for this series. There will a new one once this one is integrated.

Comment by Frank Zago (Inactive) [ 05/Nov/14 ]

0 -> NULL:
http://review.whamcloud.com/12567