Skip to content

Commit ac68726

Browse files
committed
Clean up the "non-POD memaccess" stuff some. This adds a properly named
diagnostic group to cover the cases where we have definitively bad behavior: dynamic classes. It also rips out the existing support for POD-based checking. This didn't work well, and triggered too many false positives. I'm looking into a possibly more principled way to warn on the fundamental buggy construct here. POD-ness isn't the critical aspect anyways, so a clean slate is better. This also removes some silliness from the code until the new checks arrive. llvm-svn: 132534
1 parent 7ae2638 commit ac68726

File tree

3 files changed

+21
-46
lines changed

3 files changed

+21
-46
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,8 @@ def warn_builtin_unknown : Warning<"use of unknown builtin %0">, DefaultError;
264264
def warn_dyn_class_memaccess : Warning<
265265
"%select{destination for|source of}0 this %1 call is a pointer to dynamic "
266266
"class %2; vtable pointer will be overwritten">,
267-
InGroup<DiagGroup<"non-pod-memaccess">>;
268-
def warn_non_pod_memaccess : Warning<
269-
"%select{destination for|source of}0 this %1 call is a pointer to non-POD "
270-
"type %2">,
271-
InGroup<DiagGroup<"non-pod-memaccess">>, DefaultIgnore;
272-
def note_non_pod_memaccess_silence : Note<
267+
InGroup<DiagGroup<"dynamic-class-memaccess">>;
268+
def note_bad_memaccess_silence : Note<
273269
"explicitly cast the pointer to silence this warning">;
274270

275271
/// main()

clang/lib/Sema/SemaChecking.cpp

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1814,7 +1814,7 @@ static bool isDynamicClassType(QualType T) {
18141814

18151815
/// \brief Check for dangerous or invalid arguments to memset().
18161816
///
1817-
/// This issues warnings on known problematic or dangerous or unspecified
1817+
/// This issues warnings on known problematic, dangerous or unspecified
18181818
/// arguments to the standard 'memset', 'memcpy', and 'memmove' function calls.
18191819
///
18201820
/// \param Call The call expression to diagnose.
@@ -1836,27 +1836,21 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
18361836
if (PointeeTy->isVoidType())
18371837
continue;
18381838

1839-
unsigned DiagID = 0;
18401839
// Always complain about dynamic classes.
1841-
if (isDynamicClassType(PointeeTy))
1842-
DiagID = diag::warn_dyn_class_memaccess;
1843-
// Check the C++11 POD definition regardless of language mode; it is more
1844-
// relaxed than earlier definitions and we don't want spurious warnings.
1845-
else if (!PointeeTy->isCXX11PODType())
1846-
DiagID = diag::warn_non_pod_memaccess;
1847-
else
1840+
if (isDynamicClassType(PointeeTy)) {
1841+
DiagRuntimeBehavior(
1842+
Dest->getExprLoc(), Dest,
1843+
PDiag(diag::warn_dyn_class_memaccess)
1844+
<< ArgIdx << FnName << PointeeTy
1845+
<< Call->getCallee()->getSourceRange());
1846+
} else {
18481847
continue;
1849-
1850-
DiagRuntimeBehavior(
1851-
Dest->getExprLoc(), Dest,
1852-
PDiag(DiagID)
1853-
<< ArgIdx << FnName << PointeeTy
1854-
<< Call->getCallee()->getSourceRange());
1848+
}
18551849

18561850
SourceRange ArgRange = Call->getArg(0)->getSourceRange();
18571851
DiagRuntimeBehavior(
18581852
Dest->getExprLoc(), Dest,
1859-
PDiag(diag::note_non_pod_memaccess_silence)
1853+
PDiag(diag::note_bad_memaccess_silence)
18601854
<< FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
18611855
break;
18621856
}

clang/test/SemaCXX/warn-non-pod-memset.cpp renamed to clang/test/SemaCXX/warn-bad-memaccess.cpp

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,42 @@
1-
// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memaccess -verify %s
1+
// RUN: %clang_cc1 -fsyntax-only -Wdynamic-class-memaccess -verify %s
22

33
extern "C" void *memset(void *, int, unsigned);
44
extern "C" void *memmove(void *s1, const void *s2, unsigned n);
55
extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
66

7-
// Several POD types that should not warn.
7+
// Several types that should not warn.
88
struct S1 {} s1;
99
struct S2 { int x; } s2;
1010
struct S3 { float x, y; S1 s[4]; void (*f)(S1**); } s3;
1111

12-
// We use the C++11 concept of POD for this warning, so ensure a non-aggregate
13-
// still warns.
1412
class C1 {
1513
int x, y, z;
1614
public:
1715
void foo() {}
1816
} c1;
1917

20-
// Non-POD types that should warn.
21-
struct X1 { X1(); } x1;
22-
struct X2 { ~X2(); } x2;
23-
struct X3 { virtual void f(); } x3;
24-
struct X4 : X2 {} x4;
25-
struct X5 : virtual S1 {} x5;
18+
struct X1 { virtual void f(); } x1;
19+
struct X2 : virtual S1 {} x2;
2620

2721
void test_warn() {
2822
memset(&x1, 0, sizeof x1); // \
29-
// expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
30-
// expected-note {{explicitly cast the pointer to silence this warning}}
31-
memset(&x2, 0, sizeof x2); // \
32-
// expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
33-
// expected-note {{explicitly cast the pointer to silence this warning}}
34-
memset(&x3, 0, sizeof x3); // \
3523
// expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
3624
// expected-note {{explicitly cast the pointer to silence this warning}}
37-
memset(&x4, 0, sizeof x4); // \
38-
// expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
39-
// expected-note {{explicitly cast the pointer to silence this warning}}
40-
memset(&x5, 0, sizeof x5); // \
25+
memset(&x2, 0, sizeof x2); // \
4126
// expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
4227
// expected-note {{explicitly cast the pointer to silence this warning}}
4328

4429
memmove(&x1, 0, sizeof x1); // \
45-
// expected-warning{{destination for this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
30+
// expected-warning{{destination for this 'memmove' call is a pointer to dynamic class}} \
4631
// expected-note {{explicitly cast the pointer to silence this warning}}
4732
memmove(0, &x1, sizeof x1); // \
48-
// expected-warning{{source of this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
33+
// expected-warning{{source of this 'memmove' call is a pointer to dynamic class}} \
4934
// expected-note {{explicitly cast the pointer to silence this warning}}
5035
memcpy(&x1, 0, sizeof x1); // \
51-
// expected-warning{{destination for this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
36+
// expected-warning{{destination for this 'memcpy' call is a pointer to dynamic class}} \
5237
// expected-note {{explicitly cast the pointer to silence this warning}}
5338
memcpy(0, &x1, sizeof x1); // \
54-
// expected-warning{{source of this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
39+
// expected-warning{{source of this 'memcpy' call is a pointer to dynamic class}} \
5540
// expected-note {{explicitly cast the pointer to silence this warning}}
5641
}
5742

0 commit comments

Comments
 (0)