ahatanak created this revision.
ahatanak added a reviewer: rjmccall.

Issue a warning when non-trivial C structs (structs with '__weak' or '__strong' 
fields) are copied or initialized by calls to memset, bzero, memcpy, and 
memmove. This is similar to gcc's -Wclass-memaccess warning. It might be better 
to add support for -Wclass-memaccess and make it cover both C and C++ structs 
that are non-trivial.

rdar://problem/36124208


Repository:
  rC Clang

https://reviews.llvm.org/D45310

Files:
  include/clang/AST/NonTrivialCStructTypeVisitor.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaObjC/warn-nontrivial-struct-memaccess.m

Index: test/SemaObjC/warn-nontrivial-struct-memaccess.m
===================================================================
--- /dev/null
+++ test/SemaObjC/warn-nontrivial-struct-memaccess.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime-has-weak -x objective-c -fobjc-arc -verify %s
+
+void *memset(void *, int, __SIZE_TYPE__);
+void bzero(void *, __SIZE_TYPE__);
+void *memcpy(void *, const void *, __SIZE_TYPE__);
+void *memmove(void *, const void *, __SIZE_TYPE__);
+
+struct Trivial {
+  int f0;
+  volatile int f1;
+};
+
+struct NonTrivial0 {
+  int f0;
+  __weak id f1; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  volatile int f2;
+  id f3[10]; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+};
+
+struct NonTrivial1 {
+  id f0; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  int f1;
+  struct NonTrivial0 f2;
+};
+
+void testTrivial(struct Trivial *d, struct Trivial *s) {
+  memset(d, 0, sizeof(struct Trivial));
+  bzero(d, sizeof(struct Trivial));
+  memcpy(d, s, sizeof(struct Trivial));
+  memmove(d, s, sizeof(struct Trivial));
+}
+
+void testNonTrivial1(struct NonTrivial1 *d, struct NonTrivial1 *s) {
+  memset(d, 0, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memset((void *)d, 0, sizeof(struct NonTrivial1));
+  bzero(d, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memcpy(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+  memmove(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/AST/NonTrivialCStructTypeVisitor.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
@@ -7321,6 +7322,78 @@
   return QualType();
 }
 
+namespace {
+
+struct SearchNonTrivialToInitializeField : DefaultInitializedTypeVisitor<SearchNonTrivialToInitializeField> {
+  SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+    S.DiagRuntimeBehavior(SL, E,
+                          S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+    S.DiagRuntimeBehavior(SL, E,
+                          S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+    for (const FieldDecl *FD : FT->castAs<RecordType>()->getDecl()->fields())
+      visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+                  SourceLocation SL) {
+    const auto *AT = getContext().getAsConstantArrayType(FT);
+    visit(PDIK, getContext().getBaseElementType(AT), SL);
+  }
+  void visitTrivial(QualType FT, SourceLocation SL) {}
+
+  static void diag(QualType RT, const Expr *E, Sema &S) {
+    SearchNonTrivialToInitializeField(E, S).visitStruct(RT, SourceLocation());
+  }
+
+  ASTContext &getContext() {
+    return S.getASTContext();
+  }
+
+  const Expr *E;
+  Sema &S;
+};
+
+struct SearchNonTrivialToCopyField : CopiedTypeVisitor<SearchNonTrivialToCopyField, false> {
+  SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+    S.DiagRuntimeBehavior(SL, E,
+                          S.PDiag(diag::note_nontrivial_field) << 0);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+    S.DiagRuntimeBehavior(SL, E,
+                          S.PDiag(diag::note_nontrivial_field) << 0);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+    for (const FieldDecl *FD : FT->castAs<RecordType>()->getDecl()->fields())
+      visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveCopyKind PCK, QualType FT,
+                  SourceLocation SL) {
+    const auto *AT = getContext().getAsConstantArrayType(FT);
+    visit(PCK, getContext().getBaseElementType(AT), SL);
+  }
+  void preVisit(QualType::PrimitiveCopyKind PCK, QualType FT, SourceLocation SL) {}
+  void visitTrivial(QualType FT, SourceLocation SL) {}
+  void visitVolatileTrivial(QualType FT, SourceLocation SL) {}
+
+  static void diag(QualType RT, const Expr *E, Sema &S) {
+    SearchNonTrivialToCopyField(E, S).visitStruct(RT, SourceLocation());
+  }
+
+  ASTContext &getContext() {
+    return S.getASTContext();
+  }
+
+  const Expr *E;
+  Sema &S;
+};
+
+}
+
 /// \brief Check for dangerous or invalid arguments to memset().
 ///
 /// This issues warnings on known problematic, dangerous or unspecified
@@ -7486,7 +7559,23 @@
         PDiag(diag::warn_arc_object_memaccess)
           << ArgIdx << FnName << PointeeTy
           << Call->getCallee()->getSourceRange());
-    else
+    else if (const auto *RT = PointeeTy->getAs<RecordType>()) {
+      if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
+          RT->getDecl()->isNonTrivialToPrimitiveDefaultInitialize()) {
+          DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+                              PDiag(diag::warn_cstruct_memaccess) << ArgIdx << FnName
+                                  << PointeeTy << 0);
+          SearchNonTrivialToInitializeField::diag(PointeeTy, Dest, *this);
+      } else if ((BId == Builtin::BImemcpy || BId == Builtin::BImemmove) &&
+                 RT->getDecl()->isNonTrivialToPrimitiveCopy()) {
+          DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+                              PDiag(diag::warn_cstruct_memaccess) << ArgIdx << FnName
+                                  << PointeeTy << 1);
+          SearchNonTrivialToCopyField::diag(PointeeTy, Dest, *this);
+      } else {
+        continue;
+      }
+    } else
       continue;
 
     DiagRuntimeBehavior(
Index: lib/CodeGen/CGNonTrivialStruct.cpp
===================================================================
--- lib/CodeGen/CGNonTrivialStruct.cpp
+++ lib/CodeGen/CGNonTrivialStruct.cpp
@@ -14,6 +14,7 @@
 
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
+#include "clang/AST/NonTrivialCStructTypeVisitor.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include <array>
 
@@ -31,101 +32,6 @@
 enum { DstIdx = 0, SrcIdx = 1 };
 const char *ValNameStr[2] = {"dst", "src"};
 
-template <class Derived, class RetTy = void> struct DestructedTypeVisitor {
-  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
-    return asDerived().visit(FT.isDestructedType(), FT,
-                             std::forward<Ts>(Args)...);
-  }
-
-  template <class... Ts>
-  RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
-    if (asDerived().getContext().getAsArrayType(FT))
-      return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
-
-    switch (DK) {
-    case QualType::DK_objc_strong_lifetime:
-      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
-    case QualType::DK_nontrivial_c_struct:
-      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
-    case QualType::DK_none:
-      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
-    case QualType::DK_cxx_destructor:
-      return asDerived().visitCXXDestructor(FT, std::forward<Ts>(Args)...);
-    case QualType::DK_objc_weak_lifetime:
-      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
-    }
-
-    llvm_unreachable("unknown destruction kind");
-  }
-
-  Derived &asDerived() { return static_cast<Derived &>(*this); }
-};
-
-template <class Derived, class RetTy = void>
-struct DefaultInitializedTypeVisitor {
-  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
-    return asDerived().visit(FT.isNonTrivialToPrimitiveDefaultInitialize(), FT,
-                             std::forward<Ts>(Args)...);
-  }
-
-  template <class... Ts>
-  RetTy visit(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
-              Ts &&... Args) {
-    if (asDerived().getContext().getAsArrayType(FT))
-      return asDerived().visitArray(PDIK, FT, std::forward<Ts>(Args)...);
-
-    switch (PDIK) {
-    case QualType::PDIK_ARCStrong:
-      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
-    case QualType::PDIK_ARCWeak:
-      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
-    case QualType::PDIK_Struct:
-      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
-    case QualType::PDIK_Trivial:
-      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
-    }
-
-    llvm_unreachable("unknown default-initialize kind");
-  }
-
-  Derived &asDerived() { return static_cast<Derived &>(*this); }
-};
-
-template <class Derived, bool IsMove, class RetTy = void>
-struct CopiedTypeVisitor {
-  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
-    QualType::PrimitiveCopyKind PCK =
-        IsMove ? FT.isNonTrivialToPrimitiveDestructiveMove()
-               : FT.isNonTrivialToPrimitiveCopy();
-    return asDerived().visit(PCK, FT, std::forward<Ts>(Args)...);
-  }
-
-  template <class... Ts>
-  RetTy visit(QualType::PrimitiveCopyKind PCK, QualType FT, Ts &&... Args) {
-    asDerived().preVisit(PCK, FT, std::forward<Ts>(Args)...);
-
-    if (asDerived().getContext().getAsArrayType(FT))
-      return asDerived().visitArray(PCK, FT, std::forward<Ts>(Args)...);
-
-    switch (PCK) {
-    case QualType::PCK_ARCStrong:
-      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
-    case QualType::PCK_ARCWeak:
-      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
-    case QualType::PCK_Struct:
-      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
-    case QualType::PCK_Trivial:
-      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
-    case QualType::PCK_VolatileTrivial:
-      return asDerived().visitVolatileTrivial(FT, std::forward<Ts>(Args)...);
-    }
-
-    llvm_unreachable("unknown primitive copy kind");
-  }
-
-  Derived &asDerived() { return static_cast<Derived &>(*this); }
-};
-
 template <class Derived> struct StructVisitor {
   StructVisitor(ASTContext &Ctx) : Ctx(Ctx) {}
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -612,6 +612,13 @@
             "'%2'">;
 def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
   InGroup<ImplicitFunctionDeclare>, DefaultError;
+def warn_cstruct_memaccess : Warning<
+  "%select{destination for|source of|first operand of|second operand of}0 this "
+  "%1 call is a pointer to record %2 that is not trivial to "
+  "%select{primitive-default-initialize|primitive-copy}3">,
+  InGroup<DiagGroup<"cstruct-memaccess">>;
+def note_nontrivial_field : Note<
+  "field is non-trivial to %select{copy|default-initialize}0">;
 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 %select{|class containing a }2dynamic class %3; "
Index: include/clang/AST/NonTrivialCStructTypeVisitor.h
===================================================================
--- /dev/null
+++ include/clang/AST/NonTrivialCStructTypeVisitor.h
@@ -0,0 +1,118 @@
+//===--- TypeVisitor.h - Visitor for Type subclasses ------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the TypeVisitor interface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_NON_TRIVIAL_CSTRUCT_TYPE_VISITOR_H
+#define LLVM_CLANG_NON_TRIVIAL_CSTRUCT_TYPE_VISITOR_H
+
+#include "clang/AST/Type.h"
+
+namespace clang {
+
+template <class Derived, class RetTy = void> struct DestructedTypeVisitor {
+  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
+    return asDerived().visit(FT.isDestructedType(), FT,
+                             std::forward<Ts>(Args)...);
+  }
+
+  template <class... Ts>
+  RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
+
+    switch (DK) {
+    case QualType::DK_objc_strong_lifetime:
+      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
+    case QualType::DK_nontrivial_c_struct:
+      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
+    case QualType::DK_none:
+      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
+    case QualType::DK_cxx_destructor:
+      return asDerived().visitCXXDestructor(FT, std::forward<Ts>(Args)...);
+    case QualType::DK_objc_weak_lifetime:
+      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
+    }
+
+    llvm_unreachable("unknown destruction kind");
+  }
+
+  Derived &asDerived() { return static_cast<Derived &>(*this); }
+};
+
+template <class Derived, class RetTy = void>
+struct DefaultInitializedTypeVisitor {
+  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
+    return asDerived().visit(FT.isNonTrivialToPrimitiveDefaultInitialize(), FT,
+                             std::forward<Ts>(Args)...);
+  }
+
+  template <class... Ts>
+  RetTy visit(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+              Ts &&... Args) {
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(PDIK, FT, std::forward<Ts>(Args)...);
+
+    switch (PDIK) {
+    case QualType::PDIK_ARCStrong:
+      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
+    case QualType::PDIK_ARCWeak:
+      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
+    case QualType::PDIK_Struct:
+      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
+    case QualType::PDIK_Trivial:
+      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
+    }
+
+    llvm_unreachable("unknown default-initialize kind");
+  }
+
+  Derived &asDerived() { return static_cast<Derived &>(*this); }
+};
+
+template <class Derived, bool IsMove, class RetTy = void>
+struct CopiedTypeVisitor {
+  template <class... Ts> RetTy visit(QualType FT, Ts &&... Args) {
+    QualType::PrimitiveCopyKind PCK =
+        IsMove ? FT.isNonTrivialToPrimitiveDestructiveMove()
+               : FT.isNonTrivialToPrimitiveCopy();
+    return asDerived().visit(PCK, FT, std::forward<Ts>(Args)...);
+  }
+
+  template <class... Ts>
+  RetTy visit(QualType::PrimitiveCopyKind PCK, QualType FT, Ts &&... Args) {
+    asDerived().preVisit(PCK, FT, std::forward<Ts>(Args)...);
+
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(PCK, FT, std::forward<Ts>(Args)...);
+
+    switch (PCK) {
+    case QualType::PCK_ARCStrong:
+      return asDerived().visitARCStrong(FT, std::forward<Ts>(Args)...);
+    case QualType::PCK_ARCWeak:
+      return asDerived().visitARCWeak(FT, std::forward<Ts>(Args)...);
+    case QualType::PCK_Struct:
+      return asDerived().visitStruct(FT, std::forward<Ts>(Args)...);
+    case QualType::PCK_Trivial:
+      return asDerived().visitTrivial(FT, std::forward<Ts>(Args)...);
+    case QualType::PCK_VolatileTrivial:
+      return asDerived().visitVolatileTrivial(FT, std::forward<Ts>(Args)...);
+    }
+
+    llvm_unreachable("unknown primitive copy kind");
+  }
+
+  Derived &asDerived() { return static_cast<Derived &>(*this); }
+};
+
+} // end namespace clang
+
+#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to