Skip to content

Commit 2be0441

Browse files
ahatanakaahatanaka
committedApr 17, 2018
[Sema] Warn about memcpy'ing non-trivial C structs.
Issue a warning when non-trivial C structs are copied or initialized by calls to memset, bzero, memcpy, or memmove. rdar://problem/36124208 Differential Revision: https://reviews.llvm.org/D45310 llvm-svn: 330202
1 parent 52a84e7 commit 2be0441

File tree

3 files changed

+156
-1
lines changed

3 files changed

+156
-1
lines changed
 

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,13 @@ def err_function_needs_feature
613613
"'%2'">;
614614
def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
615615
InGroup<ImplicitFunctionDeclare>, DefaultError;
616+
def warn_cstruct_memaccess : Warning<
617+
"%select{destination for|source of|first operand of|second operand of}0 this "
618+
"%1 call is a pointer to record %2 that is not trivial to "
619+
"%select{primitive-default-initialize|primitive-copy}3">,
620+
InGroup<DiagGroup<"nontrivial-memaccess">>;
621+
def note_nontrivial_field : Note<
622+
"field is non-trivial to %select{copy|default-initialize}0">;
616623
def warn_dyn_class_memaccess : Warning<
617624
"%select{destination for|source of|first operand of|second operand of}0 this "
618625
"%1 call is a pointer to %select{|class containing a }2dynamic class %3; "

‎clang/lib/Sema/SemaChecking.cpp

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "clang/AST/ExprObjC.h"
2929
#include "clang/AST/ExprOpenMP.h"
3030
#include "clang/AST/NSAPI.h"
31+
#include "clang/AST/NonTrivialTypeVisitor.h"
3132
#include "clang/AST/OperationKinds.h"
3233
#include "clang/AST/Stmt.h"
3334
#include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@ static QualType getSizeOfArgType(const Expr *E) {
73787379
return QualType();
73797380
}
73807381

7382+
namespace {
7383+
7384+
struct SearchNonTrivialToInitializeField
7385+
: DefaultInitializedTypeVisitor<SearchNonTrivialToInitializeField> {
7386+
using Super =
7387+
DefaultInitializedTypeVisitor<SearchNonTrivialToInitializeField>;
7388+
7389+
SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
7390+
7391+
void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
7392+
SourceLocation SL) {
7393+
if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
7394+
asDerived().visitArray(PDIK, AT, SL);
7395+
return;
7396+
}
7397+
7398+
Super::visitWithKind(PDIK, FT, SL);
7399+
}
7400+
7401+
void visitARCStrong(QualType FT, SourceLocation SL) {
7402+
S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
7403+
}
7404+
void visitARCWeak(QualType FT, SourceLocation SL) {
7405+
S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
7406+
}
7407+
void visitStruct(QualType FT, SourceLocation SL) {
7408+
for (const FieldDecl *FD : FT->castAs<RecordType>()->getDecl()->fields())
7409+
visit(FD->getType(), FD->getLocation());
7410+
}
7411+
void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK,
7412+
const ArrayType *AT, SourceLocation SL) {
7413+
visit(getContext().getBaseElementType(AT), SL);
7414+
}
7415+
void visitTrivial(QualType FT, SourceLocation SL) {}
7416+
7417+
static void diag(QualType RT, const Expr *E, Sema &S) {
7418+
SearchNonTrivialToInitializeField(E, S).visitStruct(RT, SourceLocation());
7419+
}
7420+
7421+
ASTContext &getContext() { return S.getASTContext(); }
7422+
7423+
const Expr *E;
7424+
Sema &S;
7425+
};
7426+
7427+
struct SearchNonTrivialToCopyField
7428+
: CopiedTypeVisitor<SearchNonTrivialToCopyField, false> {
7429+
using Super = CopiedTypeVisitor<SearchNonTrivialToCopyField, false>;
7430+
7431+
SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
7432+
7433+
void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
7434+
SourceLocation SL) {
7435+
if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
7436+
asDerived().visitArray(PCK, AT, SL);
7437+
return;
7438+
}
7439+
7440+
Super::visitWithKind(PCK, FT, SL);
7441+
}
7442+
7443+
void visitARCStrong(QualType FT, SourceLocation SL) {
7444+
S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 0);
7445+
}
7446+
void visitARCWeak(QualType FT, SourceLocation SL) {
7447+
S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 0);
7448+
}
7449+
void visitStruct(QualType FT, SourceLocation SL) {
7450+
for (const FieldDecl *FD : FT->castAs<RecordType>()->getDecl()->fields())
7451+
visit(FD->getType(), FD->getLocation());
7452+
}
7453+
void visitArray(QualType::PrimitiveCopyKind PCK, const ArrayType *AT,
7454+
SourceLocation SL) {
7455+
visit(getContext().getBaseElementType(AT), SL);
7456+
}
7457+
void preVisit(QualType::PrimitiveCopyKind PCK, QualType FT,
7458+
SourceLocation SL) {}
7459+
void visitTrivial(QualType FT, SourceLocation SL) {}
7460+
void visitVolatileTrivial(QualType FT, SourceLocation SL) {}
7461+
7462+
static void diag(QualType RT, const Expr *E, Sema &S) {
7463+
SearchNonTrivialToCopyField(E, S).visitStruct(RT, SourceLocation());
7464+
}
7465+
7466+
ASTContext &getContext() { return S.getASTContext(); }
7467+
7468+
const Expr *E;
7469+
Sema &S;
7470+
};
7471+
7472+
}
7473+
73817474
/// \brief Check for dangerous or invalid arguments to memset().
73827475
///
73837476
/// This issues warnings on known problematic, dangerous or unspecified
@@ -7543,7 +7636,23 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
75437636
PDiag(diag::warn_arc_object_memaccess)
75447637
<< ArgIdx << FnName << PointeeTy
75457638
<< Call->getCallee()->getSourceRange());
7546-
else
7639+
else if (const auto *RT = PointeeTy->getAs<RecordType>()) {
7640+
if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
7641+
RT->getDecl()->isNonTrivialToPrimitiveDefaultInitialize()) {
7642+
DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
7643+
PDiag(diag::warn_cstruct_memaccess)
7644+
<< ArgIdx << FnName << PointeeTy << 0);
7645+
SearchNonTrivialToInitializeField::diag(PointeeTy, Dest, *this);
7646+
} else if ((BId == Builtin::BImemcpy || BId == Builtin::BImemmove) &&
7647+
RT->getDecl()->isNonTrivialToPrimitiveCopy()) {
7648+
DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
7649+
PDiag(diag::warn_cstruct_memaccess)
7650+
<< ArgIdx << FnName << PointeeTy << 1);
7651+
SearchNonTrivialToCopyField::diag(PointeeTy, Dest, *this);
7652+
} else {
7653+
continue;
7654+
}
7655+
} else
75477656
continue;
75487657

75497658
DiagRuntimeBehavior(

‎clang/test/SemaObjC/warn-nontrivial-struct-memaccess.m

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime-has-weak -x objective-c -fobjc-arc -verify %s
2+
3+
void *memset(void *, int, __SIZE_TYPE__);
4+
void bzero(void *, __SIZE_TYPE__);
5+
void *memcpy(void *, const void *, __SIZE_TYPE__);
6+
void *memmove(void *, const void *, __SIZE_TYPE__);
7+
8+
struct Trivial {
9+
int f0;
10+
volatile int f1;
11+
};
12+
13+
struct NonTrivial0 {
14+
int f0;
15+
__weak id f1; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
16+
volatile int f2;
17+
id f3[10]; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
18+
};
19+
20+
struct NonTrivial1 {
21+
id f0; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
22+
int f1;
23+
struct NonTrivial0 f2;
24+
};
25+
26+
void testTrivial(struct Trivial *d, struct Trivial *s) {
27+
memset(d, 0, sizeof(struct Trivial));
28+
bzero(d, sizeof(struct Trivial));
29+
memcpy(d, s, sizeof(struct Trivial));
30+
memmove(d, s, sizeof(struct Trivial));
31+
}
32+
33+
void testNonTrivial1(struct NonTrivial1 *d, struct NonTrivial1 *s) {
34+
memset(d, 0, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
35+
memset((void *)d, 0, sizeof(struct NonTrivial1));
36+
bzero(d, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
37+
memcpy(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
38+
memmove(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
39+
}

0 commit comments

Comments
 (0)
Please sign in to comment.