Clang Analyzer was Re: [PATCH 3/7 v2] staging/ozwpan: Fix NULL vs zero in ozeltbuf.c (sparse warning

Clang Analyzer was Re: [PATCH 3/7 v2] staging/ozwpan: Fix NULL vs zero in ozeltbuf.c (sparse warning

[Peter Huewe]

[-- Attachment #1: Type: Text/Plain, Size: 4487 bytes --]

Am Freitag, 15. Februar 2013, 15:52:26 schrieb Dan Carpenter:
> > @@ -151,7 +151,7 @@ int oz_elt_stream_create(struct oz_elt_buf *buf, u8
> > id, int max_buf_count)
> > 
> >  int oz_elt_stream_delete(struct oz_elt_buf *buf, u8 id)
> >  {
> >  
> >  	struct list_head *e;
> > 
> > -	struct oz_elt_stream *st;
> > +	struct oz_elt_stream *st = NULL;
> > 
> >  	oz_trace("oz_elt_stream_delete(0x%x)\n", id);
> >  	spin_lock_bh(&buf->lock);
> >  	e = buf->stream_list.next;
> 
> You changed the code here.  The original code would crash if
> buf->stream_list was empty.  I don't if that can happen, but I still
> consider it a bug fix.

Yeah - you're right. It's a bug fix and I should have mentioned it.

> You've fixed a couple of these uninitialized variable bugs recently.
> Is this is a clang warning?  GCC doesn't catch it.

(Added janitors on CC as it might be interesting for people over there as 
well).

Exactly, 
Clang reports it as "Branch condition evaluates to a garbage value"

I usually do sparse, smatch and coccicheck, but
lately I've been doing some research on using clang as a static code analyzer, 
especially with the _awesome_ scan-build tool / scan-view frontend.
It works great most of the time, but it requires quite some time to evaluate 
the results and sort out the false positives (which are quite high in number). 

With scan build you can simply type  something like

 scan-build --keep-going -o /tmp make CC="ccc-analyzer -isystem /data/linux-
staging/include/linux/" -C /data/linux-staging/ M=`pwd` -j15 

And then get the results in a nice web browser interface with scan-view.
scan-view is a simple local webserver display the results, also let's you open 
the files directly and send out bug reports but is not really necessary, you 
can also open the html files directly. (without the the bug report and file open 
capabilty)

The syntax for scan-build is more or less
scan-build make CC="ccc-analyzer" MAKE_OPTIONS
where MAKE_OPTIONS is simply the rest of your make commandline, so it can be 
almost anything.

I usually add --keep-going to scan-build so that I get some report if 
something fails and also add
-isystem to ccc-analyzer (which is clang) to (try to) silence some warnings in 
system headers.

I did attach the output of some intermediate result, which you can simply open 
in your webbrowser.
If you open report-tNqJkl.html in your browser (or rather 2013-02-15-1/report-
tNqJkl.html#EndPath) you see the exact flow how this bug could be triggered.

For those who don't want to open the attachment it looks something like this:


151	int oz_elt_stream_delete(struct oz_elt_buf *buf, u8 id)
152	{
153		struct list_head *e;
154		struct oz_elt_stream *st;
155		oz_trace("oz_elt_stream_delete(0x%x)\n", id);
156		spin_lock_bh(&buf->lock);
	
->C1	Calling 'spin_lock_bh'	
->C2	Returning from 'spin_lock_bh'	

157		e = buf->stream_list.next;
158		while (e != &buf->stream_list) {
	
->C3	Loop condition is false. Execution continues on line 166	

159			st = container_of(e, struct oz_elt_stream, link);
160			if (st->id == id) {
161				list_del(e);
162				break;
163			}
164			st = NULL;
165		}
166		if (!st) {
	
->C4	Branch condition evaluates to a garbage value

167			spin_unlock_bh(&buf->lock);
168			return -1;
169		}


I marked the clang annotations with ->C prefix.

Of course the web interface is MUCH better.



The only real issue _I_ currently have with clang is that is has some problems 
with some inline assembler code which he always fails to compile. 
For static analysis purposes I simply have 5 patches with comment these 
passages out.
As far as I've heard it works out of the box for most people even without 
these patches - maybe I compiled clang/llvm incorrectly.

And of course the high number false positives and stuff I simply don't 
understand ;) (e.g. "bugs" with a path length of over 128 ;)

In the llvm _source_ package the tools are located under:
llvm/tools/clang/tools/scan-build/
llvm/tools/clang/tools/scan-view/
not in the llvm-build directory!
 (yes this is a bit strange)



If anyone want's something 'analyzed' with clang by me, simply drop me a mail 
and I can let it run on whatever code you want.
If there is interest I could send out the clang report for the whole staging 
subsystem ;) which I suprisingly have laying around ;)

If a more detailed write up on howto setup clang and how to use it as a static 
code analyzer for the kernel I could proably write something about it.


Thanks,
PeterH



[-- Attachment #2: clang-results.tar.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 52460 bytes --]

Re: Clang Analyzer was Re: [PATCH 3/7 v2] staging/ozwpan: Fix NULL vs zero in ozeltbuf.c (sparse war

[Dan Carpenter]

Are you using this version of clang?:

http://git.linuxfoundation.org/llvmlinux.git/

regards,
dan carpenter

Re: Clang Analyzer was Re: [PATCH 3/7 v2] staging/ozwpan: Fix NULL vs zero in ozeltbuf.c (sparse war

[Peter Hüwe]

Am Freitag, 15. Februar 2013, 17:11:02 schrieb Dan Carpenter:
> Are you using this version of clang?:
> 
> http://git.linuxfoundation.org/llvmlinux.git/
> 
> regards,
> dan carpenter

Hi,

nope - plain vanilla llvm/clang, but I use some of the kernel patches from 
there:
http://git.linuxfoundation.org/?p=llvmlinux.git;a=tree;f=arch/x86_64/patches;hb=master

The main reason for not using llvmlinux is that it builds a lot more than I 
need (e.g. buildbot/qemu) but maybe I should give it a try some time.


Peter