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

[rtrieu]

LGTM

[rnk]

Closed by commit rL211972 (authored by @rnk).

include/clang/Basic/DiagnosticSemaKinds.td

 def warn_redecl_library_builtin : Warning<
   "incompatible redeclaration of library function %0">,
   InGroup<DiagGroup<"incompatible-library-redeclaration">>;
 def err_builtin_definition : Error<"definition of builtin function %0">;
 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">;
 def warn_sizeof_pointer_expr_memaccess : Warning<
   "'%0' call operates on objects of type %1 while the size is based on a " 
   "different type %2">, 
   InGroup<SizeofPointerMemaccess>;
 def warn_sizeof_pointer_expr_memaccess_note : Note<

lib/Sema/SemaChecking.cpp

static bool CheckMemorySizeofForComparison(Sema &S, const Expr *E,
   S.Diag(SizeRange.getBegin(), diag::note_memsize_comparison_cast_silence)
       << FixItHint::CreateInsertion(SizeRange.getBegin(), "(size_t)(")
       << FixItHint::CreateInsertion(S.getLocForEndOfToken(SizeRange.getEnd()),
                                     ")");
 
   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;
-  
+/// \brief Determine whether the given type is or contains a dynamic class type
+/// (e.g., whether it has a vtable).
+static bool isOrContainsDynamicClass(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 false;
+
+  if (RD->isDynamicClass())
+    return true;
+
+  // 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()) {
+    if (isOrContainsDynamicClass(FD->getType(), IsContained)) {
+      IsContained = true;
+    }
+  }
   return false;
 }
 
 /// \brief If E is a sizeof expression, returns its argument expression,
 /// otherwise returns NULL.
 static const Expr *getSizeOfExprArg(const Expr* E) {
   if (const UnaryExprOrTypeTraitExpr *SizeOf =
       dyn_cast<UnaryExprOrTypeTraitExpr>(E))
if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
                                 << FnName << SizeOfArgTy << ArgIdx
                                 << PointeeTy << Dest->getSourceRange()
                                 << LenExpr->getSourceRange());
           break;
         }
       }
 
       // Always complain about dynamic classes.
-      if (isDynamicClassType(PointeeTy)) {
+      bool IsContained;
+      if (isOrContainsDynamicClass(PointeeTy, IsContained)) {
 
         unsigned OperationType = 0;
         // "overwritten" if we're warning about the destination for any call
         // but memcmp; otherwise a verb appropriate to the call.
         if (ArgIdx != 0 || BId == Builtin::BImemcmp) {
           if (BId == Builtin::BImemcpy)
             OperationType = 1;
           else if(BId == Builtin::BImemmove)
             OperationType = 2;
           else if (BId == Builtin::BImemcmp)
             OperationType = 3;
         }
           
         DiagRuntimeBehavior(
           Dest->getExprLoc(), Dest,
           PDiag(diag::warn_dyn_class_memaccess)
             << (BId == Builtin::BImemcmp ? ArgIdx + 2 : ArgIdx)
-            << FnName << PointeeTy
-            << OperationType
+            << FnName << IsContained << PointeeTy << OperationType
             << Call->getCallee()->getSourceRange());
       } else if (PointeeTy.hasNonTrivialObjCLifetime() &&
                BId != Builtin::BImemset)
         DiagRuntimeBehavior(
           Dest->getExprLoc(), Dest,
           PDiag(diag::warn_arc_object_memaccess)
             << ArgIdx << FnName << PointeeTy
             << Call->getCallee()->getSourceRange());

test/SemaCXX/warn-bad-memaccess.cpp

class C1 {
   int x, y, z;
 public:
   void foo() {}
 } c1;
 
 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}} \
       // 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 dynamic class}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
 
__builtin___memmove_chk(0, &x1, sizeof x1, sizeof x1); //                   \
       // expected-warning{{source of this '__builtin___memmove_chk' call is a pointer to dynamic class}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   __builtin___memcpy_chk(&x1, 0, sizeof x1, sizeof x1); //                    \
       // expected-warning{{destination for this '__builtin___memcpy_chk' call is a pointer to dynamic class}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   __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}}
+  // 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}}
+  // 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}}
+  // 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) {
   int i, *iptr;
   float y;
   char c;
 
   memset(&i, 0, sizeof i);
   memset(&iptr, 0, sizeof iptr);
   memset(&y, 0, sizeof y);
   memset(&c, 0, sizeof c);
   memset(void_ptr, 0, 42);
   memset(&s1, 0, sizeof s1);
   memset(&s2, 0, sizeof s2);
   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);
 
   // Dead code shouldn't warn.
   if (false) memset(&x1, 0, sizeof x1);
 }
 
 namespace N {
   void *memset(void *, int, unsigned);
   void test_nowarn() {
     N::memset(&x1, 0, sizeof x1);
   }
 }