Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -429,6 +429,10 @@ "argument to 'sizeof' in %0 call is the same pointer type %1 as the " "%select{destination|source}2; expected %3 or an explicit length">, InGroup; +def warn_sizeof_pointer_product_memaccess : Warning< + "argument to 'sizeof' in %0 call has a different type %1 as the " + "%select{destination|source}2; expected %3">, + InGroup; def warn_strlcpycat_wrong_size : Warning< "size argument in %0 call appears to be size of the source; " "expected the size of the destination">, Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -4705,15 +4705,71 @@ } /// \brief If E is a sizeof expression, returns its argument type. -static QualType getSizeOfArgType(const Expr *E) { +static QualType getSizeOfArgType(const Expr *E, bool RequireType = false) { if (const UnaryExprOrTypeTraitExpr *SizeOf = - dyn_cast(E)) - if (SizeOf->getKind() == clang::UETT_SizeOf) + dyn_cast(E)) + if (SizeOf->getKind() == clang::UETT_SizeOf && + (!RequireType || SizeOf->isArgumentType())) return SizeOf->getTypeOfArgument(); return QualType(); } +/// \brief Tries to deconstruct E into the product of sizeof(type) and Sub. +static bool getSizeOfArgTypeProduct(const Expr *E, QualType &SizeOf, + const Expr *&Factor) { + const auto *Product = dyn_cast(E); + if (!Product || Product->getOpcode() != BO_Mul) + return false; + + QualType SizeOfType; + SizeOfType = getSizeOfArgType(Product->getLHS(), true); + if (SizeOfType != QualType()) { + SizeOf = SizeOfType; + Factor = Product->getRHS(); + return true; + } + SizeOfType = getSizeOfArgType(Product->getRHS(), true); + if (SizeOfType != QualType()) { + SizeOf = SizeOfType; + Factor = Product->getLHS(); + return true; + } + return false; +} + +static bool typesAreCompatibleIgnoreQualifiers(ASTContext &Context, + QualType &LHSType, + QualType &RHSType) { + if (Context.typesAreCompatible(LHSType, RHSType)) + return true; + + if (LHSType->isAnyPointerType() && RHSType->isAnyPointerType()) { + QualType LHSPointee = LHSType->getPointeeType().getUnqualifiedType(); + QualType RHSPointee = RHSType->getPointeeType().getUnqualifiedType(); + return typesAreCompatibleIgnoreQualifiers(Context, LHSPointee, RHSPointee); + } + return false; +} + +static bool isSizeOfArgFactorTypeCompatible(ASTContext &Context, + QualType &SizeOfArgFactorTy, + QualType &EltType) { + QualType LHSType = + Context.getCanonicalType(SizeOfArgFactorTy).getUnqualifiedType(); + QualType RHSType = Context.getCanonicalType(EltType).getUnqualifiedType(); + + if (LHSType->hasSignedIntegerRepresentation() && + LHSType != Context.getWCharType()) + LHSType = Context.getCorrespondingUnsignedType(LHSType); + + if (RHSType->hasSignedIntegerRepresentation() && + RHSType != Context.getWCharType()) + RHSType = Context.getCorrespondingUnsignedType(RHSType); + + return typesAreCompatibleIgnoreQualifiers(Context, LHSType, RHSType); +} + /// \brief Check for dangerous or invalid arguments to memset(). /// /// This issues warnings on known problematic, dangerous or unspecified @@ -4741,9 +4797,16 @@ Call->getLocStart(), Call->getRParenLoc())) return; - // We have special checking when the length is a sizeof expression. + // We have special checking when the length is a sizeof expression, or + // a product where one factor is a sizeof expression. QualType SizeOfArgTy = getSizeOfArgType(LenExpr); const Expr *SizeOfArg = getSizeOfExprArg(LenExpr); + + QualType SizeOfArgFactorTy; + const Expr *OtherSizeOfFactor; + bool SizeOfIsFactor = + getSizeOfArgTypeProduct(LenExpr, SizeOfArgFactorTy, OtherSizeOfFactor); + llvm::FoldingSetNodeID SizeOfArgID; for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) { @@ -4840,6 +4903,18 @@ if (PointeeTy == QualType()) continue; + if (SizeOfIsFactor) { + QualType EltType(QualType(PointeeTy->getBaseElementTypeUnsafe(), 0)); + + if (!EltType->isCharType() && + !isSizeOfArgFactorTypeCompatible(Context, SizeOfArgFactorTy, EltType)) + DiagRuntimeBehavior(LenExpr->getExprLoc(), Dest, + PDiag(diag::warn_sizeof_pointer_product_memaccess) + << FnName << SizeOfArgFactorTy << ArgIdx + << EltType << Dest->getSourceRange() + << LenExpr->getSourceRange()); + } + // Always complain about dynamic classes. bool IsContained; if (const CXXRecordDecl *ContainedRD = Index: test/Analysis/taint-generic.c =================================================================== --- test/Analysis/taint-generic.c +++ test/Analysis/taint-generic.c @@ -143,7 +143,7 @@ int *buf1 = (int*)malloc(ts*sizeof(int)); // expected-warning {{Untrusted data is used to specify the buffer size}} char *dst = (char*)calloc(ts, sizeof(char)); //expected-warning {{Untrusted data is used to specify the buffer size}} bcopy(buf1, dst, ts); // expected-warning {{Untrusted data is used to specify the buffer size}} - __builtin_memcpy(dst, buf1, (ts + 4)*sizeof(char)); // expected-warning {{Untrusted data is used to specify the buffer size}} + __builtin_memcpy(dst, (char*)buf1, (ts + 4)*sizeof(char)); // expected-warning {{Untrusted data is used to specify the buffer size}} // If both buffers are trusted, do not issue a warning. char *dst2 = (char*)malloc(ts*sizeof(char)); // expected-warning {{Untrusted data is used to specify the buffer size}} Index: test/SemaCXX/warn-memset-bad-sizeof.cpp =================================================================== --- test/SemaCXX/warn-memset-bad-sizeof.cpp +++ test/SemaCXX/warn-memset-bad-sizeof.cpp @@ -1,5 +1,8 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s // + +typedef __SIZE_TYPE__ size_t; + 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); @@ -16,6 +19,9 @@ typedef double Mat[4][4]; +typedef int IntType; +typedef unsigned int UIntType; + template inline Dest bit_cast(const Source& source) { Dest dest; @@ -116,6 +122,49 @@ }), 0, sizeof(s)); } +void f1() { + size_t arr[8]; + size_t arr2[8]; + int** a = new int*[4]; + Mat m; + + /* Should warn */ + memset(arr, 0, 8 * sizeof(int)); // expected-warning {{argument to 'sizeof' in 'memset' call has a different type 'int' as the destination; expected 'size_t'}} + memset(a, 0, 4 * sizeof(short*)); // expected-warning {{argument to 'sizeof' in 'memset' call has a different type 'short *' as the destination; expected 'int *'}} + memcpy(arr, arr2, 4 * sizeof(int)); // expected-warning {{argument to 'sizeof' in 'memcpy' call has a different type 'int' as the source; expected 'size_t'}} expected-warning{{argument to 'sizeof' in 'memcpy' call has a different type 'int' as the destination; expected 'size_t'}} + memcpy(arr, a, 1 * sizeof(size_t)); // expected-warning {{argument to 'sizeof' in 'memcpy' call has a different type 'size_t' (aka 'unsigned long') as the source; expected 'int *'}} + + /* Shouldn't warn */ + memcpy(arr, arr2, 4 * sizeof(size_t)); + memset(arr, 0, 8 * sizeof(size_t)); + memset(arr, 0, 8 * sizeof(const size_t)); + memset(&arr, 0, sizeof(size_t) * 3); + memset(a, 0, 4 * sizeof(int*)); + memset(m, 0, sizeof(double) * 16); + + char buf[4 * sizeof(int)]; + memset(buf, 0, 4 * sizeof(int)); + + unsigned int i[4]; + memset(&i, 0, 4 * sizeof(int)); + + IntType i2[4]; + memset(i2, 0, 4 * sizeof(UIntType)); + + wchar_t arr3[8]; + memset(arr3, 0, 2 * sizeof(wchar_t)); + + float arr4[8]; + memset(arr4, 0, 1 * sizeof(float)); + + const int** ca = new const int*[4]; + memset(ca, 0, 1 * sizeof(int*)); + + int const* const** ca2 = new int const* const*[4]; + memset(ca2, 0, 1 * sizeof(int**)); + memset(ca2, 0, 1 * sizeof(volatile int **)); +} + namespace ns { void memset(void* s, char c, int n); void f(int* i) {