Extend -Wdynamic-class-memaccess to records containing dynamic classes

Reviewers: rtrieu

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D4325

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@211972 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 58 insertions, 20 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 6f671f4d39..72a2533f54 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -411,8 +411,8 @@ def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
   InGroup<ImplicitFunctionDeclare>, DefaultError;
 def warn_dyn_class_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this "
-  "%1 call is a pointer to dynamic class %2; vtable pointer will be "
-  "%select{overwritten|copied|moved|compared}3">,
+  "%1 call is a pointer to %select{|class containing a }2dynamic class %3; "
+  "vtable pointer will be %select{overwritten|copied|moved|compared}4">,
   InGroup<DiagGroup<"dynamic-class-memaccess">>;
 def note_bad_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index adc4c4a04b..72d3b2761d 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -3983,15 +3983,35 @@ static bool CheckMemorySizeofForComparison(Sema &S, const Expr *E,
   return true;
 }
 
-/// \brief Determine whether the given type is a dynamic class type (e.g.,
-/// whether it has a vtable).
-static bool isDynamicClassType(QualType T) {
-  if (CXXRecordDecl *Record = T->getAsCXXRecordDecl())
-    if (CXXRecordDecl *Definition = Record->getDefinition())
-      if (Definition->isDynamicClass())
-        return true;
-  
-  return false;
+/// \brief Determine whether the given type is or contains a dynamic class type
+/// (e.g., whether it has a vtable).
+static const CXXRecordDecl *getContainedDynamicClass(QualType T,
+                                                     bool &IsContained) {
+  // Look through array types while ignoring qualifiers.
+  const Type *Ty = T->getBaseElementTypeUnsafe();
+  IsContained = false;
+
+  const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
+  RD = RD ? RD->getDefinition() : nullptr;
+  if (!RD)
+    return nullptr;
+
+  if (RD->isDynamicClass())
+    return RD;
+
+  // Check all the fields.  If any bases were dynamic, the class is dynamic.
+  // It's impossible for a class to transitively contain itself by value, so
+  // infinite recursion is impossible.
+  for (auto *FD : RD->fields()) {
+    bool SubContained;
+    if (const CXXRecordDecl *ContainedRD =
+            getContainedDynamicClass(FD->getType(), SubContained)) {
+      IsContained = true;
+      return ContainedRD;
+    }
+  }
+
+  return nullptr;
 }
 
 /// \brief If E is a sizeof expression, returns its argument expression,
@@ -4135,7 +4155,9 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
       }
 
       // Always complain about dynamic classes.
-      if (isDynamicClassType(PointeeTy)) {
+      bool IsContained;
+      if (const CXXRecordDecl *ContainedRD =
+              getContainedDynamicClass(PointeeTy, IsContained)) {
 
         unsigned OperationType = 0;
         // "overwritten" if we're warning about the destination for any call
@@ -4153,8 +4175,7 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
           Dest->getExprLoc(), Dest,
           PDiag(diag::warn_dyn_class_memaccess)
             << (BId == Builtin::BImemcmp ? ArgIdx + 2 : ArgIdx)
-            << FnName << PointeeTy
-            << OperationType
+            << FnName << IsContained << ContainedRD << OperationType
             << Call->getCallee()->getSourceRange());
       } else if (PointeeTy.hasNonTrivialObjCLifetime() &&
                BId != Builtin::BImemset)
diff --git a/test/SemaCXX/warn-bad-memaccess.cpp b/test/SemaCXX/warn-bad-memaccess.cpp
index 7a7459acee..e86610ab64 100644
--- a/test/SemaCXX/warn-bad-memaccess.cpp
+++ b/test/SemaCXX/warn-bad-memaccess.cpp
@@ -24,6 +24,11 @@ public:
 struct X1 { virtual void f(); } x1;
 struct X2 : virtual S1 {} x2;
 
+struct ContainsDynamic { X1 dynamic; } contains_dynamic;
+struct DeepContainsDynamic { ContainsDynamic m; } deep_contains_dynamic;
+struct ContainsArrayDynamic { X1 dynamic[1]; } contains_array_dynamic;
+struct ContainsPointerDynamic { X1 *dynamic; } contains_pointer_dynamic;
+
 void test_warn() {
   memset(&x1, 0, sizeof x1); // \
       // expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
@@ -33,22 +38,22 @@ void test_warn() {
       // 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 dynamic class 'struct X1'; vtable pointer will be overwritten}} \
+      // expected-warning{{destination for this 'memmove' call is a pointer to dynamic class 'X1'; vtable pointer will be overwritten}} \
       // 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 dynamic class 'struct X1'; vtable pointer will be moved}} \
+      // expected-warning{{source of this 'memmove' call is a pointer to dynamic class 'X1'; vtable pointer will be moved}} \
       // 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 dynamic class 'struct X1'; vtable pointer will be overwritten}} \
+      // expected-warning{{destination for this 'memcpy' call is a pointer to dynamic class 'X1'; vtable pointer will be overwritten}} \
       // 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 dynamic class 'struct X1'; vtable pointer will be copied}} \
+      // expected-warning{{source of this 'memcpy' call is a pointer to dynamic class 'X1'; vtable pointer will be copied}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memcmp(&x1, 0, sizeof x1); // \
-      // expected-warning{{first operand of this 'memcmp' call is a pointer to dynamic class 'struct X1'; vtable pointer will be compared}} \
+      // expected-warning{{first operand of this 'memcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memcmp(0, &x1, sizeof x1); // \
-      // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'struct X1'; vtable pointer will be compared}} \
+      // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
 
   __builtin_memset(&x1, 0, sizeof x1); // \
@@ -90,6 +95,16 @@ void test_warn() {
   __builtin___memcpy_chk(0, &x1, sizeof x1, sizeof x1); //                    \
       // expected-warning{{source of this '__builtin___memcpy_chk' call is a pointer to dynamic class}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
+
+  // expected-warning@+2 {{destination for this 'memset' call is a pointer to class containing a dynamic class 'X1'}}
+  // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
+  memset(&contains_dynamic, 0, sizeof(contains_dynamic));
+  // expected-warning@+2 {{destination for this 'memset' call is a pointer to class containing a dynamic class 'X1'}}
+  // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
+  memset(&deep_contains_dynamic, 0, sizeof(deep_contains_dynamic));
+  // expected-warning@+2 {{destination for this 'memset' call is a pointer to class containing a dynamic class 'X1'}}
+  // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
+  memset(&contains_array_dynamic, 0, sizeof(contains_array_dynamic));
 }
 
 void test_nowarn(void *void_ptr) {
@@ -107,6 +122,8 @@ void test_nowarn(void *void_ptr) {
   memset(&s3, 0, sizeof s3);
   memset(&c1, 0, sizeof c1);
 
+  memset(&contains_pointer_dynamic, 0, sizeof(contains_pointer_dynamic));
+
   // Unevaluated code shouldn't warn.
   (void)sizeof memset(&x1, 0, sizeof x1);