Static analysis at Mozilla

[Sean Kelly]

Steven Schveighoffer Wrote:

> On Thu, 10 Jun 2010 15:55:18 -0400, Sean Kelly <sean at invisibleduck.org>  
> wrote:
> 
> > Ali Çehreli Wrote:
> >
> >> Sean Kelly wrote:
> >>  > bearophile Wrote:
> >>  >
> >>  >> C++ Static Analysis as done on the large Mozilla codebase:
> >>  >> http://blog.ezyang.com/2010/06/static-analysis-mozilla/
> >>  >> It shows that it's important to have a more powerful static
> >> reflection in D. It works well with scoped user-defined attributes too.
> >>  >
> >>  > As much as I like static analysis, it still has a long way to go.
> >> For example, here's some C code that a static analysis tool recently
> >> flagged as broken:
> >>  >
> >>  >     size_t fn( char** pdst, char* src, size_t srclen ) {
> >>  >         __thread static char* dst      = NULL;
> >>  >         __thread static size_t dstcap = 0;
> >>  >         if( dstcap < srclen ) {
> >>  >             dstcap = srclen;
> >>  >             dst      = realloc( dst, dstcap );
> >>  >         }
> >>  >         memcpy( dst, src, srclen ); // Purify: ERROR - uninitialized
> >> write
> >>  >         *pdst = dst;
> >>  >         return srclen;
> >>  >     }
> >>  >
> >>  > Basically, it wasn't smart enough to realize that dst would
> >>  > always be non-NULL when the memcpy occurred, let alone that it
> >>  > would also always be large enough.  For such false positives,
> >>  > it's generally necessary to insert pointless code simply to
> >>  > silence the error, thus complicating the function and
> >>  > increasing the cost of maintenance.  I still believe that the
> >>  > benefits of static analysis vastly outweigh the cost, but I'd
> >>  > love to see more intelligence in branch analysis if nothing
> >>  > else.
> >>
> >> realloc may return NULL. Perhaps they are catching that condition?
> >
> > I suppose so.  Maybe I should change the if statement to a loop and see  
> > what happens.
> 
> What about if srclen is 0?  Won't memcpy then be passed a null pointer via  
> dst?  Does the static analyzer look inside memcpy to see if it uses the  
> pointer when the size is 0?

That's an artifact of my simplified example.  The real code doesn't dereference src if srclen is 0.