Hi Lenny,
I think this looks good. One high-level comment I have is that should we plan
on modeling (eventually, in a later patch) the actually copy as well? For
strcat(), you return the new size of the combined string, but it would be great
if we actually modeled that the new string was indeed a combination of the two
other strings. To model this efficiently, we will likely need to introduce a
new SVal that represents the concatenation (so we don't literally copy the
values of strings, and also have the ability to treat string concatenations
symbolically).
Ted
On Apr 6, 2011, at 1:32 PM, Lenny Maiorani wrote:
> 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
>
> <strcat-and-strncat-modeling-checker.diff>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
