[PATCH] [StaticAnalyzer] New checker StringPlusChar

[Richard Smith]

This should be a warning, not a static analyser check, shouldn't it?
On 1 Oct 2013 09:34, "Jordan Rose" <jordan_rose at apple.com> wrote:

> The categories are supposed to be very broad groupings: "Logic error",
> "Memory leak", "Convention X". "Logic error" is probably fine here, even
> though it's generic, since there's not really a group that fits this
> general kind of mistake. I just want to avoid magic strings—we shouldn't
> have variants of the same thing because they'll show up as different
> categories.
>
> Comments on the patch:
>
> +    // Only check add operators.
> +    if (B->getOpcode() != BO_Add)
>
> How about +=?
>
>
> +    const CharacterLiteral *CharExpr =
> +        dyn_cast<CharacterLiteral>(B->getRHS()->IgnoreImpCasts());
>
> Should this be a character literal, or just any expression with "char"
> type? I realize that someone could be using a char-sized variable as an
> index, but that seems fairly rare, and harder to catch by hand.
>
>
> +    if (!(StringRefExpr->getType() ==
> +
>  Context.getPointerType(Context.getConstType(Context.CharTy)) ||
> +          StringRefExpr->getType() ==
> Context.getPointerType(Context.CharTy)))
>
> Things to consider:
> 1. Do explicitly signed or unsigned chars count? In that case, you should
> use Type::isCharType().
> 2. Do wide chars count? Then you should use Type::isAnyCharacterType().
> 3. Even if none of the above, you should consider typedefs, so it's still
> worth going through getCanonicalType() and getPointeeType() rather than
> building up a new pointer type.
>
> One possibility:
> StringRefExpr->getType()->getPointeeType()->isAnyCharacterType(). At this
> point, though, StringRefExpr->getType() probably deserves a helper variable.
>
>
> +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.StringPlusChar
> -verify %s
>
> Please include the "core" package in these checks; running without core
> enabled isn't a use case we're interested in.
>
> Jordan
>
>
> On Sep 30, 2013, at 3:25 , Anders Rönnholm <Anders.Ronnholm at evidente.se>
> wrote:
>
> Hi Jordan,
>
> Thank you for your comments. I have changed the check to use
> RecursiveASTVisitor. However i'm not sure what i should write as category
> in bugreport. You mentioned adding a new category in CommonBugCategories
> for SIzeofOnExpression, should i do that or is what i have enough for now?
>
>
> I have provided a new diff.
>
> //Anders
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131001/e418306b/attachment.html>