From ac6872655bc64ac54c05dbf47e2e790b2b1bca9c Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Fri, 3 Jun 2011 06:23:57 +0000 Subject: [PATCH] Clean up the "non-POD memaccess" stuff some. This adds a properly named diagnostic group to cover the cases where we have definitively bad behavior: dynamic classes. It also rips out the existing support for POD-based checking. This didn't work well, and triggered too many false positives. I'm looking into a possibly more principled way to warn on the fundamental buggy construct here. POD-ness isn't the critical aspect anyways, so a clean slate is better. This also removes some silliness from the code until the new checks arrive. llvm-svn: 132534 --- .../clang/Basic/DiagnosticSemaKinds.td | 8 ++--- clang/lib/Sema/SemaChecking.cpp | 26 ++++++--------- ...-pod-memset.cpp => warn-bad-memaccess.cpp} | 33 +++++-------------- 3 files changed, 21 insertions(+), 46 deletions(-) rename clang/test/SemaCXX/{warn-non-pod-memset.cpp => warn-bad-memaccess.cpp} (64%) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8fb4eaa166cf..33c3445d51dc 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -264,12 +264,8 @@ def warn_builtin_unknown : Warning<"use of unknown builtin %0">, DefaultError; 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< + InGroup>; +def note_bad_memaccess_silence : Note< "explicitly cast the pointer to silence this warning">; /// main() diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 7430cfb119e3..05c257ab8332 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1814,7 +1814,7 @@ static bool isDynamicClassType(QualType T) { /// \brief Check for dangerous or invalid arguments to memset(). /// -/// This issues warnings on known problematic or dangerous or unspecified +/// This issues warnings on known problematic, dangerous or unspecified /// arguments to the standard 'memset', 'memcpy', and 'memmove' function calls. /// /// \param Call The call expression to diagnose. @@ -1836,27 +1836,21 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call, 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 + if (isDynamicClassType(PointeeTy)) { + DiagRuntimeBehavior( + Dest->getExprLoc(), Dest, + PDiag(diag::warn_dyn_class_memaccess) + << ArgIdx << FnName << PointeeTy + << Call->getCallee()->getSourceRange()); + } 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) + PDiag(diag::note_bad_memaccess_silence) << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)")); break; } diff --git a/clang/test/SemaCXX/warn-non-pod-memset.cpp b/clang/test/SemaCXX/warn-bad-memaccess.cpp similarity index 64% rename from clang/test/SemaCXX/warn-non-pod-memset.cpp rename to clang/test/SemaCXX/warn-bad-memaccess.cpp index e0abdbd2aff2..e7d095f0edce 100644 --- a/clang/test/SemaCXX/warn-non-pod-memset.cpp +++ b/clang/test/SemaCXX/warn-bad-memaccess.cpp @@ -1,57 +1,42 @@ -// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memaccess -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wdynamic-class-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. +// Several types that should not warn. struct S1 {} s1; struct S2 { int x; } s2; struct S3 { float x, y; S1 s[4]; void (*f)(S1**); } s3; -// We use the C++11 concept of POD for this warning, so ensure a non-aggregate -// still warns. class C1 { int x, y, z; public: void foo() {} } c1; -// Non-POD types that should warn. -struct X1 { X1(); } x1; -struct X2 { ~X2(); } x2; -struct X3 { virtual void f(); } x3; -struct X4 : X2 {} x4; -struct X5 : virtual S1 {} x5; +struct X1 { virtual void f(); } x1; +struct X2 : virtual S1 {} x2; void test_warn() { memset(&x1, 0, sizeof x1); // \ - // 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 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 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 non-POD type}} \ - // expected-note {{explicitly cast the pointer to silence this warning}} - memset(&x5, 0, sizeof x5); // \ + memset(&x2, 0, sizeof x2); // \ // 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-warning{{destination for this 'memmove' call is a pointer to dynamic class}} \ // 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-warning{{source of this 'memmove' call is a pointer to dynamic class}} \ // 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-warning{{destination for this 'memcpy' call is a pointer to dynamic class}} \ // 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-warning{{source of this 'memcpy' call is a pointer to dynamic class}} \ // expected-note {{explicitly cast the pointer to silence this warning}} }