[cfe-commits] [PATCH] review request - strcat modeling addition to CStringChecker

[Lenny Maiorani]

On 04/05/2011 01:29 PM, Ted Kremenek wrote:
> Hi Lenny,
>
> I'm a bit dubious about the following:
>
> +  // ultimately contain both.
> +  if (isAppending) {
> +    // Get the string length of the destination, or give up.
> +    SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
> +    if (dstStrLength.isUndef())
> +      return;
> +
> +    NonLoc *srcStrLengthNL = dyn_cast<NonLoc>(&strLength);
> +    NonLoc *dstStrLengthNL = dyn_cast<NonLoc>(&dstStrLength);
> +
> +    QualType addTy = C.getSValBuilder().getContext().IntTy;
> +
> +    strLength = C.getSValBuilder().evalBinOpNN(state, BO_Add,
> +                                               *srcStrLengthNL, *dstStrLengthNL,
> +                                               addTy);
> +  }
>
> The dyn_cast<>  followed by the unguarded call to 'evalBinOpNN' looks wrong.  There is no guarantee that those values are non-null (which is why I assume you used a dyn_cast<>).  This looks like a potential null dereference.
Ted,

Thanks for reviewing. Turns out there is another bug too. addTy 
shouldn't be IntTy, it should be getSizeType(). This wasn't causing a 
problem on 64-bit OSX, but before you had replied here, I switched 
machines to 64-bit Linux and it did matter. So, that is fixed along with 
your suggestion.

This patch now implements both strcat() and strncat() modeling since 
strncat() was a very small addition.

Please re-review. :)

-Lenny

-------------- next part --------------
A non-text attachment was scrubbed...
Name: strcat-and-strncat-modeling-checker.diff
Type: text/x-patch
Size: 10614 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110406/1f813f07/attachment.bin>