From: Nico Weber Date: Tue, 14 Jun 2011 16:14:58 +0000 (+0000) Subject: Warn on memset(ptr, 0, sizeof(ptr)). Diagnostic wording by Jordy Rose. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e4a1c64700304459ac436fe29cb498f2da3b6194;p=clang Warn on memset(ptr, 0, sizeof(ptr)). Diagnostic wording by Jordy Rose. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@132996 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d7c352e570..6e851091d4 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -269,6 +269,9 @@ def warn_dyn_class_memaccess : Warning< InGroup>; def note_bad_memaccess_silence : Note< "explicitly cast the pointer to silence this warning">; +def warn_sizeof_pointer : Warning< + "the argument to sizeof is pointer type %0, expected %1 to match " + "%select{first|second}2 argument to %3">; /// main() // static/inline main() are not errors in C, just in C++. diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 9c47e1e1e2..389dd91f32 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1812,6 +1812,20 @@ static bool isDynamicClassType(QualType T) { return false; } +/// \brief If E is a sizeof expression, returns the expression's type in +/// OutType. +static bool sizeofExprType(const Expr* E, QualType *OutType) { + if (const UnaryExprOrTypeTraitExpr *SizeOf = + dyn_cast(E)) { + if (SizeOf->getKind() != clang::UETT_SizeOf) + return false; + + *OutType = SizeOf->getTypeOfArgument(); + return true; + } + return false; +} + /// \brief Check for dangerous or invalid arguments to memset(). /// /// This issues warnings on known problematic, dangerous or unspecified @@ -1827,8 +1841,10 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call, return; unsigned LastArg = FnName->isStr("memset")? 1 : 2; + const Expr *LenExpr = Call->getArg(2)->IgnoreParenImpCasts(); for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) { const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts(); + SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange(); QualType DestTy = Dest->getType(); if (const PointerType *DestPtrTy = DestTy->getAs()) { @@ -1836,6 +1852,17 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call, if (PointeeTy->isVoidType()) continue; + // Catch "memset(p, 0, sizeof(p))" -- needs to be sizeof(*p). + QualType SizeofTy; + if (sizeofExprType(LenExpr, &SizeofTy) && + Context.typesAreCompatible(SizeofTy, DestTy)) { + // Note: This complains about sizeof(typeof(p)) as well. + SourceLocation loc = LenExpr->getSourceRange().getBegin(); + Diag(loc, diag::warn_sizeof_pointer) + << SizeofTy << PointeeTy << ArgIdx << FnName; + break; + } + // Always complain about dynamic classes. if (isDynamicClassType(PointeeTy)) { DiagRuntimeBehavior( @@ -1847,7 +1874,6 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call, continue; } - SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange(); DiagRuntimeBehavior( Dest->getExprLoc(), Dest, PDiag(diag::note_bad_memaccess_silence) diff --git a/test/SemaCXX/warn-memset-bad-sizeof.cpp b/test/SemaCXX/warn-memset-bad-sizeof.cpp new file mode 100644 index 0000000000..c000b44773 --- /dev/null +++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// +extern "C" void *memset(void *, int, unsigned); +extern "C" void *memmove(void *s1, const void *s2, unsigned n); +extern "C" void *memcpy(void *s1, const void *s2, unsigned n); + +struct S {int a, b, c, d;}; +typedef S* PS; + +struct Foo {}; +typedef const Foo& CFooRef; +typedef const Foo CFoo; +typedef volatile Foo VFoo; +typedef const volatile Foo CVFoo; + +typedef double Mat[4][4]; + +template +inline Dest bit_cast(const Source& source) { + Dest dest; + memcpy(&dest, &source, sizeof(dest)); + return dest; +} + +// http://www.lysator.liu.se/c/c-faq/c-2.html#2-6 +void f(char fake_array[8], Mat m, const Foo& const_foo) { + S s; + S* ps = &s; + PS ps2 = &s; + char arr[5]; + char* parr[5]; + Foo foo; + + /* Should warn */ + memset(&s, 0, sizeof(&s)); // \ + // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memset'}} + memset(ps, 0, sizeof(ps)); // \ + // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memset'}} + memset(ps2, 0, sizeof(ps2)); // \ + // expected-warning {{the argument to sizeof is pointer type 'PS' (aka 'S *'), expected 'S' to match first argument to 'memset'}} + memset(ps2, 0, sizeof(typeof(ps2))); // \ + // expected-warning {{the argument to sizeof is pointer type 'typeof (ps2)' (aka 'S *'), expected 'S' to match first argument to 'memset'}} + memset(ps2, 0, sizeof(PS)); // \ + // expected-warning {{the argument to sizeof is pointer type 'PS' (aka 'S *'), expected 'S' to match first argument to 'memset'}} + memset(fake_array, 0, sizeof(fake_array)); // \ + // expected-warning {{the argument to sizeof is pointer type 'char *', expected 'char' to match first argument to 'memset'}} + + memcpy(&s, 0, sizeof(&s)); // \ + // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memcpy'}} + memcpy(0, &s, sizeof(&s)); // \ + // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match second argument to 'memcpy'}} + + /* Shouldn't warn */ + memset((void*)&s, 0, sizeof(&s)); + memset(&s, 0, sizeof(s)); + memset(&s, 0, sizeof(S)); + memset(&s, 0, sizeof(const S)); + memset(&s, 0, sizeof(volatile S)); + memset(&s, 0, sizeof(volatile const S)); + memset(&foo, 0, sizeof(CFoo)); + memset(&foo, 0, sizeof(VFoo)); + memset(&foo, 0, sizeof(CVFoo)); + memset(ps, 0, sizeof(*ps)); + memset(ps2, 0, sizeof(*ps2)); + memset(ps2, 0, sizeof(typeof(*ps2))); + memset(arr, 0, sizeof(arr)); + memset(parr, 0, sizeof(parr)); + + memcpy(&foo, &const_foo, sizeof(Foo)); + memcpy((void*)&s, 0, sizeof(&s)); + memcpy(0, (void*)&s, sizeof(&s)); + + CFooRef cfoo = foo; + memcpy(&foo, &cfoo, sizeof(Foo)); + + memcpy(0, &arr, sizeof(arr)); + typedef char Buff[8]; + memcpy(0, &arr, sizeof(Buff)); + + unsigned char* puc; + bit_cast(puc); + + float* pf; + bit_cast(pf); + + int iarr[14]; + memset(&iarr[0], 0, sizeof iarr); + + int* iparr[14]; + memset(&iparr[0], 0, sizeof iparr); + + memset(m, 0, sizeof(Mat)); + + // Copy to raw buffer shouldn't warn either + memcpy(&foo, &arr, sizeof(Foo)); + memcpy(&arr, &foo, sizeof(Foo)); +}