[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330202: [Sema] Warn about memcpy'ing non-trivial C
structs. (authored by ahatanak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45310?vs=142705&id=142815#toc
Repository:
rC Clang
https://reviews.llvm.org/D45310
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/SemaObjC/warn-nontrivial-struct-memaccess.m
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -613,6 +613,13 @@
"'%2'">;
def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
InGroup, 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>;
+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: test/SemaObjC/warn-nontrivial-struct-memaccess.m
===
--- test/SemaObjC/warn-nontrivial-struct-memaccess.m
+++ 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/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@
return QualType();
}
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+ using Super =
+ DefaultInitializedTypeVisitor;
+
+ SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+ void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+ asDerived().visitArray(PDIK, AT, SL);
+ return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+ }
+
+ 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()->getDecl()->fields())
+ visit(FD->getType(), FD->getLocation());
+ }
+ void visitArray(QualType::PrimitiveDefaultInitializeKind P
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks; LGTM. Repository: rC Clang https://reviews.llvm.org/D45310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak updated this revision to Diff 142705.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Fix indentation.
Repository:
rC Clang
https://reviews.llvm.org/D45310
Files:
include/clang/AST/NonTrivialTypeVisitor.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/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@
return QualType();
}
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+ using Super =
+ DefaultInitializedTypeVisitor;
+
+ SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+ void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+ asDerived().visitArray(PDIK, AT, SL);
+ return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+ }
+
+ 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()->getDecl()->fields())
+ visit(FD->getType(), FD->getLocation());
+ }
+ void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK,
+ const ArrayType *AT, SourceLocation SL) {
+visit(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 {
+ using Super = CopiedTypeVisitor;
+
+ SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+ void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+ asDerived().visitArray(PCK, AT, SL);
+ return;
+}
+
+Super::visitWithKind(PCK, FT, SL);
+ }
+
+ void visitARCStrong(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
rjmccall added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:7651
+ << PointeeTy << 1);
+ SearchNonTrivialToCopyField::diag(PointeeTy, Dest, *this);
+ } else {
Indentation seems messed up in these two clauses.
Repository:
rC Clang
https://reviews.llvm.org/D45310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak added inline comments.
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+if (asDerived().getContext().getAsArrayType(FT))
+ return asDerived().visitArray(DK, FT, std::forward(Args)...);
+
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Should you have this pass the array type down? And is it really
> > > important to do this in the generic visitor? It seems like something you
> > > could do in an IRGen subclass.
> > The subclasses in CGNonTrivialStruct.cpp need the size and the element type
> > of the array to be passed to visitArray, so I think we have to pass the
> > array type to visitArray. I guess it's possible to move this to the
> > subclasses, but then the visit methods in the subclasses have to check
> > whether the type is an array or not. I think we had a discussion on how
> > arrays should be handled in this review: https://reviews.llvm.org/D41228.
> >
> > But perhaps you have a better idea in mind?
> Well, you could "override" the visit method in the subclass, e.g.:
>
> template
> class CGDestructedTypeVisitor : public DestructedTypeVisitor RetTy> {
> using super = DestructedTypeVisitor;
>
> public:
> using super::asDerived;
> using super::visit;
>
> template
> RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
> if (asDerived().getContext().getAsArrayType(FT))
> return asDerived().visitArray(DK, FT, std::forward(Args)...);
>
> return super::visit(DK, FT, std::forward(Args)...);
> }
> };
>
> It's a bit more boilerplate, but I really feel like the array logic doesn't
> belong in the generic visitor.
>
> About the array type: I wasn't trying to suggest that you should pass the
> element type to visitArray, I was suggesting you could just pass the array
> type as an `ArrayType*`, since that's what `visitArray` actually wants.
I incorporated both of your suggestions. I renamed the overloaded method
'visit' to 'visitWithKind' so that the 'visit' method that doesn't take the
DestructionKind doesn't get hidden by the one that takes DestructionKind and is
defined in the derived classes. There are a couple of places in
CGNonTrivialStruct.cpp that call the former method.
Repository:
rC Clang
https://reviews.llvm.org/D45310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak updated this revision to Diff 142691.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Move method visitArray to the derived classes and pass the array type to the
method instead of the QualType.
Repository:
rC Clang
https://reviews.llvm.org/D45310
Files:
include/clang/AST/NonTrivialTypeVisitor.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/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@
return QualType();
}
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+ using Super =
+ DefaultInitializedTypeVisitor;
+
+ SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+ void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+ asDerived().visitArray(PDIK, AT, SL);
+ return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+ }
+
+ 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()->getDecl()->fields())
+ visit(FD->getType(), FD->getLocation());
+ }
+ void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK,
+ const ArrayType *AT, SourceLocation SL) {
+visit(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 {
+ using Super = CopiedTypeVisitor;
+
+ SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+ void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+ asDerived().visitArray(PCK, AT, SL);
+ return;
+}
+
+Super::visitWithKind(PCK, FT, SL);
+ }
+
+ void visitARC
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
rjmccall added inline comments.
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+if (asDerived().getContext().getAsArrayType(FT))
+ return asDerived().visitArray(DK, FT, std::forward(Args)...);
+
ahatanak wrote:
> rjmccall wrote:
> > Should you have this pass the array type down? And is it really important
> > to do this in the generic visitor? It seems like something you could do in
> > an IRGen subclass.
> The subclasses in CGNonTrivialStruct.cpp need the size and the element type
> of the array to be passed to visitArray, so I think we have to pass the array
> type to visitArray. I guess it's possible to move this to the subclasses, but
> then the visit methods in the subclasses have to check whether the type is an
> array or not. I think we had a discussion on how arrays should be handled in
> this review: https://reviews.llvm.org/D41228.
>
> But perhaps you have a better idea in mind?
Well, you could "override" the visit method in the subclass, e.g.:
template
class CGDestructedTypeVisitor : public DestructedTypeVisitor {
using super = DestructedTypeVisitor;
public:
using super::asDerived;
using super::visit;
template
RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
if (asDerived().getContext().getAsArrayType(FT))
return asDerived().visitArray(DK, FT, std::forward(Args)...);
return super::visit(DK, FT, std::forward(Args)...);
}
};
It's a bit more boilerplate, but I really feel like the array logic doesn't
belong in the generic visitor.
About the array type: I wasn't trying to suggest that you should pass the
element type to visitArray, I was suggesting you could just pass the array type
as an `ArrayType*`, since that's what `visitArray` actually wants.
Repository:
rC Clang
https://reviews.llvm.org/D45310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak updated this revision to Diff 141291.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D45310
Files:
include/clang/AST/NonTrivialTypeVisitor.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/NonTrivialTypeVisitor.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(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()->getDecl()->fields())
+ visit(FD->getType(), FD->getLocation());
+ }
+ void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+const auto *AT = getContext().getAsArrayType(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(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()->getDecl()->fields())
+ visit(FD->getType(), FD->getLocation());
+ }
+ void visitArray(QualType::PrimitiveCopyKind PCK, QualType FT,
+ SourceLocation SL) {
+const auto *AT = getContext().getAsArrayType(FT);
+visit(PCK, getContext().getBaseElement
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak added inline comments.
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+if (asDerived().getContext().getAsArrayType(FT))
+ return asDerived().visitArray(DK, FT, std::forward(Args)...);
+
rjmccall wrote:
> Should you have this pass the array type down? And is it really important to
> do this in the generic visitor? It seems like something you could do in an
> IRGen subclass.
The subclasses in CGNonTrivialStruct.cpp need the size and the element type of
the array to be passed to visitArray, so I think we have to pass the array type
to visitArray. I guess it's possible to move this to the subclasses, but then
the visit methods in the subclasses have to check whether the type is an array
or not. I think we had a discussion on how arrays should be handled in this
review: https://reviews.llvm.org/D41228.
But perhaps you have a better idea in mind?
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:619
+ "%select{primitive-default-initialize|primitive-copy}3">,
+ InGroup>;
+def note_nontrivial_field : Note<
rjmccall wrote:
> I think this warning group should be -Wnontrivial-memaccess, and maybe
> -Wclass-memaccess should just be a subgroup of it.
Yes, we can create different DiagGroups when support for -Wclass-memaccess
(warning for C++ classes) is added.
Repository:
rC Clang
https://reviews.llvm.org/D45310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
rjmccall added inline comments.
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:12
+//
+//===--===//
+
The header comment here was clearly just copied from another file.
I would name this header "NonTrivialTypeVisitor.h"; I don't think the CStruct
adds anything, especially because the visitors actually start from types.
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+if (asDerived().getContext().getAsArrayType(FT))
+ return asDerived().visitArray(DK, FT, std::forward(Args)...);
+
Should you have this pass the array type down? And is it really important to
do this in the generic visitor? It seems like something you could do in an
IRGen subclass.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:619
+ "%select{primitive-default-initialize|primitive-copy}3">,
+ InGroup>;
+def note_nontrivial_field : Note<
I think this warning group should be -Wnontrivial-memaccess, and maybe
-Wclass-memaccess should just be a subgroup of it.
Comment at: lib/Sema/SemaChecking.cpp:7343
+ SourceLocation SL) {
+const auto *AT = getContext().getAsConstantArrayType(FT);
+visit(PDIK, getContext().getBaseElementType(AT), SL);
It would be better to just getAsArrayType, for generality purposes.
Repository:
rC Clang
https://reviews.llvm.org/D45310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
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(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()->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(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 (c
