[cfe-commits] [PATCH] review request: mempcpy() checker implementation for Static Analyzer

[Lenny Maiorani]

On 03/30/2011 03:29 PM, Ted Kremenek wrote:
> On Mar 30, 2011, at 12:06 PM, Lenny Maiorani wrote:
>
>> Please review attached implementation of mempcpy() checker (and commit if acceptable).
>>
>> -Lenny
>>
>> <mempcpy-checker.diff>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> Hi Lenny,
>
> Minor nits:
>
> (1) You must use spaces instead of tabs.
>
> (2) For comments:
>
> +	// get the length to copy
>
> please style them as follows to follow LLVM conventions:
>
>         // Get the length to copy.
>
> Other than that, the main logic point I don't understand is the following code:
>
> +  // get the value of the Dest
> +  SVal destVal = state->getSVal(Dest);
> +
> +  // ensure the destination is not null
> +  state = checkNonNull(C, state, Dest, destVal);
> +  if (!state)
> +    return;
> +
> +  // get the value of the Src
> +  SVal srcVal = state->getSVal(Source);
> +
> +  // ensure the source is not null
> +  state = checkNonNull(C, state, Source, srcVal);
> +  if (!state)
> +    return;
> +
>
> If either the 'Src' or 'Dest' is null, what are the right semantics?  Does mempcpy do nothing?  Is the return value null?  We should probably model that.
>
> Cheers,
> Ted
>
>
>
Ok, I will make those changes. Not sure how my emacs settings got 
changed, but...

As for the modeling of mempcpy() (and memcpy() for that matter): the 
behavior is that if NULL is passed as either the src or dst there will 
be a NULL pointer dereference unless the number of bytes to copy is 0. I 
am re-working the flow through evalCopyCommon to accurately reflect that.

-Lenny