[PATCH] [StaticAnalyzer] New checker Sizeof on expression

[Aaron Ballman]

A few thoughts:

Could this be generalized to fit _Alignof and _Generic (the
controlling expression) as well? The behavior is the same for all
three (they're unevaluated).

Also, in terms of sizeof, if the expression modifies a VLA, the
behavior is sadly well-defined (in C). Eg)

size_t f(size_t n) {
  /* n must be incremented */
  size_t a = sizeof(int[++n]);
  return n;
}

This will return n + 1 because it affects the size of the VLA. So we
probably should not warn on that case.

~Aaron

On Thu, Jan 23, 2014 at 9:40 AM, Anders Rönnholm
<Anders.Ronnholm at evidente.se> wrote:
> Hi,
>
> New one with comments handled.
>
> ________________________________________
> Från: Jordan Rose [jordan_rose at apple.com]
> Skickat: den 20 december 2013 19:15
> Till: Anders Rönnholm
> Cc: cfe-commits at cs.uiuc.edu; Daniel Marjamäki; Anna Zaks; David Blaikie; Richard Smith; Matt Calabrese
> Ämne: Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression
>
> On Dec 10, 2013, at 4:38 , Anders Rönnholm <Anders.Ronnholm at evidente.se<mailto:Anders.Ronnholm at evidente.se>> wrote:
>
> Are you OK to commit this patch or do you see more issues?
>
> I'm not sure if anyone else has ideological concerns. There's always a flag to turn this off, I suppose.
>
>
> +  if (S.isSFINAEContext())
> +      return;
>
> Code style: extra indent?
>
>
> +  if(E->HasSideEffects(S.getASTContext()))
> +    return;
>
> sizeof doesn't evaluate its argument, so I'm not sure why you wouldn't want to warn here.
>
>
> +  const FunctionDecl *FD = S.getCurFunctionDecl();
> +  if(FD && FD->isFunctionTemplateSpecialization())
> +    return;
>
> Code style: space after if. (Above too, actually.)
>
>
> +    if (Binop->getLHS()->getType()->isArrayType() ||
> +        Binop->getLHS()->getType()->isAnyPointerType() ||
> +        Binop->getRHS()->getType()->isArrayType() ||
> +        Binop->getRHS()->getType()->isAnyPointerType())
> +      return;
>
> I don't think this is correct...the user is only trying to get ptrdiff_t if both the LHS and RHS are pointer-ish.
>
>
> +def warn_sizeof_bin_op : Warning<
> +  "using sizeof() on an expression with an operator may yield unexpected results">,
> +  InGroup<SizeofOnExpression>;
> +
> +def warn_sizeof_sizeof : Warning<
> +  "using sizeof() on sizeof() may yield unexpected results.">,
> +  InGroup<SizeofOnExpression>;
> +
>
> sizeof doesn't actually require parens, so we shouldn't put the parens in the diagnostics.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>