[cfe-commits] r142157 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaType.cpp test/Sema/static-array.c

[Douglas Gregor]

On Oct 16, 2011, at 2:17 PM, Peter Collingbourne wrote:

> Author: pcc
> Date: Sun Oct 16 16:17:32 2011
> New Revision: 142157
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=142157&view=rev
> Log:
> Add sema checks for calls to functions taking static array parameters
> 
> Added:
>    cfe/trunk/test/Sema/static-array.c
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaChecking.cpp
>    cfe/trunk/lib/Sema/SemaType.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=142157&r1=142156&r2=142157&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Oct 16 16:17:32 2011
> @@ -2716,6 +2716,9 @@
>   "zero size arrays are an extension">;
> def err_typecheck_zero_array_size : Error<
>   "zero-length arrays are not permitted in C++">;
> +def warn_typecheck_zero_static_array_size : Warning<
> +  "'static' has no effect on zero-length arrays">,
> +  InGroup<DiagGroup<"array-bounds">>;
> def err_at_least_one_initializer_needed_to_size_array : Error<
>   "at least one initializer value required to size array">;
> def err_array_size_non_int : Error<"size of array has non-integer type %0">;
> @@ -4509,6 +4512,11 @@
> def warn_null_arg : Warning<
>   "null passed to a callee which requires a non-null argument">,
>   InGroup<NonNull>;
> +def warn_static_array_too_small : Warning<
> +  "array argument is too small; contains %0 elements, callee requires at least %1">,
> +  InGroup<DiagGroup<"array-bounds">>;
> +def note_callee_static_array : Note<
> +  "callee declares array parameter as static here">;
> def warn_empty_format_string : Warning<
>   "format string is empty">, InGroup<FormatZeroLength>;
> def warn_format_string_is_wide_literal : Warning<
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=142157&r1=142156&r2=142157&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sun Oct 16 16:17:32 2011
> @@ -6103,6 +6103,10 @@
>                              const Expr * const *ExprArgs,
>                              SourceLocation CallSiteLoc);
> 
> +  void CheckStaticArrayArguments(const FunctionDecl *FDecl,
> +                                 const Expr * const *ExprArgs,
> +                                 SourceLocation CallSiteLoc);
> +
>   void CheckPrintfScanfArguments(const CallExpr *TheCall, bool HasVAListArg,
>                                  unsigned format_idx, unsigned firstDataArg,
>                                  bool isPrintf);
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=142157&r1=142156&r2=142157&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Oct 16 16:17:32 2011
> @@ -360,6 +360,9 @@
>                           TheCall->getCallee()->getLocStart());
>   }
> 
> +  CheckStaticArrayArguments(FDecl, TheCall->getArgs(),
> +                            TheCall->getCallee()->getLocStart());
> +
>   // Builtin handling
>   int CMF = -1;
>   switch (FDecl->getBuiltinID()) {
> @@ -1362,6 +1365,64 @@
>   }
> }
> 
> +static void DiagnoseCalleeStaticArrayParam(Sema &S, ParmVarDecl *PVD) {
> +  TypeLoc TL = PVD->getTypeSourceInfo()->getTypeLoc();
> +  if (ArrayTypeLoc *ATL = dyn_cast<ArrayTypeLoc>(&TL))
> +    S.Diag(PVD->getLocation(), diag::note_callee_static_array)
> +      << ATL->getLocalSourceRange();
> +}
> +
> +/// CheckStaticArrayArguments - Check that each argument corresponding to a
> +/// static array parameter is non-null, and that if it is formed by
> +/// array-to-pointer decay, the underlying array is sufficiently large.
> +///
> +/// C99 6.7.5.3p7: If the keyword static also appears within the [ and ] of the
> +/// array type derivation, then for each call to the function, the value of the
> +/// corresponding actual argument shall provide access to the first element of
> +/// an array with at least as many elements as specified by the size expression.
> +void
> +Sema::CheckStaticArrayArguments(const FunctionDecl *FDecl,
> +                                const Expr * const *ExprArgs,
> +                                SourceLocation CallSiteLoc) {
> +  // Static array parameters are not supported in C++.
> +  if (getLangOptions().CPlusPlus)
> +    return;
> +
> +  for (FunctionDecl::param_const_iterator i = FDecl->param_begin(),
> +       e = FDecl->param_end(); i != e; ++i, ++ExprArgs) {
> +    const Expr *ArgExpr = *ExprArgs;
> +    QualType OrigTy = (*i)->getOriginalType();
> +
> +    const ArrayType *AT = Context.getAsArrayType(OrigTy);
> +    if (!AT || AT->getSizeModifier() != ArrayType::Static)
> +      continue;
> +
> +    if (ArgExpr->isNullPointerConstant(Context,
> +                                       Expr::NPC_NeverValueDependent)) {
> +      Diag(CallSiteLoc, diag::warn_null_arg) << ArgExpr->getSourceRange();
> +      DiagnoseCalleeStaticArrayParam(*this, *i);
> +      continue;
> +    }
> +
> +    const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(AT);
> +    if (!CAT)
> +      continue;
> +
> +    const ConstantArrayType *ArgCAT =
> +      Context.getAsConstantArrayType(ArgExpr->IgnoreParenImpCasts()->getType());
> +    if (!ArgCAT)
> +      continue;
> +
> +    if (ArgCAT->getSize().ult(CAT->getSize())) {
> +      Diag(CallSiteLoc, diag::warn_static_array_too_small)
> +        << ArgExpr->getSourceRange()
> +        << (unsigned) ArgCAT->getSize().getZExtValue()
> +        << (unsigned) CAT->getSize().getZExtValue();
> +      DiagnoseCalleeStaticArrayParam(*this, *i);
> +    }
> +  }
> +}
> +

Why is this implemented as a separate pass over the parameter/argument pairs? It should be implemented as part of Sema::GatherArgumentsForCall, when we're already inspecting the parameter and argument.

	- Doug