[Qemu-devel] Static analysis using clang on the x86_64 target

[Qemu-devel] Static analysis using clang on the x86_64 target

[Amit Shah]

Hello,

Here's a run of the clang analyzer on qemu sources for the x86_64
target.

See

http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/

for the results.

There are a few results there which look dubious but a lot of the output
can be useful to fix the bugs.

What's nice about the tool is that the output is the source code
annotated with the branch decisions that were taken to point out to the
case where a bug would be triggered.

Doing this for all the targets takes a really long time plus lots of
disk space (I stopped the compile at 400M of clang output).

If there's interest in this kind of result, I can post a link to the
list every week or so. However, some bugs reported make it slightly less
appealing as real bugs could get lost in the noise.

For information on the clang static analyzer, see

http://clang-analyzer.llvm.org/

		Amit

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[malc]

On Tue, 12 Jan 2010, Amit Shah wrote:

> Hello,
> 
> Here's a run of the clang analyzer on qemu sources for the x86_64
> target.
> 
> See
> 
> http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/
> 
> for the results.
> 
> There are a few results there which look dubious but a lot of the output
> can be useful to fix the bugs.
> 
> What's nice about the tool is that the output is the source code
> annotated with the branch decisions that were taken to point out to the
> case where a bug would be triggered.
> 
> Doing this for all the targets takes a really long time plus lots of
> disk space (I stopped the compile at 400M of clang output).
> 
> If there's interest in this kind of result, I can post a link to the
> list every week or so. However, some bugs reported make it slightly less
> appealing as real bugs could get lost in the noise.
> 
> For information on the clang static analyzer, see
> 
> http://clang-analyzer.llvm.org/

Both audio.c issues are wrong, dma/sb16 is of useless operation variety
nevertheless the "fix" is pushed, hopefuly clang shouldn't complain about
them anymore.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Amit Shah]

On (Tue) Jan 12 2010 [22:01:15], malc wrote:
> 
> Both audio.c issues are wrong, dma/sb16 is of useless operation variety
> nevertheless the "fix" is pushed, hopefuly clang shouldn't complain about
> them anymore.

Yes, the count went down to 95 from 98 after your commits.

There are a few false positives; I think clang is right in reporting
them and also that we can't really elegantly make those reports
disappear.

		Amit

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Blue Swirl]

On Tue, Jan 12, 2010 at 6:13 PM, Amit Shah <amit.shah@redhat.com> wrote:
> Hello,
>
> Here's a run of the clang analyzer on qemu sources for the x86_64
> target.
>
> See
>
> http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/
>
> for the results.
>
> There are a few results there which look dubious but a lot of the output
> can be useful to fix the bugs.
>
> What's nice about the tool is that the output is the source code
> annotated with the branch decisions that were taken to point out to the
> case where a bug would be triggered.
>
> Doing this for all the targets takes a really long time plus lots of
> disk space (I stopped the compile at 400M of clang output).
>
> If there's interest in this kind of result, I can post a link to the
> list every week or so. However, some bugs reported make it slightly less
> appealing as real bugs could get lost in the noise.

I'd be very interested in the results of Sparc32 and Sparc64 analyses.

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Amit Shah]

On (Tue) Jan 12 2010 [19:35:08], Blue Swirl wrote:
> On Tue, Jan 12, 2010 at 6:13 PM, Amit Shah <amit.shah@redhat.com> wrote:
> > Hello,
> >
> > Here's a run of the clang analyzer on qemu sources for the x86_64
> > target.
> >
> > See
> >
> > http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/
> >
> > for the results.
> >
> > There are a few results there which look dubious but a lot of the output
> > can be useful to fix the bugs.
> >
> > What's nice about the tool is that the output is the source code
> > annotated with the branch decisions that were taken to point out to the
> > case where a bug would be triggered.
> >
> > Doing this for all the targets takes a really long time plus lots of
> > disk space (I stopped the compile at 400M of clang output).
> >
> > If there's interest in this kind of result, I can post a link to the
> > list every week or so. However, some bugs reported make it slightly less
> > appealing as real bugs could get lost in the noise.
> 
> I'd be very interested in the results of Sparc32 and Sparc64 analyses.

OK, I added the two targets to the run and got the following result:

http://amitshah.fedorapeople.org/clang-output/2010-01-13-1/

The bug count went up from 95 for just x86-64 to 131.

However, a lot of these are dups as files get recompiled for each
target.

		Amit

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Blue Swirl]

On Wed, Jan 13, 2010 at 7:02 AM, Amit Shah <amit.shah@redhat.com> wrote:
> On (Tue) Jan 12 2010 [19:35:08], Blue Swirl wrote:
>> On Tue, Jan 12, 2010 at 6:13 PM, Amit Shah <amit.shah@redhat.com> wrote:
>> > Hello,
>> >
>> > Here's a run of the clang analyzer on qemu sources for the x86_64
>> > target.
>> >
>> > See
>> >
>> > http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/
>> >
>> > for the results.
>> >
>> > There are a few results there which look dubious but a lot of the output
>> > can be useful to fix the bugs.
>> >
>> > What's nice about the tool is that the output is the source code
>> > annotated with the branch decisions that were taken to point out to the
>> > case where a bug would be triggered.
>> >
>> > Doing this for all the targets takes a really long time plus lots of
>> > disk space (I stopped the compile at 400M of clang output).
>> >
>> > If there's interest in this kind of result, I can post a link to the
>> > list every week or so. However, some bugs reported make it slightly less
>> > appealing as real bugs could get lost in the noise.
>>
>> I'd be very interested in the results of Sparc32 and Sparc64 analyses.
>
> OK, I added the two targets to the run and got the following result:
>
> http://amitshah.fedorapeople.org/clang-output/2010-01-13-1/
>
> The bug count went up from 95 for just x86-64 to 131.
>
> However, a lot of these are dups as files get recompiled for each
> target.

Thanks. I fixed the warnings related to Sparc32. Were there really no
new warnings for Sparc64?

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Amit Shah]

On (Wed) Jan 13 2010 [19:08:11], Blue Swirl wrote:
> 
> Thanks. I fixed the warnings related to Sparc32. Were there really no
> new warnings for Sparc64?

Looks like it; vl.c gets reported three times at the same locations so 3
arches have been compiled.

My test machine is down ATM, I can confirm later when it's up.

BTW for the patch

commit 884a0c7677cf8431d2a632673914994c2e01673d

    pcnet: remove dead nested assignment, spotted by clang
    
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 91d106d..44b5b31 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1608,7 +1608,7 @@ static void pcnet_aprom_writeb(void *opaque,
uint32_t addr, uint32_t val)
 static uint32_t pcnet_aprom_readb(void *opaque, uint32_t addr)
 {
     PCNetState *s = opaque;
-    uint32_t val = s->prom[addr &= 15];
+    uint32_t val = s->prom[addr & 15];
 #ifdef PCNET_DEBUG
     printf("pcnet_aprom_readb addr=0x%08x val=0x%02x\n", addr, val);
 #endif


if debugging is enabled, addr will now print a different value than
earlier.

		Amit

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Blue Swirl]

On Thu, Jan 14, 2010 at 3:09 AM, Amit Shah <amit.shah@redhat.com> wrote:
> On (Wed) Jan 13 2010 [19:08:11], Blue Swirl wrote:
>>
>> Thanks. I fixed the warnings related to Sparc32. Were there really no
>> new warnings for Sparc64?
>
> Looks like it; vl.c gets reported three times at the same locations so 3
> arches have been compiled.
>
> My test machine is down ATM, I can confirm later when it's up.
>
> BTW for the patch
>
> commit 884a0c7677cf8431d2a632673914994c2e01673d
>
>    pcnet: remove dead nested assignment, spotted by clang
>
> diff --git a/hw/pcnet.c b/hw/pcnet.c
> index 91d106d..44b5b31 100644
> --- a/hw/pcnet.c
> +++ b/hw/pcnet.c
> @@ -1608,7 +1608,7 @@ static void pcnet_aprom_writeb(void *opaque,
> uint32_t addr, uint32_t val)
>  static uint32_t pcnet_aprom_readb(void *opaque, uint32_t addr)
>  {
>     PCNetState *s = opaque;
> -    uint32_t val = s->prom[addr &= 15];
> +    uint32_t val = s->prom[addr & 15];
>  #ifdef PCNET_DEBUG
>     printf("pcnet_aprom_readb addr=0x%08x val=0x%02x\n", addr, val);
>  #endif
>
>
> if debugging is enabled, addr will now print a different value than
> earlier.

It should be harmless, pcnet_aprom_readb doesn't do addr & 15 either.
It's probably a relic from the time when the devices were passed the
MMIO address instead of offset.

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

[Amit Shah]

On (Wed) Jan 13 2010 [12:32:54], Amit Shah wrote:
> > 
> > I'd be very interested in the results of Sparc32 and Sparc64 analyses.
> 
> OK, I added the two targets to the run and got the following result:
> 
> http://amitshah.fedorapeople.org/clang-output/2010-01-13-1/
> 
> The bug count went up from 95 for just x86-64 to 131.

The count currently is at 107:

http://amitshah.fedorapeople.org/clang-output/2010-01-21-1/

A few new ones have been introduced.

		Amit