WebP does not pass the clang static analyzer

[GoogleCodeExporter]

If you compile WebP using clang and run the analyzer 
(http://clang-analyzer.llvm.org/) over it, you will run into warnings flagged 
by the analyzer. At least one of them (malloc 0 size buffer warning) highlights 
a potential non-portable issue for WebP.

Original issue reported on code.google.com by dmaclach@chromium.org on 18 Jan 2013 at 6:25

[GoogleCodeExporter]

libwebp/v0_2/src/utils/utils.c:37:10: warning: Call to 'calloc' has an 
allocation size of 0 bytes  return calloc((size_t)nmemb, size         ^      
~~~~libwebp/v0_2/src/utils/utils.c:32:10: warning: Call to 'malloc' has an 
allocation size of 0 bytes  return malloc((size_t)(nmemb * size)         ^      
~~~~~~~~~~~~~2 warnings generated.

libwebp/v0_2/src/dec/frame.c:496:3: warning: Value stored to 'mem' is never 
read  mem += alpha_siz  ^      ~~~~~~~~1 warning generated.

libwebp/v0_2/src/utils/quant_levels.c:133:15: warning: Assigned value is 
garbage or undefined      data[n] = map[dat              1 warning generated.

Original comment by dmaclach@chromium.org on 18 Jan 2013 at 6:35

[GoogleCodeExporter]

* libwebp/v0_2/src/utils/utils.c:32:10: warning: Call to 'malloc' has an 
allocation size of 0 bytes  

  not sure i understand this one. malloc(size)'s man says:
       "If size is  0,  then
       malloc()  returns either NULL, or a unique pointer value that can later
       be successfully passed to free()."

* libwebp/v0_2/src/dec/frame.c:496:3: this one is a valid warning. the unused 
'mem += alpha_size' was left over as a reminder...

* libwebp/v0_2/src/utils/quant_levels.c:133:15: this one is pretty wrong, as 
the analyzer doesn't see that min_s and max_s loop bounds span exactly the 
data[n] range. So, no map[] entry can generate an undefined data[n] read.

Original comment by pascal.m...@gmail.com on 21 Jan 2013 at 6:14

[GoogleCodeExporter]

not sure i understand this one. malloc(size)'s man says:
       "If size is  0,  then
       malloc()  returns either NULL, or a unique pointer value that can later
       be successfully passed to free()."

So there is code that calls this that treats a NULL pointer as out of memory 
(e.g. buffer.c:104) so depending on your platform you may get a different 
behavior, which is probably not intended. The warning is telling you that the 
implementation is not strictly defined so it is treating it as a potential 
error condition.

Original comment by dmaclach@chromium.org on 21 Jan 2013 at 8:14

[GoogleCodeExporter]

re: * libwebp/v0_2/src/utils/quant_levels.c:133:15: this one is pretty wrong, 
as the analyzer doesn't see that min_s and max_s loop bounds span exactly the 
data[n] range. So, no map[] entry can generate an undefined data[n] read.

If you added an assert in there to verify that would shut off the clang 
warning, and then just add a definition of NDEBUG to your release builds to 
make sure you don't get a speed hit.

Original comment by dmaclach@chromium.org on 21 Jan 2013 at 8:18

[GoogleCodeExporter]

"So there is code that calls this that treats a NULL pointer as out of memory "

-> that's definitely a good point!
I uploaded https://gerrit.chromium.org/gerrit/#/c/41752/ to address this.

Original comment by pascal.m...@gmail.com on 22 Jan 2013 at 12:27

[GoogleCodeExporter]

https://gerrit.chromium.org/gerrit/#/c/41754/ should address the second warning 
at libwebp/v0_2/src/dec/frame.c:496

Original comment by pascal.m...@gmail.com on 22 Jan 2013 at 12:41

[GoogleCodeExporter]

marking as resolved. Please re-open if there's other static warnings to 
investigate...

Original comment by pascal.m...@gmail.com on 11 Sep 2013 at 7:44

• Changed state: Fixed

[GoogleCodeExporter]

Have we verified this against the new static analyzer in Xcode 5.0?

Original comment by dmacl...@google.com on 11 Sep 2013 at 8:03

[GoogleCodeExporter]

The version of clang [1] we run on build.webmproject.org has some things to say 
@v0.3.1-235-g80b54e1

$ scan-build ./configure
$ scan-build make

I don't currently install scan-build there, but this could be added, it seems 
to be low noise.

What version is Xcode 5.0 shipping with? Any exotic params?

[1] clang version 3.4 (trunk 187307)

Original comment by jz...@google.com on 12 Sep 2013 at 12:05

Attachments:

• v0.3.1-235-g80b54e1-scan-build.tar.gz

[GoogleCodeExporter]

As a sidenote, we should run with the '--enable-everything' option.

$ scan-build ./configure --enable-everything
$ scan-build make

Original comment by urv...@google.com on 12 Sep 2013 at 12:13

[GoogleCodeExporter]

added some more fixes:

https://gerrit.chromium.org/gerrit/#/c/67155

most of the time, it's about adding asserts to provide guidance.

Original comment by pascal.m...@gmail.com on 12 Sep 2013 at 12:04

• Changed state: Started

[GoogleCodeExporter]

All known issues related to this have been addressed.

I've also added a per-commit job to prevent regressions [1]. Note there still 
is one bug reported, but it's spurious and further it isn't being reported 
properly. There's an open bug for the latter [2]. I haven't searched for one 
for the former or tried to create a reduced test case for a new bug report.

[1] http://build.webmproject.org/jenkins/job/libwebp__static_analysis/default/
[2] http://llvm.org/bugs/show_bug.cgi?id=12421

Original comment by jz...@google.com on 18 Dec 2013 at 5:39

• Changed state: Fixed