r371605 - [Diagnostics] Add -Wsizeof-array-div
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 7 08:08:52 PDT 2019
I gave this another try now that we have a compiler with rL372600. Another
thing the warning currently warns on is code like this:
char memory[kOpcodeMemory];
OpcodeFactory opcode_maker(memory, sizeof(memory));
size_t count = sizeof(memory) / sizeof(PolicyOpcode);
or
int32_t fds[sizeof(buffer->data) / sizeof(int32_t)], i, count;
size_t size;
(the latter from wayland).
What do you think about also not emitting the warning if the lhs sizeof is
an array of signed or unsigned char? The warning wants the rhs sizeof to be
sizeof(char) which is 1, and dividing by that doesn't really make sense. So
this might be a change that improves false negative rate while probably not
hurting true positive rate.
On Mon, Sep 23, 2019 at 9:11 AM Dávid Bolvanský <david.bolvansky at gmail.com>
wrote:
> Yeah, this needs to be handled a bit differently (if we want so).
>
> po 23. 9. 2019 o 15:07 Nico Weber <thakis at chromium.org> napísal(a):
>
>> It still warns if the inner array is in a struct. That's probably ok
>> though.
>>
>> struct Point {
>> int xy[2];
>> };
>>
>> void f() {
>> Point points[3];
>> for (int i = 0; i < sizeof(points) / sizeof(int); ++i)
>> (&points[0].xy[0])[i] = 0;
>> }
>>
>> On Mon, Sep 23, 2019 at 8:54 AM Nico Weber <thakis at chromium.org> wrote:
>>
>>> That was fast. Thanks much! :)
>>>
>>> On Mon, Sep 23, 2019 at 8:52 AM Dávid Bolvanský <
>>> david.bolvansky at gmail.com> wrote:
>>>
>>>> Hello,
>>>>
>>>> Thanks for the proposed idea, implemented in rL372600.
>>>>
>>>> po 23. 9. 2019 o 14:23 Nico Weber <thakis at chromium.org> napísal(a):
>>>>
>>>>> We're looking at turning this one.
>>>>>
>>>>> One thing that this warns about that's a false positive where we've
>>>>> seen it is this code for nested arrays:
>>>>>
>>>>> float m[4][4];
>>>>> for (int i = 0; i < sizeof(m) / sizeof(**m); ++i) (&**m)[i] = 0;
>>>>>
>>>>> (Why would anyone write code like this? It's a reduced example;
>>>>> consider e.g. wanting to call std::generate_n() on all elements of a nested
>>>>> array.)
>>>>>
>>>>> Can we make the warning not fire when dividing the size of a nested
>>>>> array by the size of the deepest base type?
>>>>>
>>>>> On Wed, Sep 11, 2019 at 6:58 AM David Bolvansky via cfe-commits <
>>>>> cfe-commits at lists.llvm.org> wrote:
>>>>>
>>>>>> Author: xbolva00
>>>>>> Date: Wed Sep 11 03:59:47 2019
>>>>>> New Revision: 371605
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=371605&view=rev
>>>>>> Log:
>>>>>> [Diagnostics] Add -Wsizeof-array-div
>>>>>>
>>>>>> Summary: Clang version of https://www.viva64.com/en/examples/v706/
>>>>>>
>>>>>> Reviewers: rsmith
>>>>>>
>>>>>> Differential Revision: https://reviews.llvm.org/D67287
>>>>>>
>>>>>> Added:
>>>>>> cfe/trunk/test/Sema/div-sizeof-array.cpp
>>>>>> Modified:
>>>>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>>> cfe/trunk/lib/Sema/SemaExpr.cpp
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11
>>>>>> 03:59:47 2019
>>>>>> @@ -3406,6 +3406,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<DiagGroup<"sizeof-pointer-div">>;
>>>>>> +def warn_division_sizeof_array : Warning<
>>>>>> + "expression does not compute the number of elements in this array;
>>>>>> element "
>>>>>> + "type is %0, not %1">,
>>>>>> + InGroup<DiagGroup<"sizeof-array-div">>;
>>>>>>
>>>>>> def note_function_warning_silence : Note<
>>>>>> "prefix with the address-of operator to silence this warning">;
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>>>>>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019
>>>>>> @@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe
>>>>>> else
>>>>>> RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>>>>>>
>>>>>> - if (!LHSTy->isPointerType() || RHSTy->isPointerType())
>>>>>> - return;
>>>>>> - if
>>>>>> (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() !=
>>>>>> - RHSTy.getCanonicalType().getUnqualifiedType())
>>>>>> - return;
>>>>>> + if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
>>>>>> + if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(),
>>>>>> RHSTy))
>>>>>> + return;
>>>>>>
>>>>>> - S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS <<
>>>>>> LHS->getSourceRange();
>>>>>> - if (const auto *DRE = dyn_cast<DeclRefExpr>(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<DeclRefExpr>(LHSArg)) {
>>>>>> + if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>>>>>> + S.Diag(LHSArgDecl->getLocation(),
>>>>>> diag::note_pointer_declared_here)
>>>>>> + << LHSArgDecl;
>>>>>> + }
>>>>>> + } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
>>>>>> + QualType ArrayElemTy = ArrayTy->getElementType();
>>>>>> + if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
>>>>>> + S.Context.getTypeSize(ArrayElemTy) ==
>>>>>> S.Context.getTypeSize(RHSTy))
>>>>>> + return;
>>>>>> + S.Diag(Loc, diag::warn_division_sizeof_array)
>>>>>> + << LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
>>>>>> + if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
>>>>>> + if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>>>>>> + S.Diag(LHSArgDecl->getLocation(),
>>>>>> diag::note_array_declared_here)
>>>>>> + << LHSArgDecl;
>>>>>> + }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>>
>>>>>> Added: cfe/trunk/test/Sema/div-sizeof-array.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)
>>>>>> +++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019
>>>>>> @@ -0,0 +1,28 @@
>>>>>> +// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
>>>>>> +
>>>>>> +template <typename Ty, int N>
>>>>>> +int f(Ty (&Array)[N]) {
>>>>>> + return sizeof(Array) / sizeof(Ty); // Should not warn
>>>>>> +}
>>>>>> +
>>>>>> +typedef int int32;
>>>>>> +
>>>>>> +void test(void) {
>>>>>> + int arr[12]; // expected-note 2 {{array 'arr'
>>>>>> declared here}}
>>>>>> + unsigned long long arr2[4];
>>>>>> + int *p = &arr[0];
>>>>>> + int a1 = sizeof(arr) / sizeof(*arr);
>>>>>> + int a2 = sizeof arr / sizeof p; // expected-warning {{expression
>>>>>> does not compute the number of elements in this array; element type is
>>>>>> 'int', not 'int *'}}
>>>>>> + int a4 = sizeof arr2 / sizeof p;
>>>>>> + int a5 = sizeof(arr) / sizeof(short); // expected-warning
>>>>>> {{expression does not compute the number of elements in this array; element
>>>>>> type is 'int', not 'short'}}
>>>>>> + int a6 = sizeof(arr) / sizeof(int32);
>>>>>> + int a7 = sizeof(arr) / sizeof(int);
>>>>>> + int a9 = sizeof(arr) / sizeof(unsigned int);
>>>>>> + const char arr3[2] = "A";
>>>>>> + int a10 = sizeof(arr3) / sizeof(char);
>>>>>> +
>>>>>> + int arr4[10][12]; // expected-note 3
>>>>>> {{array 'arr4' declared here}}
>>>>>> + int b1 = sizeof(arr4) / sizeof(arr2[12]); // expected-warning
>>>>>> {{expression does not compute the number of elements in this array; element
>>>>>> type is 'int [12]', not 'unsigned long long'}}
>>>>>> + int b2 = sizeof(arr4) / sizeof(int *); // expected-warning
>>>>>> {{expression does not compute the number of elements in this array; element
>>>>>> type is 'int [12]', not 'int *'}}
>>>>>> + int b3 = sizeof(arr4) / sizeof(short *); // expected-warning
>>>>>> {{expression does not compute the number of elements in this array; element
>>>>>> type is 'int [12]', not 'short *'}}
>>>>>> +}
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at lists.llvm.org
>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191007/ffe2ac8c/attachment.html>
More information about the cfe-commits
mailing list