[cfe-commits] r130786 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/SemaCXX/warn-non-pod-memset.cpp

[Nico Weber]

Hi Doug,

(below)

On Tue, May 3, 2011 at 1:37 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Tue May  3 15:37:33 2011
> New Revision: 130786
>
> URL: http://llvm.org/viewvc/llvm-project?rev=130786&view=rev
> Log:
> Extend -Wnon-pod-memset to also encompass memcpy() and memmove(),
> checking both the source and the destination operands, renaming the
> warning group to -Wnon-pod-memaccess and tweaking the diagnostic text
> in the process.
>
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaChecking.cpp
>    cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=130786&r1=130785&r2=130786&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May  3 15:37:33 2011
> @@ -261,13 +261,15 @@
>  def err_types_compatible_p_in_cplusplus : Error<
>   "__builtin_types_compatible_p is not valid in C++">;
>  def warn_builtin_unknown : Warning<"use of unknown builtin %0">, DefaultError;
> -def warn_dyn_class_memset : Warning<
> -  "destination for this memset call is a pointer to a dynamic class %0">,
> -  InGroup<DiagGroup<"non-pod-memset">>;
> -def warn_non_pod_memset : Warning<
> -  "destination for this memset call is a pointer to a non-POD type %0">,
> -  InGroup<DiagGroup<"non-pod-memset">>, DefaultIgnore;
> -def note_non_pod_memset_silence : Note<
> +def warn_dyn_class_memaccess : Warning<
> +  "%select{destination for|source of}0 this %1 call is a pointer to dynamic "
> +  "class %2; vtable pointer will be overwritten">,
> +  InGroup<DiagGroup<"non-pod-memaccess">>;
> +def warn_non_pod_memaccess : Warning<
> +  "%select{destination for|source of}0 this %1 call is a pointer to non-POD "
> +  "type %2">,
> +  InGroup<DiagGroup<"non-pod-memaccess">>, DefaultIgnore;
> +def note_non_pod_memaccess_silence : Note<
>   "explicitly cast the pointer to silence this warning">;
>
>  /// main()
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=130786&r1=130785&r2=130786&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Tue May  3 15:37:33 2011
> @@ -5552,7 +5552,8 @@
>                                  unsigned format_idx, unsigned firstDataArg,
>                                  bool isPrintf);
>
> -  void CheckMemsetArguments(const CallExpr *Call);
> +  void CheckMemsetcpymoveArguments(const CallExpr *Call,
> +                                   const IdentifierInfo *FnName);
>
>   void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
>                             SourceLocation ReturnLoc);
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=130786&r1=130785&r2=130786&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May  3 15:37:33 2011
> @@ -318,11 +318,13 @@
>                           TheCall->getCallee()->getLocStart());
>   }
>
> -  // Memset handling
> -  if (FnInfo->isStr("memset") &&
> -      FDecl->getLinkage() == ExternalLinkage &&
> -      (!getLangOptions().CPlusPlus || FDecl->isExternC()))
> -    CheckMemsetArguments(TheCall);
> +  // Memset/memcpy/memmove handling
> +  if (FDecl->getLinkage() == ExternalLinkage &&
> +      (!getLangOptions().CPlusPlus || FDecl->isExternC())) {
> +    if (FnInfo->isStr("memset") || FnInfo->isStr("memcpy") ||
> +        FnInfo->isStr("memmove"))
> +      CheckMemsetcpymoveArguments(TheCall, FnInfo);
> +  }
>
>   return false;
>  }
> @@ -1813,45 +1815,51 @@
>  /// \brief Check for dangerous or invalid arguments to memset().
>  ///
>  /// This issues warnings on known problematic or dangerous or unspecified
> -/// arguments to the standard 'memset' function call.
> +/// arguments to the standard 'memset', 'memcpy', and 'memmove' function calls.
>  ///
>  /// \param Call The call expression to diagnose.
> -void Sema::CheckMemsetArguments(const CallExpr *Call) {
> +void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
> +                                       const IdentifierInfo *FnName) {
>   // It is possible to have a non-standard definition of memset.  Validate
>   // we have the proper number of arguments, and if not, abort further
>   // checking.
>   if (Call->getNumArgs() != 3)
>     return;
>
> -  const Expr *Dest = Call->getArg(0)->IgnoreParenImpCasts();
> -
> -  QualType DestTy = Dest->getType();
> -  if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
> -    QualType PointeeTy = DestPtrTy->getPointeeType();
> -    if (PointeeTy->isVoidType())
> -      return;
> -
> -    unsigned DiagID = 0;
> -    // Always complain about dynamic classes.
> -    if (isDynamicClassType(PointeeTy))
> -      DiagID = diag::warn_dyn_class_memset;
> -    // Check the C++11 POD definition regardless of language mode; it is more
> -    // relaxed than earlier definitions and we don't want spurious warnings.
> -    else if (!PointeeTy->isCXX11PODType())
> -      DiagID = diag::warn_non_pod_memset;
> -    else
> -      return;
> +  unsigned LastArg = FnName->isStr("memset")? 1 : 2;
> +  for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) {
> +    const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts();
> +
> +    QualType DestTy = Dest->getType();
> +    if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
> +      QualType PointeeTy = DestPtrTy->getPointeeType();
> +      if (PointeeTy->isVoidType())
> +        continue;
> +
> +      unsigned DiagID = 0;
> +      // Always complain about dynamic classes.
> +      if (isDynamicClassType(PointeeTy))
> +        DiagID = diag::warn_dyn_class_memaccess;
> +      // Check the C++11 POD definition regardless of language mode; it is more
> +      // relaxed than earlier definitions and we don't want spurious warnings.
> +      else if (!PointeeTy->isCXX11PODType())
> +        DiagID = diag::warn_non_pod_memaccess;
> +      else
> +        continue;
>
> -    DiagRuntimeBehavior(
> -      Dest->getExprLoc(), Dest,
> -      PDiag(DiagID)
> -        << PointeeTy << Call->getCallee()->getSourceRange());
> -
> -    SourceRange ArgRange = Call->getArg(0)->getSourceRange();
> -    DiagRuntimeBehavior(
> -      Dest->getExprLoc(), Dest,
> -      PDiag(diag::note_non_pod_memset_silence)
> -        << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
> +      DiagRuntimeBehavior(
> +        Dest->getExprLoc(), Dest,
> +        PDiag(DiagID)
> +          << ArgIdx << FnName << PointeeTy
> +          << Call->getCallee()->getSourceRange());
> +
> +      SourceRange ArgRange = Call->getArg(0)->getSourceRange();

^ Should this be getArg(ArgIdx)?

> +      DiagRuntimeBehavior(
> +        Dest->getExprLoc(), Dest,
> +        PDiag(diag::note_non_pod_memaccess_silence)
> +          << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
> +      break;
> +    }
>   }
>  }
>
>
> Modified: cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp?rev=130786&r1=130785&r2=130786&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp Tue May  3 15:37:33 2011
> @@ -1,6 +1,8 @@
> -// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memset -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memaccess -verify %s
>
>  extern "C" void *memset(void *, int, unsigned);
> +extern "C" void *memmove(void *s1, const void *s2, unsigned n);
> +extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
>
>  // Several POD types that should not warn.
>  struct S1 {} s1;
> @@ -24,19 +26,32 @@
>
>  void test_warn() {
>   memset(&x1, 0, sizeof x1); // \
> -      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> +      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
>       // expected-note {{explicitly cast the pointer to silence this warning}}
>   memset(&x2, 0, sizeof x2); // \
> -      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> +      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
>       // expected-note {{explicitly cast the pointer to silence this warning}}
>   memset(&x3, 0, sizeof x3); // \
> -      // expected-warning {{destination for this memset call is a pointer to a dynamic class}} \
> +      // expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
>       // expected-note {{explicitly cast the pointer to silence this warning}}
>   memset(&x4, 0, sizeof x4); // \
> -      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
> +      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
>       // expected-note {{explicitly cast the pointer to silence this warning}}
>   memset(&x5, 0, sizeof x5); // \
> -      // expected-warning {{destination for this memset call is a pointer to a dynamic class}} \
> +      // expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
> +      // expected-note {{explicitly cast the pointer to silence this warning}}
> +
> +  memmove(&x1, 0, sizeof x1); // \
> +      // expected-warning{{destination for this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
> +      // expected-note {{explicitly cast the pointer to silence this warning}}
> +  memmove(0, &x1, sizeof x1); // \
> +      // expected-warning{{source of this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
> +      // expected-note {{explicitly cast the pointer to silence this warning}}
> +  memcpy(&x1, 0, sizeof x1); // \
> +      // expected-warning{{destination for this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
> +      // expected-note {{explicitly cast the pointer to silence this warning}}
> +  memcpy(0, &x1, sizeof x1); // \
> +      // expected-warning{{source of this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
>       // expected-note {{explicitly cast the pointer to silence this warning}}
>  }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>