[PATCH] [StaticAnalyzer] New checker Sizeof on expression

[Anders Rönnholm]

<<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).
<<
I wonder if we could wait with this.

<<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.
<<
Good because this checker does not warn in this 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
>