Hi Lenny,
I think this looks very good, but I have a couple comments:
+ // If the check is for strnlen() then bind the return value to no more than
+ // the maxlen value.
+ SVal maxlenVal;
+ if (IsStrnlen) {
+ const Expr *maxlenExpr = CE->getArg(1);
+ maxlenVal = state->getSVal(maxlenExpr);
+
+ NonLoc * strLengthNL = dyn_cast<NonLoc>(&strLength);
+ NonLoc * maxlenValNL = dyn_cast<NonLoc>(&maxlenVal);
+
+ QualType cmpTy = C.getSValBuilder().getContext().IntTy;
+ const GRState *stateTrue, *stateFalse;
+
+ // Check if the strLength is greater than or equal to the maxlen
+ llvm::tie(stateTrue, stateFalse) =
+ state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_GE,
+ *strLengthNL,
*maxlenValNL,
+ cmpTy)));
+
+ // If the strLength is greater than or equal to the maxlen, set strLength
+ // to maxlen
+ if (stateTrue && !stateFalse) {
+ strLength = maxlenVal;
+ }
+ }
+
Is there a reason 'maxlen' is declared outside if the 'if' block? It's not
used elsewhere. Why not just write:
+ if (IsStrnlen) {
+ const Expr *maxlenExpr = CE->getArg(1);
+ SVal maxlenVal = state->getSVal(maxlenExpr);
+
I strongly prefer this style because it keeps the lifetime of variables
associated with the logic they are intended to be paired with, and no more. By
glancing at the code, I couldn't tell if maxlenVal was use elsewhere unless I
searched for it.
Other minor nits:
+ NonLoc * strLengthNL = dyn_cast<NonLoc>(&strLength);
+ NonLoc * maxlenValNL = dyn_cast<NonLoc>(&maxlenVal);
please write:
NonLoc *strLengthNL
instead of:
NonLoc * strLengthNL
That matches the coding style of the majority of the codebase (which is
gradually moving in this direction).
Other than that, looks great!
On Feb 7, 2011, at 12:06 PM, Lenny Maiorani wrote:
> The attached patch is available for review/inclusion.
>
>
> This patch adds support for checking strnlen() in the Static Analyzer by
> extending and common-izing the strlen() checker.
>
> The checker first computes the length of the string, then compares that to
> the value of the maxlen arg. If the maxlen is less than the length of the
> string the maxlen value becomes the bind value instead of the length of the
> string.
>
> -Lenny
>
> <strnlen-checker.diff>
>
>
> __o
> _`\<,_
> (*)/ (*)
> ~~~~~~~~~~~~~~~~~~~~
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
