Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang static analysis #1721

Closed
chrisrd opened this issue Sep 13, 2013 · 6 comments
Closed

Clang static analysis #1721

chrisrd opened this issue Sep 13, 2013 · 6 comments
Labels
Component: Test Suite Indicates an issue with the test framework or a test case Type: Building Indicates an issue related to building binaries
Milestone

Comments

@chrisrd
Copy link
Contributor

chrisrd commented Sep 13, 2013

The code base (zfs + spl) should come up clean under static analysis by clang (and others?). To assist getting and keeping clean, the build tools should be updated to include this analysis, e.g. "make clang".

See Also:

http://thread.gmane.org/gmane.linux.file-systems.zfs.user/1503

@marcin-github
Copy link

Very short instruction:
Install clang with support of static-analyzer (on gentoo: USE=""clang static-analyzer" emerge =llvm-9999)

./autogen.sh && ./configure --with-linux=<correct path> <rest of args> &&  scan-build --html-title="spl at $(git log -n1|grep commit)" -analyze-headers -maxloop 15 make

I'm not familiar with make (and git, probably there is a much better way to determine on which commit are sources) so I know the proposition below is very erroneous but it gives idea what is needed in Makefile:

clang:
        scan-build --html-title="spl at $(git log -n1|grep commit)" -analyze-headers -maxloop 15 make

@ryao
Copy link
Contributor

ryao commented Sep 13, 2013

Unfortunately, static analysis tends to generate false positives when applied to kernel code. A specific false positive is that Clang's static analyzer is written under the assumption that memory allocation functions can return NULL while kmem_alloc() with KM_SLEEP guarantees that they either return a value or never return at all. I suppose we could add code to catch NULL return values to silence it, but that should be unnecessary.

@behlendorf
Copy link
Contributor

Static analyzers such as coverity support annotations to avoid false positives. Does anyone know is clang provides a similar mechanism.

@maci0
Copy link
Contributor

maci0 commented Sep 13, 2013

@marcin-github the ./configure step should probably be run through scan-build as well
see: http://clang-analyzer.llvm.org/scan-build.html

@behlendorf http://clang-analyzer.llvm.org/annotations.html i guess this is what you are looking for

@behlendorf behlendorf removed this from the 0.6.5 milestone Oct 29, 2014
@behlendorf behlendorf added this to the 0.8.0 milestone Jul 15, 2016
@behlendorf behlendorf added the Component: Test Suite Indicates an issue with the test framework or a test case label Jul 15, 2016
@behlendorf
Copy link
Contributor

Related to #1392. For anyone interested in working on this the first step would be getting the existing code base to pass the static analyzer without errors.

@behlendorf
Copy link
Contributor

We've adopted Coverity as our static checker. That said I'd love to be running multiple checkers, for example it would be nice to run this against every PR which we can't do with Coverity. However, until someone has time to work on this I'm closing the issue.

https://scan.coverity.com/projects/zfsonlinux-zfs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Test Suite Indicates an issue with the test framework or a test case Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

No branches or pull requests

5 participants