From: Douglas Gregor Date: Tue, 3 May 2011 20:37:33 +0000 (+0000) Subject: Extend -Wnon-pod-memset to also encompass memcpy() and memmove(), X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=06bc9eb9908e42696775b395b290827bde468c8b;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@130786 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7056fbc928..7c4583f7dc 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -261,13 +261,15 @@ def err_builtin_definition : Error<"definition of builtin function %0">; 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>; -def warn_non_pod_memset : Warning< - "destination for this memset call is a pointer to a non-POD type %0">, - InGroup>, 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>; +def warn_non_pod_memaccess : Warning< + "%select{destination for|source of}0 this %1 call is a pointer to non-POD " + "type %2">, + InGroup>, DefaultIgnore; +def note_non_pod_memaccess_silence : Note< "explicitly cast the pointer to silence this warning">; /// main() diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 7d970a8695..d70a67f9fe 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -5552,7 +5552,8 @@ private: 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); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 76f20ce530..10645fd23e 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -318,11 +318,13 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) { 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 @@ static bool isDynamicClassType(QualType T) { /// \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()) { - 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; - - 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*)")); + 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()) { + 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) + << ArgIdx << FnName << PointeeTy + << Call->getCallee()->getSourceRange()); + + SourceRange ArgRange = Call->getArg(0)->getSourceRange(); + DiagRuntimeBehavior( + Dest->getExprLoc(), Dest, + PDiag(diag::note_non_pod_memaccess_silence) + << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)")); + break; + } } } diff --git a/test/SemaCXX/warn-non-pod-memset.cpp b/test/SemaCXX/warn-non-pod-memset.cpp index 6b017f338b..e0abdbd2af 100644 --- a/test/SemaCXX/warn-non-pod-memset.cpp +++ b/test/SemaCXX/warn-non-pod-memset.cpp @@ -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 @@ struct X5 : virtual S1 {} x5; 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}} }