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

[Douglas Gregor]

On Apr 27, 2011, at 12:05 AM, Chandler Carruth wrote:

> Author: chandlerc
> Date: Wed Apr 27 02:05:31 2011
> New Revision: 130299
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=130299&view=rev
> Log:
> Add a warning (-Wnon-pod-memset) for calls to memset() with
> a destination pointer that points to a non-POD type. This can flag such
> horrible bugs as overwriting vptrs when a previously POD structure is
> suddenly given a virtual method, or creating objects that crash on
> practically any use by zero-ing out a member when its changed from
> a const char* to a std::string, etc.

Are you planning to do the same for pointers-to-non-POD passed to other memory-manipulation functions (memcpy, memmove) as well? If so, the warning option name should be generalized.

> Added:
>    cfe/trunk/test/SemaCXX/warn-non-pod-memset.cpp
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaChecking.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 27 02:05:31 2011
> @@ -261,6 +261,11 @@
> 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_non_pod_memset : Warning<
> +  "destination for this memset call is a pointer to a non-POD type %0">,
> +  InGroup<DiagGroup<"non-pod-memset">>;
> +def note_non_pod_memset_silence : Note<
> +  "explicitly cast the pointer to silence this warning">;
> 
> /// main()
> // static/inline main() are not errors in C, just in C++.
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 27 02:05:31 2011
> @@ -5516,6 +5516,8 @@
>                                  unsigned format_idx, unsigned firstDataArg,
>                                  bool isPrintf);
> 
> +  void CheckMemsetArguments(const CallExpr *Call);
> +
>   void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
>                             SourceLocation ReturnLoc);
>   void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex);
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=130299&r1=130298&r2=130299&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Apr 27 02:05:31 2011
> @@ -318,6 +318,10 @@
>                           TheCall->getCallee()->getLocStart());
>   }
> 
> +  // Memset handling
> +  if (FnInfo->isStr("memset"))
> +    CheckMemsetArguments(TheCall);
> +
>   return false;
> }

Why not check if this is the "memset" library builtin via Builtin::Context?

> @@ -1791,6 +1795,36 @@
>   }
> }
> 
> +//===--- CHECK: Standard memory functions ---------------------------------===//
> +
> +/// \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.
> +///
> +/// \param Call The call expression to diagnose.
> +void Sema::CheckMemsetArguments(const CallExpr *Call) {
> +  assert(Call->getNumArgs() == 3 && "Unexpected number of arguments to memset");
> +  const Expr *Dest = Call->getArg(0)->IgnoreParenImpCasts();
> +
> +  QualType DestTy = Dest->getType();
> +  if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
> +    QualType PointeeTy = DestPtrTy->getPointeeType();
> +    if (!PointeeTy->isPODType()) {
> +      DiagRuntimeBehavior(
> +        Dest->getExprLoc(), Dest,
> +        PDiag(diag::warn_non_pod_memset)
> +          << 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*)"));
> +    }
> +  }
> +}


Looks great!

	- Doug