Re: r371222 - [Diagnostics] Refactor code for -Wsizeof-pointer-div, catch more cases; also add -Wsizeof-array-div

2019-09-06 Thread Richard Smith via cfe-commits 
In addition to the below, this also seems to be missing any test coverage
for the new warning. Did you forget to add a test file?

On Fri, 6 Sep 2019 at 12:38, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> This reports things like
>
> error: expresion will return the incorrect number of elements in the
> array; the array element type is 'const char *', not 'char *'
>
> which doesn't look quite right...
>
> On Fri, Sep 6, 2019 at 12:11 PM David Bolvansky via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xbolva00
>> Date: Fri Sep  6 09:12:48 2019
>> New Revision: 371222
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=371222=rev
>> Log:
>> [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).
>>
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/test/Sema/div-sizeof-ptr.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371222=371221=371222=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep  6
>> 09:12:48 2019
>> @@ -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">,
>>
>
This warning text doesn't seem right (what is "the incorrect number of
elements"?).

Perhaps "expression does not compute the number of elements in the array
%0; element type is %1, not %2"?

(Also you misspelled "expression".)


> +  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 : W
>>  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<
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=371222=371221=371222=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep  6 09:12:48 2019
>> @@ -13008,7 +13008,7 @@ void Sema::CheckArrayAccess(const Expr *
>>
>>if (ND)
>>  DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr,
>> -PDiag(diag::note_array_index_out_of_bounds)
>> +PDiag(diag::note_array_declared_here)
>>  << ND->getDeclName());
>>  }
>>
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371222=371221=371222=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep  6 09:12:48 2019
>> @@ -9135,7 +9135,7 @@ static void checkArithmeticNull(Sema ,
>><< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
>>  }
>>
>> -static void DiagnoseDivisionSizeofPointer(Sema , Expr *LHS, Expr *RHS,
>> +static void DiagnoseDivisionSizeofPointerOrArray(Sema , Expr *LHS,
>> Expr *RHS,
>>SourceLocation Loc) {
>>const auto *LUE = dyn_cast(LHS);
>>const auto *RUE = dyn_cast(RHS);
>> @@ -9154,16 +9154,29 @@ static void DiagnoseDivisionSizeofPointe
>>else
>>  RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>>
>> -  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
>> -return;
>> -  if (LHSTy->getPointeeType().getCanonicalType() !=
>> RHSTy.getCanonicalType())
>> -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)
>> -

Re: r371222 - [Diagnostics] Refactor code for -Wsizeof-pointer-div, catch more cases; also add -Wsizeof-array-div

2019-09-06 Thread Nico Weber via cfe-commits 
This reports things like

error: expresion will return the incorrect number of elements in the array;
the array element type is 'const char *', not 'char *'

which doesn't look quite right...

On Fri, Sep 6, 2019 at 12:11 PM David Bolvansky via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: xbolva00
> Date: Fri Sep  6 09:12:48 2019
> New Revision: 371222
>
> URL: http://llvm.org/viewvc/llvm-project?rev=371222=rev
> Log:
> [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).
>
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/test/Sema/div-sizeof-ptr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371222=371221=371222=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep  6
> 09:12:48 2019
> @@ -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 : W
>  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<
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=371222=371221=371222=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep  6 09:12:48 2019
> @@ -13008,7 +13008,7 @@ void Sema::CheckArrayAccess(const Expr *
>
>if (ND)
>  DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr,
> -PDiag(diag::note_array_index_out_of_bounds)
> +PDiag(diag::note_array_declared_here)
>  << ND->getDeclName());
>  }
>
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371222=371221=371222=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep  6 09:12:48 2019
> @@ -9135,7 +9135,7 @@ static void checkArithmeticNull(Sema ,
><< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
>  }
>
> -static void DiagnoseDivisionSizeofPointer(Sema , Expr *LHS, Expr *RHS,
> +static void DiagnoseDivisionSizeofPointerOrArray(Sema , Expr *LHS, Expr
> *RHS,
>SourceLocation Loc) {
>const auto *LUE = dyn_cast(LHS);
>const auto *RUE = dyn_cast(RHS);
> @@ -9154,16 +9154,29 @@ static void DiagnoseDivisionSizeofPointe
>else
>  RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>
> -  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
> -return;
> -  if (LHSTy->getPointeeType().getCanonicalType() !=
> RHSTy.getCanonicalType())
> -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;
> +  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;
> +}
> +  } else if (isa(LHSTy) &&

r371222 - [Diagnostics] Refactor code for -Wsizeof-pointer-div, catch more cases; also add -Wsizeof-array-div

2019-09-06 Thread David Bolvansky via cfe-commits 
Author: xbolva00
Date: Fri Sep  6 09:12:48 2019
New Revision: 371222

URL: http://llvm.org/viewvc/llvm-project?rev=371222=rev
Log:
[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).


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/div-sizeof-ptr.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371222=371221=371222=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep  6 09:12:48 
2019
@@ -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 : W
 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<

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=371222=371221=371222=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep  6 09:12:48 2019
@@ -13008,7 +13008,7 @@ void Sema::CheckArrayAccess(const Expr *
 
   if (ND)
 DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr,
-PDiag(diag::note_array_index_out_of_bounds)
+PDiag(diag::note_array_declared_here)
 << ND->getDeclName());
 }
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371222=371221=371222=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep  6 09:12:48 2019
@@ -9135,7 +9135,7 @@ static void checkArithmeticNull(Sema ,
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
-static void DiagnoseDivisionSizeofPointer(Sema , Expr *LHS, Expr *RHS,
+static void DiagnoseDivisionSizeofPointerOrArray(Sema , Expr *LHS, Expr *RHS,
   SourceLocation Loc) {
   const auto *LUE = dyn_cast(LHS);
   const auto *RUE = dyn_cast(RHS);
@@ -9154,16 +9154,29 @@ static void DiagnoseDivisionSizeofPointe
   else
 RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
 
-  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
-return;
-  if (LHSTy->getPointeeType().getCanonicalType() != RHSTy.getCanonicalType())
-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;
+  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;
+}
+  } 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)
+