Static analysis at Mozilla

div0 div0 at users.sourceforge.net
Thu Jun 10 13:23:00 PDT 2010


On 10/06/2010 20:57, 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?
>
> -Steve

memcpy may get passed a null, but it's never uninitialized.
The analysis is wrong and so is the code. doh!

-- 
My enormous talent is exceeded only by my outrageous laziness.
http://www.ssTk.co.uk


More information about the Digitalmars-d mailing list