From fd07568074689654e78ef38e700f9a019ffa42d3 Mon Sep 17 00:00:00 2001 From: David Bolvansky Date: Fri, 6 Sep 2019 16:12:48 +0000 Subject: [PATCH] [Diagnostics] Refactor code for -Wsizeof-pointer-div, catch more cases; also add -Wsizeof-array-div Previously, -Wsizeof-pointer-div failed to catch: const int *r; sizeof(r) / sizeof(int); Now fixed. Also introduced -Wsizeof-array-div which catches bugs like: sizeof(r) / sizeof(short); (Array element type does not match type of sizeof operand). llvm-svn: 371222 --- .../clang/Basic/DiagnosticSemaKinds.td | 6 +++- clang/lib/Sema/SemaChecking.cpp | 2 +- clang/lib/Sema/SemaExpr.cpp | 35 +++++++++++++------ clang/test/Sema/div-sizeof-ptr.cpp | 18 +++++----- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 00d05f971b9d..cbb2688dbf3f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3391,6 +3391,10 @@ def note_pointer_declared_here : Note< def warn_division_sizeof_ptr : Warning< "'%0' will return the size of the pointer, not the array itself">, InGroup>; +def warn_division_sizeof_array : Warning< + "expresion will return the incorrect number of elements in the array; the array " + "element type is %0, not %1">, + InGroup>; def note_function_warning_silence : Note< "prefix with the address-of operator to silence this warning">; @@ -8003,7 +8007,7 @@ def warn_array_index_precedes_bounds : Warning< def warn_array_index_exceeds_bounds : Warning< "array index %0 is past the end of the array (which contains %1 " "element%s2)">, InGroup; -def note_array_index_out_of_bounds : Note< +def note_array_declared_here : Note< "array %0 declared here">; def warn_printf_insufficient_data_args : Warning< diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index b060ec73da7f..7d17b0ecd49a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -13008,7 +13008,7 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, if (ND) DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr, - PDiag(diag::note_array_index_out_of_bounds) + PDiag(diag::note_array_declared_here) << ND->getDeclName()); } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index b5ad208e998a..ca7cf94f9c47 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -9135,7 +9135,7 @@ static void checkArithmeticNull(Sema &S, ExprResult &LHS, ExprResult &RHS, << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); } -static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS, +static void DiagnoseDivisionSizeofPointerOrArray(Sema &S, Expr *LHS, Expr *RHS, SourceLocation Loc) { const auto *LUE = dyn_cast(LHS); const auto *RUE = dyn_cast(RHS); @@ -9154,16 +9154,29 @@ static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS, else RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType(); - if (!LHSTy->isPointerType() || RHSTy->isPointerType()) - return; - if (LHSTy->getPointeeType().getCanonicalType() != RHSTy.getCanonicalType()) - return; + if (LHSTy->isPointerType() && !RHSTy->isPointerType()) { + if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() != + RHSTy.getCanonicalType().getUnqualifiedType()) + return; - S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange(); - if (const auto *DRE = dyn_cast(LHSArg)) { - if (const ValueDecl *LHSArgDecl = DRE->getDecl()) - S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here) - << LHSArgDecl; + S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange(); + if (const auto *DRE = dyn_cast(LHSArg)) { + if (const ValueDecl *LHSArgDecl = DRE->getDecl()) + S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here) + << LHSArgDecl; + } + } else if (isa(LHSTy) && !RHSTy->isArrayType()) { + QualType ArrayElemTy = cast(LHSTy)->getElementType(); + if (isa(ArrayElemTy) || + ArrayElemTy.getCanonicalType().getUnqualifiedType() == + RHSTy.getCanonicalType().getUnqualifiedType()) + return; + S.Diag(Loc, diag::warn_division_sizeof_array) << ArrayElemTy << RHSTy; + if (const auto *DRE = dyn_cast(LHSArg)) { + if (const ValueDecl *LHSArgDecl = DRE->getDecl()) + S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here) + << LHSArgDecl; + } } } @@ -9200,7 +9213,7 @@ QualType Sema::CheckMultiplyDivideOperands(ExprResult &LHS, ExprResult &RHS, return InvalidOperands(Loc, LHS, RHS); if (IsDiv) { DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv); - DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc); + DiagnoseDivisionSizeofPointerOrArray(*this, LHS.get(), RHS.get(), Loc); } return compType; } diff --git a/clang/test/Sema/div-sizeof-ptr.cpp b/clang/test/Sema/div-sizeof-ptr.cpp index 835c858e5922..abb7bbadf0e4 100644 --- a/clang/test/Sema/div-sizeof-ptr.cpp +++ b/clang/test/Sema/div-sizeof-ptr.cpp @@ -7,18 +7,20 @@ int f(Ty (&Array)[N]) { typedef int int32; -void test(int *p, int **q) { // expected-note 5 {{pointer 'p' declared here}} - int a1 = sizeof(p) / sizeof(*p); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} - int a2 = sizeof p / sizeof *p; // expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}} - int a3 = sizeof(p) / sizeof(int); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} - int a4 = sizeof(p) / sizeof(p[0]); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} +void test(int *p, int **q) { // expected-note 5 {{pointer 'p' declared here}} + const int *r; // expected-note {{pointer 'r' declared here}} + int a1 = sizeof(p) / sizeof(*p); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} + int a2 = sizeof p / sizeof *p; // expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}} + int a3 = sizeof(p) / sizeof(int); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} + int a4 = sizeof(p) / sizeof(p[0]); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} int a5 = sizeof(p) / sizeof(int32); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}} + int a6 = sizeof(r) / sizeof(int); // expected-warning {{'sizeof (r)' will return the size of the pointer, not the array itself}} int32 *d; // expected-note 2 {{pointer 'd' declared here}} - int a6 = sizeof(d) / sizeof(int32); // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}} - int a7 = sizeof(d) / sizeof(int); // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}} + int a7 = sizeof(d) / sizeof(int32); // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}} + int a8 = sizeof(d) / sizeof(int); // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}} - int a8 = sizeof(*q) / sizeof(**q); // expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}} + int a9 = sizeof(*q) / sizeof(**q); // expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}} // Should not warn int b1 = sizeof(int *) / sizeof(int);